-
Notifications
You must be signed in to change notification settings - Fork 16
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
Prepare DID storage for multiple DIDs per DID Subject #3196
Conversation
- removed tenant option for did:web - moved sql storage from did:web to global
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.
How are we going to keep services and VMs over multiple DIDs belonging to the same subject in sync?
vdr/didweb/manager.go
Outdated
Subject: newDID.String(), | ||
} | ||
var doc *sql.DIDDocument | ||
if doc, err = documentStore.CreateOrUpdate(sqlDid, []sql.SqlVerificationMethod{{ |
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.
You're not really creating a new DID document here? You could have multiple DIDs (even multiple did:web's?) belonging to the same subject, which should then all return the same services and VMs?
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.
True, but I wanted this PR to just update the DB model. In a later PR we'll move this logic one level higher (so the web manager will no longer have a create just a read/list/resolve)
} | ||
|
||
// SqlDIDManager is the implementation of the DIDManager interface | ||
type SqlDIDManager struct { |
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.
already in sql
package, so no need to prepend types with Sql
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.
true, but DIDManager
is the interface. When declaring vars from other packages you should use the interface. We won't see var a sql.SqlDIDManager
anywhere. Also I'm not sure the package will not be renamed after some refactors.
Use the DB as primary and (for now) the network update as side-effect (maybe within the sql transaction) |
Co-authored-by: reinkrul <info@reinkrul.nl>
Co-authored-by: reinkrul <info@reinkrul.nl>
Co-authored-by: reinkrul <info@reinkrul.nl>
closes #3193