Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patchwork integration #54

Open
wants to merge 53 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
69cf20e
send each shard with attachment if attached
Mar 19, 2019
bcf159d
remove log
Mar 19, 2019
98883aa
minimal assert function
ameba23 Apr 2, 2019
82c9c2b
function for unpacking blob ref and encryption key
ameba23 Apr 2, 2019
8f26469
share method handles attachment
ameba23 Apr 2, 2019
68b2d13
test for share method handles attachment
ameba23 Apr 2, 2019
dc88834
add test for unencrypted blob ref
ameba23 Apr 2, 2019
a3b0106
fix error handling
ameba23 Apr 2, 2019
1d832f3
shared also contains attachment name
ameba23 Apr 2, 2019
e45cf26
update test
ameba23 Apr 2, 2019
ddc9f58
add forward-request publish method
ameba23 Apr 3, 2019
f526c63
add test for forward-request publish method
ameba23 Apr 3, 2019
068db25
add query for forwardRequest from others
ameba23 Apr 3, 2019
ed9b841
add query for forwardRequest from yourself
ameba23 Apr 3, 2019
ccc662a
fix type being camel case
ameba23 Apr 4, 2019
192cd0d
add query for forwardRequest by author
ameba23 Apr 4, 2019
27d3018
expose forwardRequests methods
ameba23 Apr 4, 2019
61a7283
query for forwardRequest by secret owner
ameba23 Apr 4, 2019
7ec787b
query for forwardRequests relating to shards you hold
ameba23 Apr 4, 2019
877e0a6
export method
ameba23 Apr 4, 2019
5ba42f3
use lodash get
ameba23 Apr 4, 2019
1f3399f
bug in methods.js
ameba23 Apr 5, 2019
6a3e3fd
typo
ameba23 Apr 5, 2019
45eac4f
split publish into publishAll function
Apr 5, 2019
10f7157
add publishAll to methods API
Apr 5, 2019
77ff194
normalise recps as feedIds
Apr 5, 2019
ae0369f
find only forward requests relating to shards for SSB identities
ameba23 Apr 5, 2019
20d6041
Merge pull request #52 from blockades/add-blob-reference-on-shard-pub…
ameba23 Apr 7, 2019
e564791
Merge pull request #53 from blockades/forward-request
ameba23 Apr 7, 2019
d6d9d7a
fix forward-request test
Apr 8, 2019
a59d724
export forwardRequest sync
Apr 8, 2019
2d6e381
use aysncMap in forOwnShards
ameba23 Apr 8, 2019
771c7ce
fix from-others query
ameba23 Apr 8, 2019
f5c494e
fix from-self query
ameba23 Apr 8, 2019
c854420
change function name
ameba23 Apr 8, 2019
2857197
forOwnShards query takes filter as argument
ameba23 Apr 8, 2019
ad55b44
forOwnShards query filter argument is optional
ameba23 Apr 8, 2019
b359959
forward messages contain id of related shard message
ameba23 Apr 8, 2019
f2d21ce
forward messages contain id of forwardRequest if present
ameba23 Apr 8, 2019
a3bbb62
lint
ameba23 Apr 8, 2019
a603a0e
fix filter function
Apr 10, 2019
6b39cfa
link is required param by schema, not blobId
Apr 10, 2019
ece1f79
comment filter and pass pull/for-own-shards to API
Apr 11, 2019
0f45ea3
forward publish method takes an object, which may contain a requestId
ameba23 Apr 16, 2019
5f8466e
update test
ameba23 Apr 16, 2019
61901a6
update v2 test
ameba23 Apr 16, 2019
3d65df0
fix share method test for link not blobId
ameba23 Apr 16, 2019
7a6b394
remove unused variable
ameba23 Apr 16, 2019
8bcfe87
remove another unused variable
ameba23 Apr 16, 2019
49325a7
fix forwardRequest publishAll test
ameba23 Apr 16, 2019
4ba4640
uncomment filter
ameba23 Apr 16, 2019
817c6e9
readme
ameba23 Apr 16, 2019
dd0c68c
stuff
Apr 25, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions forward-request/async/build.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const { isForwardRequest, errorParser } = require('ssb-dark-crystal-schema')

module.exports = function (server) {
return function buildForwardRequest ({ secretOwner, recp }, callback) {
var content = {
type: 'dark-crystal/forward-request',
version: '1.0.0',
secretOwner,
recps: [recp, server.id]
}

// TODO: check that secretOwner !== recp or sever.id

if (!isForwardRequest(content)) return callback(isForwardRequest.errors)

callback(null, content)
}
}
40 changes: 40 additions & 0 deletions forward-request/async/publish-all.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
const pull = require('pull-stream')
const { isMsg, isFeed } = require('ssb-ref')

const buildForwardRequest = require('../async/build')
const publish = require('../../lib/publish-msg')

module.exports = function (server) {
return function publishAll ({ secretOwner, recps }, callback) {
let feedIds = recps
.map(recp => typeof recp === 'string' ? recp : recp.link)
.filter(Boolean)
.filter(isFeed)

if (feedIds.length !== recps.length) return callback(new Error('forwardRequests publishAll: all recps must be valid feedIds'), recps)
if (!validRecps(feedIds)) return callback(new Error('forwardRequests publishAll: all recps must be valid feedIds', feedIds))
if (!isFeed(secretOwner)) return callback(new Error('forwardRequests publishAll: invalid feedId', secretOwner))

pull(
pull.values(feedIds.map(recp => ({ secretOwner, recp }))),
pull.asyncMap(buildForwardRequest(server)),
pull.collect((err, forwardRequests) => {
if (err) return callback(err)

publishAll(forwardRequests)
})
)

function publishAll (forwardRequests) {
pull(
pull.values(forwardRequests),
pull.asyncMap(publish(server)),
pull.collect(callback)
)
}
}
}

function validRecps (recps) {
return recps.every(isFeed)
}
35 changes: 35 additions & 0 deletions forward-request/pull/by-secret-owner.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
const pull = require('pull-stream')
const next = require('pull-next-query')
const { isForwardRequest } = require('ssb-dark-crystal-schema')
const { isFeedId } = require('ssb-ref')

module.exports = function (server) {
return function bySecretOwner (secretOwner, opts = {}) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is forSecretOwner rather than bySecretOwner, because we're looking for all requests that have been created by person A, who may or may not be person B, and as person C, its a big semantic difference.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im happy to change it. i was imaging its short for 'indexed by secret owner', like we've got a query for shards indexed 'by root'. but if you are seeing it as 'forward requests authored by secret owner' then thats not what we want.

assert(isFeedId(secretOwner), 'secretOwner must be a valid feedId')
const query = [{
$filter: {
value: {
timestamp: { $gt: 0 }, // needed for pull-next-query to stepOn on published timestamp
content: { type: 'dark-crystal/forward-request' }
}
}
}, {
$filter: {
value: {
content: { secretOwner }
}
}
}]

const _opts = Object.assign({}, { query, limit: 100 }, opts)

return pull(
next(server.query.read, _opts),
pull.filter(isForwardRequest)
)
}
}

function assert (test, message) {
if (!test) throw new Error(message || 'AssertionError')
}
32 changes: 32 additions & 0 deletions forward-request/pull/for-own-shards.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
const pull = require('pull-stream')
// const next = require('pull-next-query')
const ShardsFromOthers = require('../../shard/pull/from-others')
const forwardRequestBySecretOwner = require('./by-secret-owner')
const { get } = require('lodash')

