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

Fix 'previous' in the members tangle #88

Merged
merged 19 commits into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
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
33 changes: 23 additions & 10 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,27 +214,40 @@ function init (ssb, config) {
* (because they can't post messages to the group before then)
*/

const memberType = (type) => type === 'group/add-member' || type === 'group/exclude-member'
Copy link
Member

Choose a reason for hiding this comment

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

rename isMemberType to hint it will return Boolean

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


/* Tangle: auto-add tangles.group info to all private-group messages */
const getGroupTangle = GetGroupTangle(ssb, keystore)
const getGroupTangle = GetGroupTangle(ssb, keystore, 'group')
const getMembersTangle = GetGroupTangle(ssb, keystore, 'members')
ssb.publish.hook(function (publish, args) {
const [content, cb] = args
if (!content.recps) return publish.apply(this, args)

if (!isGroup(content.recps[0])) return publish.apply(this, args)

onKeystoreReady(() => {
getGroupTangle(content.recps[0], (err, tangle) => {
// NOTE there are two ways an err can occur in getGroupTangle
// 1. recps is not a groupId
// 2. unknown groupId,
if (!keystore.group.has(content.recps[0])) return cb(Error('unknown groupId'))

getGroupTangle(content.recps[0], (err, groupTangle) => {
if (err) return cb(Error("Couldn't get group tangle", { cause: err }))

// Rather than cb(err) here we we pass it on to boxers to see if an err is needed
if (err) return publish.apply(this, args)
// we only want to have to calculate the members tangle if it's gonna be used
const maybeMembersTangle = memberType(content.type)
? getMembersTangle
: (_, cb) => cb()

set(content, 'tangles.group', tangle)
tanglePrune(content) // prune the group tangle down if needed
maybeMembersTangle(content.recps[0], (err, membersTangle) => {
if (err) return cb(Error("Couldn't get members tangle", { cause: err }))

publish.call(this, content, cb)
set(content, 'tangles.group', groupTangle)
tanglePrune(content) // prune the group tangle down if needed
if (memberType(content.type)) {
set(content, 'tangles.members', membersTangle)
tanglePrune(content, 'members')
}

publish.call(this, content, cb)
})
Copy link
Member

Choose a reason for hiding this comment

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

the maybeMembersTangle is not something I'm fond of. Prefer early return?

getGroupTangle(content.recps[0], (err, groupTangle) => {
  if (err) return cb(Error("Couldn't get group tangle", { cause: err }))

  set(content, 'tangles.group', groupTangle)
  tanglePrune(content, 'group')

  if (!memberType(content.type)) {
    return publish.call(this, content, cb)
  }

  getMembersTangle(content.recps[0], (err, membersTangle) => {
    if (err) return cb(Error("Couldn't get members tangle", { cause: err }))

    set(content, 'tangles.members', membersTangle)
    tanglePrune(content, 'members')
    publish.call(this, content, cb)
  })

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yeah that's smart

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

})
})
})
Expand Down
21 changes: 11 additions & 10 deletions lib/get-group-tangle.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ const Strategy = require('@tangle/strategy')

// for figuring out what "previous" should be for the group

const TANGLE = 'group'
const strategy = new Strategy({})

module.exports = function GetGroupTangle (server, keystore) {
module.exports = function GetGroupTangle (server, keystore, tangle = 'group') {
const strategy = new Strategy({})
const cache = new Map([]) // groupId > new Reduce (tangleTips)

// LISTEN
Expand Down Expand Up @@ -50,10 +48,13 @@ module.exports = function GetGroupTangle (server, keystore) {

// if it's in the cache, then get the cached value, then callback
if (cache.has(groupId)) {
return cb(null, {
root: info.root,
previous: Object.keys(cache.get(groupId).state)
})
// this timeout seems to help for some reason. in some cases messages were posted too fast with tangles 'in parallel', e.g. 2 messages both just having previous: [rootMsgId]
return setTimeout(() => {
return cb(null, {
root: info.root,
previous: Object.keys(cache.get(groupId).state)
})
}, 0)
}
// if not in cache, compute it and add to the cache

Expand All @@ -64,7 +65,7 @@ module.exports = function GetGroupTangle (server, keystore) {
value: {
content: {
tangles: {
[TANGLE]: { root: info.root }
[tangle]: { root: info.root }
}
}
}
Expand All @@ -73,7 +74,7 @@ module.exports = function GetGroupTangle (server, keystore) {
{
$map: {
key: ['key'],
previous: ['value', 'content', 'tangles', TANGLE, 'previous']
previous: ['value', 'content', 'tangles', tangle, 'previous']
}
}
]
Expand Down
8 changes: 2 additions & 6 deletions method/group.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,9 @@ module.exports = function GroupMethods (ssb, keystore, state) {
type: 'group/exclude-member',
excludes: authorIds,
tangles: {
members: {
root,
previous: [root] // TODO calculate previous for members tangle
},

members: { root, previous: [root] },
group: { root, previous: [root] }
// NOTE: this is a dummy entry which is over-written in publish hook
// NOTE: these are dummy entries which are over-written in the publish hook
},
recps: [groupId]
}
Expand Down
2 changes: 1 addition & 1 deletion test/api/exclude-members.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ test('tribes.excludeMembers', async t => {
excludes: authorIds,

tangles: {
members: { root: groupInitMsg.key, previous: [groupInitMsg.key] },
members: { root: groupInitMsg.key, previous: [...exclude.content.tangles.members.previous] },
group: { root: groupInitMsg.key, previous: [...exclude.content.tangles.group.previous] }
},
recps: [groupId]
Expand Down
6 changes: 4 additions & 2 deletions test/api/invite.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ test('tribes.invite', async t => {
const { groupId, groupKey, groupInitMsg } = await p(kaitiaki.tribes.create)({})
t.true(groupId, 'creates group')

const selfAdd = await p(kaitiaki.getLatest)(kaitiaki.id)

const authorIds = [
newPerson.id,
FeedId()
Expand All @@ -28,8 +30,8 @@ test('tribes.invite', async t => {
recps: [groupId, ...authorIds],

tangles: {
group: { root: groupInitMsg.key, previous: [groupInitMsg.key] },
members: { root: groupInitMsg.key, previous: [groupInitMsg.key] }
group: { root: groupInitMsg.key, previous: [selfAdd.key] },
members: { root: groupInitMsg.key, previous: [selfAdd.key] }
}
}
t.deepEqual(invite.content, expected, 'kaitiaki sent invite')
Expand Down
85 changes: 66 additions & 19 deletions test/lib/get-group-tangle.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const { promisify: p } = require('util')
const test = require('tape')
const { Server, replicate } = require('../helpers')
const pull = require('pull-stream')
Expand Down Expand Up @@ -89,13 +90,14 @@ test('get-group-tangle (cache)', t => {
server.tribes.create(null, (err, data) => {
if (err) throw err

t.equal(queryCalls, 1, 'no cache for publishing of group/add-member, a backlink query was run')
// 1 for group tangle, 1 for members tangle
t.equal(queryCalls, 2, 'no cache for publishing of group/add-member, a backlink query was run')
const content = { type: 'memo', recps: [data.groupId] }

server.publish(content, (err, msg) => {
if (err) throw err

t.equal(queryCalls, 1, 'cache used for publishing next message')
t.equal(queryCalls, 2, 'cache used for publishing next message')

server.close()
t.end()
Expand Down Expand Up @@ -141,7 +143,7 @@ test(`get-group-tangle-${n}-publishes`, t => {
test('get-group-tangle', t => {
const tests = [
{
plan: 4,
plan: 5,
test: (t) => {
const DESCRIPTION = 'auto adds group tangle'
// this is an integration test, as we've hooked get-group-tangle into ssb.publish
Expand All @@ -150,27 +152,31 @@ test('get-group-tangle', t => {
ssb.tribes.create(null, (err, data) => {
t.error(err, 'create group')

const groupRoot = data.groupInitMsg.key
const groupId = data.groupId
ssb.getLatest(ssb.id, (err, selfAdd) => {
t.error(err, 'get self invite')

const content = {
type: 'yep',
recps: [groupId]
}
const groupRoot = data.groupInitMsg.key
const groupId = data.groupId

ssb.publish(content, (err, msg) => {
t.error(err, 'publish a message')
const content = {
type: 'yep',
recps: [groupId]
}

ssb.get({ id: msg.key, private: true }, (err, A) => {
t.error(err, 'get that message back')
ssb.publish(content, (err, msg) => {
t.error(err, 'publish a message')

t.deepEqual(
A.content.tangles.group, // actual
{ root: groupRoot, previous: [groupRoot] }, // expected
DESCRIPTION + ' (auto added tangles.group)'
)
ssb.get({ id: msg.key, private: true }, (err, A) => {
t.error(err, 'get that message back')

ssb.close()
t.deepEqual(
A.content.tangles.group, // actual
{ root: groupRoot, previous: [selfAdd.key] }, // expected
DESCRIPTION + ' (auto added tangles.group)'
)

ssb.close()
})
})
})
})
Expand Down Expand Up @@ -286,3 +292,44 @@ test('get-group-tangle with branch', t => {
})
}
})

test('members tangle', async t => {
const alice = Server()
const bob = Server()

const { groupId, root } = await p(alice.tribes.create)({})
await p(setTimeout)(300)
const bobInvite = await p(alice.tribes.invite)(groupId, [bob.id], {})

const keystore = { group: { get: () => ({ root }) } }

const _getGroupTangle = p(GetGroupTangle(alice, keystore, 'group'))
const _getMembersTangle = p(GetGroupTangle(alice, keystore, 'members'))
const getGroupTangle = p((id, cb) => {
setTimeout(() => _getGroupTangle(id, cb), 300)
})
const getMembersTangle = p((id, cb) => {
setTimeout(() => _getMembersTangle(id, cb), 300)
})

const firstGroup = await getGroupTangle(groupId)
const firstMembers = await getMembersTangle(groupId)

t.deepEqual(firstGroup, { root, previous: [bobInvite.key] }, 'group tangle generated after add msg is correct')
t.deepEqual(firstMembers, { root, previous: [bobInvite.key] }, 'members tangle generated after add msg is correct')

const { key: bobExcludeKey } = await p(alice.tribes.excludeMembers)(groupId, [bob.id])
const bobExclude = await p(alice.get)({ id: bobExcludeKey, private: true })

t.deepEqual(bobExclude.content.tangles, { group: firstGroup, members: firstMembers }, 'exclude message gets tangles')

const secondGroup = await getGroupTangle(groupId)
const secondMembers = await getMembersTangle(groupId)

t.deepEqual(secondGroup, { root, previous: [bobExcludeKey] }, 'group tangle generated after exclude msg is correct')
t.deepEqual(secondMembers, { root, previous: [bobExcludeKey] }, 'members tangle generated after exclude msg is correct')

await Promise.all([p(alice.close)(), p(bob.close)()])

t.end()
})