Skip to content
This repository has been archived by the owner on Aug 9, 2021. It is now read-only.

(WIP) Feat/thread access controllers #435

Closed
wants to merge 10 commits into from

Conversation

zachferland
Copy link
Contributor

No description provided.

@zachferland
Copy link
Contributor Author

Still have to work out naming, mostly depends on how we address/name access controllers, and define what makes a unique access controller, thus a unique thread. Moderator access controller name should probably depend on who first mod is, and then would bubble up as different thread addresses

@zachferland zachferland self-assigned this May 17, 2019
@zachferland
Copy link
Contributor Author

Unique thread access controller is defined by rootMod (first mod), if members, and thread name (which is thread name, plus space).

This will likely also change how threads are store in space, will want more of this data, so that you have a full reference to thread, or could store the full thread address. Can also change thread name to include this.

Copy link
Member

@oed oed left a comment

Choose a reason for hiding this comment

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

Gave this a quick eye.

src/3id/keyring.js Show resolved Hide resolved
src/3id/orbitProvider.js Show resolved Hide resolved
'use strict'

const pMapSeries = require('p-map-series')
const AccessController = require('orbit-db-access-controllers/src/access-controller-interface')
Copy link
Member

Choose a reason for hiding this comment

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

This isn't used?

// TODO should memeber and non member threads create different acess controllers
// or how to deal with threads with same name with member and non member (different names?
// address of this access controler probably should just change, then different thread name
this._db = await this._orbitdb.feed(ensureAddress(address), {
Copy link
Member

Choose a reason for hiding this comment

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

What happens here if address is not passed?

}

/* Factory */
static async create (orbitdb, options = {}) {
Copy link
Member

Choose a reason for hiding this comment

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

Seems like there are a bunch of things missing here, but I assume you have the entire model figured out?

src/keyValueStore.js Show resolved Hide resolved
src/thread.js Outdated Show resolved Hide resolved
@zachferland
Copy link
Contributor Author

This first go is probably not sufficient, for our requirements seems like there needs be some causal dependence between the ACL and the data structure. One option we already discussed is to make them part of same struct, otherwise you may be able to have other pointers as well. Many tradeoffs, if separate may to have rebuild entire thread again when receiving AC update (testing each entry against updated AC), if part of the same struct, cant fetch last n entries, need entire struct to build AC.

Also some links for similar problems/work gathered by ipfs for their crdt work

https://github.com/ipfs/dynamic-data-and-capabilities
protocol/research#8
peer-base/peer-star#6
ipfs-inactive/dynamic-data-and-capabilities#25

@zachferland
Copy link
Contributor Author

implementation note, access controller right now is feedstore where each entry is {capability, id }

But options exist here, for more fields, or could be kv store where capability is a key and value is a list of ids, but feedstore seems more efficient.

@zachferland
Copy link
Contributor Author

Now that a thread is also uniquely defined by rootMod and if membersOnly, we could also add that to a a name of thread. Not necessary since that info is also embedded in entire db address by being included in the manifest file.

But also means we should probably offer the option to open a thread by name or address and should store thread subscription as entire address or with name along with rootMod and if members. Thats all the info needed to reference it.

src/3box.js Show resolved Hide resolved
@zachferland
Copy link
Contributor Author

Also need to still inherit from access controller interface to get the event emitters for updates, but some inheritance issues since one goes to es5 and the other is es6 still, need to reconcile those two things

@oed
Copy link
Member

oed commented May 23, 2019

Is it worth updating the entire 3box codebase to es6 maybe?

@zachferland
Copy link
Contributor Author

@oed for the comments here that relate to changes for the upgrade, not related to ACs, ill look at those more closely when rebasing after

@zachferland
Copy link
Contributor Author

Most of it is, or do you mean not traspile to es5? I think the problem is that orbit-db-access-controllers is not transpiled, so our transpiled es5 code is trying to inherit from the still es6 class

@oed
Copy link
Member

oed commented May 23, 2019

Oh, I see. I don't think we need to inherit from them though. I don't in the AC I wrote.

@zachferland
Copy link
Contributor Author

I don't yet either, and is most everything, the only thing was they also inherit that class from the event emitter, so it has this.emit for emitting update events, for the when the AC gets a new entry, which I suppose revalidates the entires after

@oed
Copy link
Member

oed commented May 23, 2019

Ok, I think I missed that.

@zachferland zachferland force-pushed the feat/thread-access-controllers branch from cd4d87f to c89c922 Compare May 24, 2019 00:54
@zachferland zachferland force-pushed the feat/thread-access-controllers branch from c89c922 to e09d8a4 Compare May 24, 2019 00:56
@zachferland zachferland mentioned this pull request May 24, 2019
@zachferland
Copy link
Contributor Author

Closed by #455

@zachferland zachferland closed this Jun 4, 2019
@oed oed deleted the feat/thread-access-controllers branch August 18, 2019 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants