-
Notifications
You must be signed in to change notification settings - Fork 57
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
[WIP] SyncEngine for specific protocols + delegate sync. #836
Conversation
🦋 Changeset detectedLatest commit: 96944b7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
540a4ca
to
f7471b2
Compare
090dd73
to
2b0eda3
Compare
2b9391c
to
22d86bd
Compare
ed8a9e4
to
85ec6a5
Compare
2f086dc
to
9ab0463
Compare
3f632b8
to
f264ddb
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #836 +/- ##
==========================================
+ Coverage 93.27% 93.35% +0.07%
==========================================
Files 115 116 +1
Lines 32447 32691 +244
Branches 2547 2598 +51
==========================================
+ Hits 30266 30518 +252
+ Misses 2143 2134 -9
- Partials 38 39 +1
|
b0562bb
to
903fdc2
Compare
TBDocs Report 🛑 Errors: 0 @web5/api
@web5/crypto
@web5/crypto-aws-kms
@web5/dids
@web5/credentials
TBDocs Report Updated at 2024-08-23T18:02:49Z |
5c9bfb1
to
01b5606
Compare
This refactors a lot of what's in #824 with regards to creating/fetching grants. Satisfies: #827 Introduces a `PermissionsApi` interface and an `AgentPermissionsApi` concrete implementation. The interface implements the following methods `fetchGrants`, `fetchRequests`, `isGrantRevoked`, `createGrant`, `createRequest`, `createRevocation` as convenience methods for dealing with the built-in permission protocol records. The `AgentPermissionsApi` implements an additional static method `matchGrantFromArray` which was moved from a `PermissionsUtil` class, which is used to find the appropriate grant to use when authoring a message. A Private API usedin a connected state to find and cache the correct grants to use for the request. A Permissions API which implements `request`, `grant`, `queryRequests`, and `queryGrants` that a user can utilize The `Web5` permissions api introduces 3 helper classes to represent permissions: Class to represent a permission request record. It implements convenience methods similar to the `Record` class where you can `store()`, `import()` or `send()` the underlying request record. Additionally a `grant()` method will create a `PermissionGrant` object. Class to represent a grant record. It implements convenience methods similar to the `Record` class where you can `store()`, `import()` or `send()` the underlying grant record. Additionally a `revoke()` method will create a `GrantRevocation` object, and `isRevoked()` will check if the underlying grant has been revoked. Class to represent a permission grant revocation record. It implements convenience methods similar to the `Record` class where you can `store()` or `send()` the underlying revocation record.
…getting a permission
…the grants from wallet connect
01b5606
to
96944b7
Compare
@@ -74,17 +93,19 @@ export class SyncEngineLevel implements SyncEngine { | |||
|
|||
set agent(agent: Web5PlatformAgent) { | |||
this._agent = agent; | |||
this._cachedPermissionsApi = new CachedPermissions({ agent: agent as Web5Agent, cachedDefault: true }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if agent is set to Web5PlatformAgent
in CachedPermissions
would it fix needing to cast this?
} | ||
}); | ||
|
||
let reply: MessagesReadReply; | ||
|
||
try { | ||
reply = await this.agent.rpc.sendDwnRequest({ | ||
dwnUrl, | ||
targetDid : did, | ||
dwnUrl, targetDid : did, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting mistake
|
||
constructor(options: { agent: Web5Agent, connectedDid: string, delegateDid?: string }) { | ||
this.agent = options.agent; | ||
this.connectedDid = options.connectedDid; | ||
this.delegateDid = options.delegateDid; | ||
this.permissionsApi = new AgentPermissionsApi({ agent: this.agent }); | ||
this.cachedPermissionsApi = new CachedPermissions({ agent: this.agent, cachedDefault: true }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it be named AgentCachedPermissionsApi
for consistency? it seems like all the rest of the Agent...Api
its a class that is coupled to agents and requires passing the Agent singleton into it
/** | ||
* An instance of the `AgentPermissionsApi` that is used to interact with permissions grants used during sync | ||
*/ | ||
private _cachedPermissionsApi: CachedPermissions; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
per the description this is kind of confusing. should there be some sort of inheritance?
* | ||
* This will store the grants as the DWN owner to be used later when impersonating the connected DID. | ||
*/ | ||
static async processConnectedGrants({ grants, agent, delegateDid }: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't seem like the right place for this method? it doesn't rely on any internal state from the Web5 class (from what I can tell) and seems to be pretty highly related to the connect work in Agent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rubber stamp
Resolves #826
This PR adds the ability to Sync a subset of data by providing specific protocols a user would like to sync. Additionally sync now supports using a grant and delegateDid to perform sync.
Introduced a
CachedPermissions
class to help cache the lookup of permission grants records from the DWN. This is used both inDwnApi
andSyncEngineLevel
.