module.exports = function (server) {
const shardsFromOthers = ShardsFromOthers(server)

return function forOwnShards (filter, opts = {}) {
if ((typeof filter === 'object') && (opts === null)) {
opts = filter
filter = null
}
if (!filter) filter = () => { return true } // don't filter anything

return pull(
shardsFromOthers(),
// pull.filter(filter),
pull.unique(s => get(s, 'value.author')),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what exactly this is this doing?

pull.asyncMap((shard, cb) => {
pull(
forwardRequestBySecretOwner(get(shard, 'value.author')),
pull.collect((err, forwards) => {
if (err) cb(err)
cb(null, forwards)
})
)
})
)
}
}
29 changes: 29 additions & 0 deletions forward-request/pull/from-others.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const pull = require('pull-stream')
const next = require('pull-next-query')
const { isForwardRequest } = require('ssb-dark-crystal-schema')

module.exports = function (server) {
return function fromOthers (opts = {}) {
const query = [{
$filter: {
value: {
timestamp: { $gt: 0 }, // needed for pull-next-query to stepOn on published timestamp
content: { type: 'dark-crystal/forward-request' }
}
}
}, {
$filter: {
value: {
author: { $ne: server.id }
}
}
}]

const _opts = Object.assign({}, { query, limit: 100 }, opts)

return pull(
next(server.query.read, _opts),
pull.filter(isForwardRequest)
)
}
}
29 changes: 29 additions & 0 deletions forward-request/pull/from-self.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const pull = require('pull-stream')
const next = require('pull-next-query')
const { isForwardRequest } = require('ssb-dark-crystal-schema')

module.exports = function (server) {
return function fromSelf (opts = {}) {
const query = [{
$filter: {
value: {
timestamp: { $gt: 0 }, // needed for pull-next-query to stepOn on published timestamp
content: { type: 'dark-crystal/forward-request' }
}
}
}, {
$filter: {
value: {
author: server.id
}
}
}]

const _opts = Object.assign({}, { query, limit: 100 }, opts)

return pull(
next(server.query.read, _opts),
pull.filter(isForwardRequest)
)
}
}
4 changes: 3 additions & 1 deletion forward/async/build.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const isForward = require('../../isForward')

