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

Allow excluding people in large groups #93

Merged
merged 5 commits into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
9 changes: 8 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ const { promisify } = require('util')
const pull = require('pull-stream')
const paraMap = require('pull-paramap')
const pullMany = require('pull-many')
const multicb = require('multicb')
const lodashGet = require('lodash.get')
const chunk = require('lodash.chunk')
const clarify = require('clarify-error')
const {
where,
Expand Down Expand Up @@ -245,9 +247,14 @@ module.exports = {
// prettier-ignore
if (err) return cb(clarify(err, "Couldn't post init msg on new epoch when excluding members"))

addMembers(groupId, remainingMembers, {}, (err) => {
const done = multicb()
chunk(remainingMembers, 15).forEach((membersToAdd) => {
addMembers(groupId, membersToAdd, {}, done())
})
done((err) => {
// prettier-ignore
if (err) return cb(clarify(err, "Couldn't re-add remaining members when excluding members"))

return cb()
})
Copy link
Member

Choose a reason for hiding this comment

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

We can do this will pull-streams. This has a couple advantages:

  • we aren't introducing an new module
  • we can control the rate of publishing of these messages (yours could try publishing a lot of messages all at once and trigger a lot of tangle calculations in parallel)
Suggested change
const done = multicb()
chunk(remainingMembers, 15).forEach((membersToAdd) => {
addMembers(groupId, membersToAdd, {}, done())
})
done((err) => {
// prettier-ignore
if (err) return cb(clarify(err, "Couldn't re-add remaining members when excluding members"))
return cb()
})
pull(
pull.values(chunk(remainingMembers, 15)),
pull.asyncMap((members, cb) => addMembers(groupId, members, {}, cb)),
pull.collect((err) => {
// prettier-ignore
if (err) return cb(clarify(err, "Couldn't re-add remaining members when excluding members"))
return cb()
})
})

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done!

})
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@
"envelope-spec": "^1.1.1",
"fast-deep-equal": "^3.1.3",
"is-canonical-base64": "^1.1.1",
"lodash.chunk": "^4.2.0",
"lodash.get": "^4.4.2",
"lodash.set": "^4.3.2",
"multicb": "^1.2.2",
"private-group-spec": "^6.0.0",
"pull-paramap": "^1.2.2",
"pull-stream": "^3.7.0",
Expand Down
76 changes: 76 additions & 0 deletions test/exclude-members.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,79 @@ test("If you're not the excluder nor the excludee then you should still be in th
await p(bob.close)(true)
await p(carol.close)(true)
})

test('Can exclude a person in a group with a lot of members', async (t) => {
const alice = Testbot({
keys: ssbKeys.generate(null, 'alice'),
mfSeed: Buffer.from(
'000000000000000000000000000000000000000000000000000000000000a1ce',
'hex'
),
})

const peers = Array.from({ length: 20 }).map(() => Testbot())

async function sync() {
await Promise.all(peers.map((peer) => replicate(alice, peer)))
}

await alice.tribes2.start()
await Promise.all(peers.map((peer) => peer.tribes2.start()))

const peerRoots = await Promise.all(
peers.map((peer) => p(peer.metafeeds.findOrCreate)())
).catch((err) => t.error(err, 'Error getting root feeds for peers'))
const peerRootIds = peerRoots.map((root) => root.id)

const { id: groupId } = await alice.tribes2.create()

await sync()

await alice.tribes2.addMembers(groupId, peerRootIds.slice(0, 10))
await alice.tribes2.addMembers(groupId, peerRootIds.slice(10))

await sync()

await Promise.all(peers.map((peer) => peer.tribes2.acceptInvite(groupId)))

await alice.tribes2
.excludeMembers(groupId, [peerRootIds[0]])
.then(() =>
t.pass('alice was able to exclude bob in a group with many members')
)
.catch((err) =>
t.error(
err,
'alice was unable to exclude bob in a group with many members'
)
)

await sync()

const [bob, ...others] = peers

const bobGroup = await bob.tribes2.get(groupId)
t.deepEquals(
bobGroup,
{ id: groupId, excluded: true },
'bob is excluded from group'
)

await Promise.all(
others.map((other) => {
return (async () => {
const otherGroup = await other.tribes2.get(groupId)

if (otherGroup.excluded) throw 'got excluded'
if (otherGroup.readKeys.length !== 2) throw 'not enough readkeys'

return otherGroup
})()
})
)
.then(() => t.pass("Others didn't get excluded from the group"))
.catch(() => t.fail('Others got excluded from the group'))

await p(alice.close)(true)
await Promise.all(peers.map((peer) => p(peer.close)(true)))
})