From 7098474da3afac93a0af8a9a4a01362b258af30e Mon Sep 17 00:00:00 2001 From: Wout Slakhorst Date: Fri, 21 Jun 2024 08:53:52 +0200 Subject: [PATCH] PR feedback --- storage/sql_migrations/003_did.sql | 11 +-- vdr/didweb/manager.go | 132 +++++++++++++---------------- vdr/sql/did_document.go | 4 +- 3 files changed, 68 insertions(+), 79 deletions(-) diff --git a/storage/sql_migrations/003_did.sql b/storage/sql_migrations/003_did.sql index cfa5f8437c..330b0c58c5 100644 --- a/storage/sql_migrations/003_did.sql +++ b/storage/sql_migrations/003_did.sql @@ -11,7 +11,7 @@ create table did create index did_subject_idx on did (subject); -create table did_document +create table did_document_version ( -- id is v4 uuid id varchar(36) not null primary key, @@ -31,7 +31,7 @@ create table did_verificationmethod -- data is a JSON object containing the verification method data, e.g. the public key. -- When producing the verificationMethod, data is used as JSON base object and the id and type are added. data $TEXT_TYPE not null, - foreign key (did_document_id) references did_document (id) on delete cascade + foreign key (did_document_id) references did_document_version (id) on delete cascade ); -- this table is used to store the services for locally managed DIDs @@ -44,10 +44,11 @@ create table did_service -- data is a JSON object containing the service data, e.g. the serviceEndpoint. -- When producing the service, data is used as JSON base object and the id and type are added. data $TEXT_TYPE not null, - foreign key (did_document_id) references did_document (id) on delete cascade + foreign key (did_document_id) references did_document_version (id) on delete cascade ); -- +goose Down -drop table did; drop table did_verificationmethod; -drop table did_service; \ No newline at end of file +drop table did_service; +drop table did_document_version; +drop table did; diff --git a/vdr/didweb/manager.go b/vdr/didweb/manager.go index 4077943ef7..a1b5777a66 100644 --- a/vdr/didweb/manager.go +++ b/vdr/didweb/manager.go @@ -51,8 +51,7 @@ var _ management.DocumentManager = (*Manager)(nil) // NewManager creates a new Manager to create and update did:web DID documents. func NewManager(rootDID did.DID, tenantPath string, keyStore crypto.KeyStore, db *gorm.DB) *Manager { return &Manager{ - db: db, - //store: &sqlStore{db: db}, + db: db, rootDID: rootDID, tenantPath: tenantPath, keyStore: keyStore, @@ -80,9 +79,6 @@ func (m Manager) Deactivate(ctx context.Context, subjectDID did.DID) error { } return err } - if sqlDocument == nil { - return nil - } err = didStore.Delete(subjectDID) if err != nil { return err @@ -129,46 +125,47 @@ func (m Manager) Create(ctx context.Context, opts management.CreationOptions) (* if err != nil { return nil, nil, fmt.Errorf("parse new DID: %w", err) } - tx := m.db.Begin() - defer tx.Rollback() + var document did.Document + var verificationMethodKey crypto.Key + err = m.db.Transaction(func(tx *gorm.DB) error { + var verificationMethod *did.VerificationMethod + documentStore := sql.NewDIDDocumentManager(tx) - documentStore := sql.NewDIDDocumentManager(tx) + _, err = documentStore.Latest(*newDID) + if err == nil { + return management.ErrDIDAlreadyExists + } + if !errors.Is(err, gorm.ErrRecordNotFound) { + return err + } - _, err = documentStore.Latest(*newDID) - if err == nil { - return nil, nil, management.ErrDIDAlreadyExists - } - if !errors.Is(err, gorm.ErrRecordNotFound) { - return nil, nil, err - } + verificationMethodKey, verificationMethod, err = m.createVerificationMethod(ctx, *newDID) + if err != nil { + return err + } + vmAsJson, err := json.Marshal(verificationMethod) + if err != nil { + return err + } - verificationMethodKey, verificationMethod, err := m.createVerificationMethod(ctx, *newDID) - if err != nil { - return nil, nil, err - } - vmAsJson, err := json.Marshal(verificationMethod) - if err != nil { - return nil, nil, err - } + sqlDid := sql.DID{ + ID: newDID.String(), + Subject: newDID.String(), // todo pass through options + } + var doc *sql.DIDDocument + if doc, err = documentStore.CreateOrUpdate(sqlDid, []sql.SqlVerificationMethod{{ + ID: verificationMethod.ID.String(), + DIDDocumentID: sqlDid.ID, + Data: vmAsJson, + }}, nil); err != nil { + return fmt.Errorf("store new DID document: %w", err) + } - sqlDid := sql.DID{ - ID: newDID.String(), - Subject: newDID.String(), - } - var doc *sql.DIDDocument - if doc, err = documentStore.CreateOrUpdate(sqlDid, []sql.SqlVerificationMethod{{ - ID: verificationMethod.ID.String(), - DIDDocumentID: sqlDid.ID, - Data: vmAsJson, - }}, nil); err != nil { - return nil, nil, fmt.Errorf("store new DID document: %w", err) - } + document, err = buildDocument(*newDID, *doc) + return err + }) - document, err := buildDocument(*newDID, *doc) - if err != nil { - return nil, nil, err - } - return &document, verificationMethodKey, tx.Commit().Error + return &document, verificationMethodKey, err } func (m Manager) createVerificationMethod(ctx context.Context, ownerDID did.DID) (crypto.Key, *did.VerificationMethod, error) { @@ -230,15 +227,14 @@ func (m Manager) ListOwned(_ context.Context) ([]did.DID, error) { } func (m Manager) CreateService(_ context.Context, subjectDID did.DID, service did.Service) (*did.Service, error) { - tx := m.db.Begin() - defer tx.Rollback() - - added, err := m.createService(tx, subjectDID, service) - if err != nil { - return nil, err - } + var err error + var added *did.Service + err = m.db.Transaction(func(tx *gorm.DB) error { + added, err = m.createService(tx, subjectDID, service) + return err + }) - return added, tx.Commit().Error + return added, err } func (m Manager) createService(tx *gorm.DB, subjectDID did.DID, service did.Service) (*did.Service, error) { @@ -277,35 +273,27 @@ func (m Manager) UpdateService(_ context.Context, subjectDID did.DID, serviceID // ID not set in new version of the service, use the provided serviceID service.ID = serviceID } - - tx := m.db.Begin() - defer tx.Rollback() - - // first delete - err := m.deleteService(tx, subjectDID, serviceID) - if err != nil { - return nil, err - } - // then add - added, err := m.createService(tx, subjectDID, service) - if err != nil { - return nil, err - } + var added *did.Service + var err error + err = m.db.Transaction(func(tx *gorm.DB) error { + // first delete + err := m.deleteService(tx, subjectDID, serviceID) + if err != nil { + return err + } + // then add + added, err = m.createService(tx, subjectDID, service) + return err + }) //commit and return - return added, tx.Commit().Error + return added, err } func (m Manager) DeleteService(_ context.Context, subjectDID did.DID, serviceID ssi.URI) error { - tx := m.db.Begin() - defer tx.Rollback() - - err := m.deleteService(tx, subjectDID, serviceID) - if err != nil { - return err - } - - return tx.Commit().Error + return m.db.Transaction(func(tx *gorm.DB) error { + return m.deleteService(tx, subjectDID, serviceID) + }) } func (m Manager) deleteService(tx *gorm.DB, subjectDID did.DID, serviceID ssi.URI) error { diff --git a/vdr/sql/did_document.go b/vdr/sql/did_document.go index 8464650e00..e78ab4851e 100644 --- a/vdr/sql/did_document.go +++ b/vdr/sql/did_document.go @@ -27,7 +27,7 @@ import ( "gorm.io/gorm/schema" ) -// DIDDocument is the gorm representation of the DID table +// DIDDocument is the gorm representation of the did_document_version table type DIDDocument struct { ID string `gorm:"primaryKey"` DidID string `gorm:"column:did"` @@ -38,7 +38,7 @@ type DIDDocument struct { } func (d DIDDocument) TableName() string { - return "did_document" + return "did_document_version" } var _ DIDDocumentManager = (*SqlDIDDocumentManager)(nil)