module.exports = function buildShard (server) {
return function ({ root, shard, shareVersion, recp }, cb) {
return function ({ root, shard, shardId, requestId, shareVersion, recp }, cb) {
// this undoes the privatebox packing we've used to encrypt shards
server.private.unbox(shard, (err, theDecryptedShard) => {
if (err) return cb(err)
Expand All @@ -11,6 +11,8 @@ module.exports = function buildShard (server) {
version: '1.0.0',
root,
shard: theDecryptedShard,
shardId,
requestId,
shareVersion,
recps: [recp, server.id]
}
Expand Down
32 changes: 22 additions & 10 deletions forward/async/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ const get = require('lodash.get')
const PullShardsByRoot = require('../../shard/pull/by-root')
const buildForward = require('./build')
const publish = require('../../lib/publish-msg')
const PullForwardRequestsSecretOwner = require('../../forward-request/pull/by-secret-owner')

module.exports = function (server) {
const pullShardsByRoot = PullShardsByRoot(server)
const pullForwardRequestsSecretOwner = PullForwardRequestsSecretOwner(server)
return function publishForward (root, recp, callback) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be noted too that this breaks the pattern we use in the other publish functions, which expect a single params object, rather than two individual arguments.

if (!ref.isMsgId(root)) return callback(new Error('Invalid rootId'))
pull(
Expand All @@ -25,16 +27,26 @@ module.exports = function (server) {
if (get(shards[0], 'value.author') === recp) {
return callback(new Error('You may not forward a shard to its author. Use reply instead.'))
}

const shareVersion = get(shards[0], 'value.content.version')

const shard = get(shards[0], 'value.content.shard')

buildForward(server)({ root, shard, shareVersion, recp }, (err, content) => {
if (err) return callback(err)

publish(server)(content, callback)
})
pull(
pullForwardRequestsSecretOwner(get(shards[0], 'value.author')),
pull.collect((err, forwardRequests) => {
Copy link
Member

@m4gpi m4gpi Apr 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change doesn't belong here, since its being used here in patchbay-darl-crystal - https://github.com/blockades/patchbay-dark-crystal/blob/master/views/forward/new.js#L99 - where we're not dealing with forward-requests. We either want to do this in a separate action i.e. forward.async.publishRequestReply (ew) or something like that. Or we make this forward publish function not care about getting data beforehand. I.e. it just simply formats the data (build) and publishes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ameba23 unless this accounts for its use elsewhere?

Copy link
Member

@m4gpi m4gpi Apr 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed the Patchwork code to pass in only the rootId of the shard, so this should work for the moment. However, this is restricting as any forward responding to a request will always reference the first one that was returned from the query. See https://github.com/blockades/scuttle-dark-crystal/pull/54/files#r275383477

var requestId = null
if (err) return callback(err)
if (forwardRequests.length > 0) {
requestId = forwardRequests[0]
Copy link
Member

@m4gpi m4gpi Apr 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

If Bob has got a forward-request from Alice2 and Claire, both claiming to be Alice, and Claire's arrived first, we send it to Alice2 (by passing in recp as an argument), but Claire's requestId gets attached instead of Alice2's. When Alice2 receives a forward and tries to cross-reference it with requests sent, the ID's don't match, and Alice2's client gets confused.

Copy link
Member

@m4gpi m4gpi Apr 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should instead be passing in the requestId, but that then buggers up the code in patchbay-dark-crystal. I'm much more in favour of a refactor, having an agnostic forward.async.publish method that just takes params (much like root.async.publish), and then either getting the front-end to do the work, or having separate publish actions for the two different contexts we're now dealing with (with requests - Patchwork / without requests - Patchbay)

}

const shareVersion = get(shards[0], 'value.content.version')
const shardId = get(shards[0], 'key')
const shard = get(shards[0], 'value.content.shard')

buildForward(server)({ root, shard, shardId, requestId, shareVersion, recp }, (err, content) => {
if (err) return callback(err)

publish(server)(content, callback)
})
})
)
})
)
}
Expand Down
3 changes: 3 additions & 0 deletions lib/assert.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = function assert (test, message) {
if (!test) throw new Error(message || 'AssertionError')
}
12 changes: 12 additions & 0 deletions lib/unpackLink.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const assert = require('./assert')
const { isBlobId } = require('ssb-ref')

module.exports = function unpackLink (link) {
const index = link.indexOf('?')
const blobId = link.substring(0, index)
const blobKey = link.substring(index + 1)
assert(((index > 0) && (blobKey.length)), 'Blob not encrypted')
// TODO: test for well formed blob key
assert((isBlobId(blobId)), 'attachment contains invalid blob reference')
return { blobId, blobKey }
}
15 changes: 13 additions & 2 deletions methods.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { isRitual, isRoot, isShard, isForward } = require('ssb-dark-crystal-schema')
const { isRitual, isRoot, isShard, isForward, isForwardRequest } = require('ssb-dark-crystal-schema')
const isRequest = require('./isRequest')
const isReply = require('./isReply')

Expand Down Expand Up @@ -56,12 +56,23 @@ module.exports = {
fromOthersByRoot: require('./forward/pull/from-others-by-root')
}
},
forwardRequest: {
async: {
publishAll: require('./forward-request/async/publish-all')
},
pull: {
bySecretOwner: require('./forward-request/pull/by-secret-owner'),
fromSelf: require('./forward-request/pull/from-self'),
forOwnShards: require('./forward-request/pull/for-own-shards')
}
},
sync: {
isRitual: () => isRitual,
isRoot: () => isRoot,
isShard: () => isShard,
isForward: () => isForward,
isRequest: () => isRequest,
isReply: () => isReply
isReply: () => isReply,
isForwardRequest: () => isForwardRequest
}
}
Loading