From 5f3e3b09cf214e7d15d1e02ab58675cb79ef0606 Mon Sep 17 00:00:00 2001 From: Andres Uribe Gonzalez Date: Tue, 3 Jan 2023 15:56:11 -0500 Subject: [PATCH 1/3] Supporting cancellation --- .../presentation_exchange_integration_test.go | 4 +- pkg/server/server_operation_test.go | 6 +-- pkg/server/server_presentation_test.go | 18 ++++---- pkg/service/manifest/service.go | 1 + pkg/service/manifest/storage/storage.go | 17 ++++---- pkg/service/operation/credential/response.go | 5 +++ pkg/service/operation/storage.go | 42 +++++++++++++------ pkg/service/operation/storage/storage.go | 12 ++++++ .../operation/submission/submission.go | 11 ----- .../operation/submission/submission_test.go | 3 +- 10 files changed, 74 insertions(+), 45 deletions(-) diff --git a/integration/presentation_exchange_integration_test.go b/integration/presentation_exchange_integration_test.go index f290c8cd5..190d52201 100644 --- a/integration/presentation_exchange_integration_test.go +++ b/integration/presentation_exchange_integration_test.go @@ -5,7 +5,7 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/assert" - "github.com/tbd54566975/ssi-service/pkg/service/operation/submission" + "github.com/tbd54566975/ssi-service/pkg/service/operation/storage" ) var presentationExchangeContext = NewTestContext("PresentationExchange") @@ -109,7 +109,7 @@ func TestSubmissionFlow(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "false", done) - reviewOutput, err := ReviewSubmission(submission.ID(opID)) + reviewOutput, err := ReviewSubmission(storage.StatusObjectID(opID)) assert.NoError(t, err) status, err := getJSONElement(reviewOutput, "$.status") assert.NoError(t, err) diff --git a/pkg/server/server_operation_test.go b/pkg/server/server_operation_test.go index 2f91eca6b..647d17164 100644 --- a/pkg/server/server_operation_test.go +++ b/pkg/server/server_operation_test.go @@ -15,7 +15,7 @@ import ( "github.com/stretchr/testify/require" "github.com/tbd54566975/ssi-service/pkg/server/router" "github.com/tbd54566975/ssi-service/pkg/service/operation" - "github.com/tbd54566975/ssi-service/pkg/service/operation/submission" + opstorage "github.com/tbd54566975/ssi-service/pkg/service/operation/storage" "github.com/tbd54566975/ssi-service/pkg/storage" ) @@ -28,7 +28,7 @@ func TestOperationsAPI(t *testing.T) { holderSigner, holderDID := getSigner(t) definition := createPresentationDefinition(t, pRouter) submissionOp := createSubmission(t, pRouter, definition.PresentationDefinition.ID, VerifiableCredential(), holderDID, holderSigner) - sub := reviewSubmission(t, pRouter, submission.ID(submissionOp.ID)) + sub := reviewSubmission(t, pRouter, opstorage.StatusObjectID(submissionOp.ID)) createdID := submissionOp.ID req := httptest.NewRequest( @@ -257,7 +257,7 @@ func TestOperationsAPI(t *testing.T) { holderSigner, holderDID := getSigner(t) definition := createPresentationDefinition(t, pRouter) submissionOp := createSubmission(t, pRouter, definition.PresentationDefinition.ID, VerifiableCredential(), holderDID, holderSigner) - _ = reviewSubmission(t, pRouter, submission.ID(submissionOp.ID)) + _ = reviewSubmission(t, pRouter, opstorage.StatusObjectID(submissionOp.ID)) createdID := submissionOp.ID req := httptest.NewRequest( diff --git a/pkg/server/server_presentation_test.go b/pkg/server/server_presentation_test.go index 3da290f23..a86908b75 100644 --- a/pkg/server/server_presentation_test.go +++ b/pkg/server/server_presentation_test.go @@ -20,7 +20,7 @@ import ( "github.com/tbd54566975/ssi-service/config" "github.com/tbd54566975/ssi-service/internal/keyaccess" "github.com/tbd54566975/ssi-service/pkg/server/router" - opsubmission "github.com/tbd54566975/ssi-service/pkg/service/operation/submission" + opstorage "github.com/tbd54566975/ssi-service/pkg/service/operation/storage" "github.com/tbd54566975/ssi-service/pkg/service/presentation" "github.com/tbd54566975/ssi-service/pkg/service/presentation/model" "github.com/tbd54566975/ssi-service/pkg/storage" @@ -163,14 +163,14 @@ func TestPresentationAPI(t *testing.T) { "id": "did:web:andresuribe.com", })), holderDID, holderSigner) - req := httptest.NewRequest(http.MethodGet, fmt.Sprintf("https://ssi-service.com/v1/presentations/submissions/%s", opsubmission.ID(op.ID)), nil) + req := httptest.NewRequest(http.MethodGet, fmt.Sprintf("https://ssi-service.com/v1/presentations/submissions/%s", opstorage.StatusObjectID(op.ID)), nil) w := httptest.NewRecorder() - assert.NoError(ttt, pRouter.GetSubmission(newRequestContextWithParams(map[string]string{"id": opsubmission.ID(op.ID)}), w, req)) + assert.NoError(ttt, pRouter.GetSubmission(newRequestContextWithParams(map[string]string{"id": opstorage.StatusObjectID(op.ID)}), w, req)) var resp router.GetSubmissionResponse assert.NoError(ttt, json.NewDecoder(w.Body).Decode(&resp)) - assert.Equal(ttt, opsubmission.ID(op.ID), resp.Submission.ID) + assert.Equal(ttt, opstorage.StatusObjectID(op.ID), resp.Submission.ID) assert.Equal(ttt, definition.PresentationDefinition.ID, resp.Submission.DefinitionID) assert.Equal(ttt, "pending", resp.Submission.Status) }) @@ -218,7 +218,7 @@ func TestPresentationAPI(t *testing.T) { } value := newRequestValue(ttt, request) - createdID := opsubmission.ID(submissionOp.ID) + createdID := opstorage.StatusObjectID(submissionOp.ID) req := httptest.NewRequest( http.MethodPut, fmt.Sprintf("https://ssi-service.com/v1/presentations/submissions/%s/review", createdID), @@ -243,7 +243,7 @@ func TestPresentationAPI(t *testing.T) { holderSigner, holderDID := getSigner(ttt) definition := createPresentationDefinition(ttt, pRouter) submissionOp := createSubmission(t, pRouter, definition.PresentationDefinition.ID, VerifiableCredential(), holderDID, holderSigner) - createdID := opsubmission.ID(submissionOp.ID) + createdID := opstorage.StatusObjectID(submissionOp.ID) _ = reviewSubmission(ttt, pRouter, createdID) request := router.ReviewSubmissionRequest{ @@ -323,14 +323,14 @@ func TestPresentationAPI(t *testing.T) { { Status: "pending", PresentationSubmission: &exchange.PresentationSubmission{ - ID: opsubmission.ID(op.ID), + ID: opstorage.StatusObjectID(op.ID), DefinitionID: definition.PresentationDefinition.ID, }, }, { Status: "pending", PresentationSubmission: &exchange.PresentationSubmission{ - ID: opsubmission.ID(op2.ID), + ID: opstorage.StatusObjectID(op2.ID), DefinitionID: definition.PresentationDefinition.ID, }, }, @@ -395,7 +395,7 @@ func TestPresentationAPI(t *testing.T) { { Status: "pending", PresentationSubmission: &exchange.PresentationSubmission{ - ID: opsubmission.ID(op.ID), + ID: opstorage.StatusObjectID(op.ID), DefinitionID: definition.PresentationDefinition.ID, }, }, diff --git a/pkg/service/manifest/service.go b/pkg/service/manifest/service.go index 378458f8d..c42d19da4 100644 --- a/pkg/service/manifest/service.go +++ b/pkg/service/manifest/service.go @@ -271,6 +271,7 @@ func (s Service) ProcessApplicationSubmission(request model.SubmitApplicationReq applicantDID := request.ApplicantDID storageRequest := manifeststg.StoredApplication{ ID: applicationID, + Status: opcredential.StatusFulfilled, ManifestID: manifestID, ApplicantDID: applicantDID, Application: request.Application, diff --git a/pkg/service/manifest/storage/storage.go b/pkg/service/manifest/storage/storage.go index b28648d9e..2c0bfdbbf 100644 --- a/pkg/service/manifest/storage/storage.go +++ b/pkg/service/manifest/storage/storage.go @@ -6,6 +6,7 @@ import ( "github.com/goccy/go-json" "github.com/pkg/errors" "github.com/sirupsen/logrus" + "github.com/tbd54566975/ssi-service/pkg/service/operation/credential" "github.com/TBD54566975/ssi-sdk/credential/manifest" @@ -16,9 +17,9 @@ import ( ) const ( - manifestNamespace = "manifest" - applicationNamespace = "application" - responseNamespace = "response" + manifestNamespace = "manifest" + + responseNamespace = "response" ) type StoredManifest struct { @@ -30,6 +31,8 @@ type StoredManifest struct { type StoredApplication struct { ID string `json:"id"` + Status credential.Status `json:"status"` + Reason string `json:"reason"` ManifestID string `json:"manifestId"` ApplicantDID string `json:"applicantDid"` Application manifest.CredentialApplication `json:"application"` @@ -132,11 +135,11 @@ func (ms *Storage) StoreApplication(application StoredApplication) error { if err != nil { return util.LoggingErrorMsgf(err, "could not store application: %s", id) } - return ms.db.Write(applicationNamespace, id, applicationBytes) + return ms.db.Write(credential.ApplicationNamespace, id, applicationBytes) } func (ms *Storage) GetApplication(id string) (*StoredApplication, error) { - applicationBytes, err := ms.db.Read(applicationNamespace, id) + applicationBytes, err := ms.db.Read(credential.ApplicationNamespace, id) if err != nil { return nil, util.LoggingErrorMsgf(err, "could not get application: %s", id) } @@ -152,7 +155,7 @@ func (ms *Storage) GetApplication(id string) (*StoredApplication, error) { // GetApplications attempts to get all stored applications. It will return those it can even if it has trouble with some. func (ms *Storage) GetApplications() ([]StoredApplication, error) { - gotApplications, err := ms.db.ReadAll(applicationNamespace) + gotApplications, err := ms.db.ReadAll(credential.ApplicationNamespace) if err != nil { return nil, util.LoggingErrorMsg(err, "could not get all applications") } @@ -173,7 +176,7 @@ func (ms *Storage) GetApplications() ([]StoredApplication, error) { } func (ms *Storage) DeleteApplication(id string) error { - if err := ms.db.Delete(applicationNamespace, id); err != nil { + if err := ms.db.Delete(credential.ApplicationNamespace, id); err != nil { return util.LoggingErrorMsgf(err, "could not delete application: %s", id) } return nil diff --git a/pkg/service/operation/credential/response.go b/pkg/service/operation/credential/response.go index e1e1f4d87..16b403cde 100644 --- a/pkg/service/operation/credential/response.go +++ b/pkg/service/operation/credential/response.go @@ -22,6 +22,8 @@ func (s Status) String() string { return "fulfilled" case StatusRejected: return "rejected" + case StatusCancelled: + return "cancelled" default: return "unknown" } @@ -32,4 +34,7 @@ const ( StatusPending StatusFulfilled StatusRejected + StatusCancelled ) + +const ApplicationNamespace = "application" diff --git a/pkg/service/operation/storage.go b/pkg/service/operation/storage.go index 57c2a2109..2462eea3f 100644 --- a/pkg/service/operation/storage.go +++ b/pkg/service/operation/storage.go @@ -7,6 +7,7 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/tbd54566975/ssi-service/internal/util" + "github.com/tbd54566975/ssi-service/pkg/service/operation/credential" opstorage "github.com/tbd54566975/ssi-service/pkg/service/operation/storage" "github.com/tbd54566975/ssi-service/pkg/service/operation/storage/namespace" "github.com/tbd54566975/ssi-service/pkg/service/operation/submission" @@ -23,9 +24,12 @@ type Storage struct { } func (b Storage) CancelOperation(id string) (*opstorage.StoredOperation, error) { - if strings.HasPrefix(id, submission.ParentResource) { - _, opData, err := b.db.UpdateValueAndOperation( - submission.Namespace, submission.ID(id), storage.NewUpdater(map[string]any{ + var opData []byte + var err error + switch { + case strings.HasPrefix(id, submission.ParentResource): + _, opData, err = b.db.UpdateValueAndOperation( + submission.Namespace, opstorage.StatusObjectID(id), storage.NewUpdater(map[string]any{ "status": submission.StatusCancelled, "reason": cancelledReason, }), @@ -34,16 +38,30 @@ func (b Storage) CancelOperation(id string) (*opstorage.StoredOperation, error) "done": true, }), }) - if err != nil { - return nil, errors.Wrap(err, "updating value and op") - } - var op opstorage.StoredOperation - if err = json.Unmarshal(opData, &op); err != nil { - return nil, errors.Wrap(err, "unmarshalling data") - } - return &op, nil + case strings.HasPrefix(id, credential.ParentResource): + _, opData, err = b.db.UpdateValueAndOperation( + credential.ApplicationNamespace, opstorage.StatusObjectID(id), storage.NewUpdater(map[string]any{ + "status": credential.StatusCancelled, + "reason": cancelledReason, + }), + namespace.FromID(id), id, submission.OperationUpdater{ + UpdaterWithMap: storage.NewUpdater(map[string]any{ + "done": true, + }), + }, + ) + default: + return nil, errors.New("unrecognized id structure") + } + + if err != nil { + return nil, errors.Wrap(err, "updating value and op") + } + var op opstorage.StoredOperation + if err = json.Unmarshal(opData, &op); err != nil { + return nil, errors.Wrap(err, "unmarshalling data") } - return nil, errors.New("unrecognized id structure") + return &op, nil } func (b Storage) StoreOperation(op opstorage.StoredOperation) error { diff --git a/pkg/service/operation/storage/storage.go b/pkg/service/operation/storage/storage.go index d56f4d9e4..aca5cc1da 100644 --- a/pkg/service/operation/storage/storage.go +++ b/pkg/service/operation/storage/storage.go @@ -1,6 +1,8 @@ package storage import ( + "strings" + "go.einride.tech/aip/filtering" ) @@ -35,3 +37,13 @@ type Storage interface { DeleteOperation(id string) error CancelOperation(id string) (*StoredOperation, error) } + +// StatusObjectID attempts to parse the submission id from the StatusObjectID of the operation. This is done by taking the last word +// that results from splitting the id by "/". On failures, the empty string is returned. +func StatusObjectID(opID string) string { + i := strings.LastIndex(opID, "/") + if i == -1 { + return "" + } + return opID[(i + 1):] +} diff --git a/pkg/service/operation/submission/submission.go b/pkg/service/operation/submission/submission.go index e938b7e92..e2c853602 100644 --- a/pkg/service/operation/submission/submission.go +++ b/pkg/service/operation/submission/submission.go @@ -2,7 +2,6 @@ package submission import ( "fmt" - "strings" "github.com/goccy/go-json" "github.com/pkg/errors" @@ -15,16 +14,6 @@ func IDFromSubmissionID(id string) string { return fmt.Sprintf("%s/%s", ParentResource, id) } -// ID attempts to parse the submission id from the ID of the operation. This is done by taking the last word -// that results from splitting the id by "/". On failures, the empty string is returned. -func ID(opID string) string { - i := strings.LastIndex(opID, "/") - if i == -1 { - return "" - } - return opID[(i + 1):] -} - const ( // Namespace is the namespace to be used for storing submissions. Namespace = "presentation_submission" diff --git a/pkg/service/operation/submission/submission_test.go b/pkg/service/operation/submission/submission_test.go index 14b43b1ef..27fb5e8d7 100644 --- a/pkg/service/operation/submission/submission_test.go +++ b/pkg/service/operation/submission/submission_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/tbd54566975/ssi-service/pkg/service/operation/storage" ) func TestSubmissionID(t *testing.T) { @@ -30,7 +31,7 @@ func TestSubmissionID(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.want, ID(tt.opID)) + assert.Equal(t, tt.want, storage.StatusObjectID(tt.opID)) }) } } From 41c17e4cfaa754e6f2ca72859b6ece1a2c61f45b Mon Sep 17 00:00:00 2001 From: Andres Uribe Gonzalez Date: Tue, 3 Jan 2023 18:11:23 -0500 Subject: [PATCH 2/3] Simple test for CancelOperation --- pkg/service/operation/storage_test.go | 96 +++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 pkg/service/operation/storage_test.go diff --git a/pkg/service/operation/storage_test.go b/pkg/service/operation/storage_test.go new file mode 100644 index 000000000..3d3d2e840 --- /dev/null +++ b/pkg/service/operation/storage_test.go @@ -0,0 +1,96 @@ +package operation + +import ( + "os" + "testing" + + "github.com/goccy/go-json" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/stretchr/testify/require" + storage2 "github.com/tbd54566975/ssi-service/pkg/service/manifest/storage" + "github.com/tbd54566975/ssi-service/pkg/service/operation/credential" + opstorage "github.com/tbd54566975/ssi-service/pkg/service/operation/storage" + "github.com/tbd54566975/ssi-service/pkg/service/operation/storage/namespace" + "github.com/tbd54566975/ssi-service/pkg/storage" +) + +func setupTestDB(t *testing.T) storage.ServiceStorage { + file, err := os.CreateTemp("", "bolt") + require.NoError(t, err) + name := file.Name() + s, err := storage.NewStorage(storage.Bolt, name) + require.NoError(t, err) + t.Cleanup(func() { + _ = s.Close() + _ = file.Close() + _ = os.Remove(name) + }) + return s +} + +func TestStorage_CancelOperation(t *testing.T) { + s := setupTestDB(t) + data, err := json.Marshal(storage2.StoredApplication{}) + require.NoError(t, err) + require.NoError(t, s.Write(credential.ApplicationNamespace, "hello", data)) + opData, err := json.Marshal(opstorage.StoredOperation{ + ID: "credentials/responses/hello", + Done: false, + }) + require.NoError(t, err) + require.NoError(t, s.Write(namespace.FromParent("credentials/responses"), "credentials/responses/hello", opData)) + + type fields struct { + db storage.ServiceStorage + } + type args struct { + id string + } + tests := []struct { + name string + fields fields + args args + want *opstorage.StoredOperation + wantErr bool + }{ + { + name: "bad id returns error", + fields: fields{ + db: s, + }, + args: args{ + id: "hello", + }, + wantErr: true, + }, + { + name: "operation for application can be cancelled", + fields: fields{ + db: s, + }, + args: args{ + id: "credentials/responses/hello", + }, + want: &opstorage.StoredOperation{ + ID: "credentials/responses/hello", + Done: true, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := Storage{ + db: tt.fields.db, + } + got, err := b.CancelOperation(tt.args.id) + if (err != nil) != tt.wantErr { + t.Errorf("CancelOperation() error = %v, wantErr %v", err, tt.wantErr) + return + } + if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(opstorage.StoredOperation{}, "Response")); diff != "" { + t.Errorf("CancelOperation() -got, +want:\n%s", diff) + } + }) + } +} From 9b218bed24e32bdd52023812577635458fae8756 Mon Sep 17 00:00:00 2001 From: Andres Uribe Gonzalez Date: Tue, 3 Jan 2023 18:43:10 -0500 Subject: [PATCH 3/3] Addded a validation test. --- pkg/service/operation/storage/storage.go | 2 +- pkg/service/operation/storage_test.go | 30 +++++++++++++++++------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/pkg/service/operation/storage/storage.go b/pkg/service/operation/storage/storage.go index aca5cc1da..1ed006e49 100644 --- a/pkg/service/operation/storage/storage.go +++ b/pkg/service/operation/storage/storage.go @@ -38,7 +38,7 @@ type Storage interface { CancelOperation(id string) (*StoredOperation, error) } -// StatusObjectID attempts to parse the submission id from the StatusObjectID of the operation. This is done by taking the last word +// StatusObjectID attempts to parse the submission id from the ID of the operation. This is done by taking the last word // that results from splitting the id by "/". On failures, the empty string is returned. func StatusObjectID(opID string) string { i := strings.LastIndex(opID, "/") diff --git a/pkg/service/operation/storage_test.go b/pkg/service/operation/storage_test.go index 3d3d2e840..7d0dee4a3 100644 --- a/pkg/service/operation/storage_test.go +++ b/pkg/service/operation/storage_test.go @@ -8,7 +8,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/require" - storage2 "github.com/tbd54566975/ssi-service/pkg/service/manifest/storage" + manifeststg "github.com/tbd54566975/ssi-service/pkg/service/manifest/storage" "github.com/tbd54566975/ssi-service/pkg/service/operation/credential" opstorage "github.com/tbd54566975/ssi-service/pkg/service/operation/storage" "github.com/tbd54566975/ssi-service/pkg/service/operation/storage/namespace" @@ -31,15 +31,9 @@ func setupTestDB(t *testing.T) storage.ServiceStorage { func TestStorage_CancelOperation(t *testing.T) { s := setupTestDB(t) - data, err := json.Marshal(storage2.StoredApplication{}) + data, err := json.Marshal(manifeststg.StoredApplication{}) require.NoError(t, err) require.NoError(t, s.Write(credential.ApplicationNamespace, "hello", data)) - opData, err := json.Marshal(opstorage.StoredOperation{ - ID: "credentials/responses/hello", - Done: false, - }) - require.NoError(t, err) - require.NoError(t, s.Write(namespace.FromParent("credentials/responses"), "credentials/responses/hello", opData)) type fields struct { db storage.ServiceStorage @@ -53,6 +47,7 @@ func TestStorage_CancelOperation(t *testing.T) { args args want *opstorage.StoredOperation wantErr bool + done bool }{ { name: "bad id returns error", @@ -77,9 +72,28 @@ func TestStorage_CancelOperation(t *testing.T) { Done: true, }, }, + { + name: "done operation returns error on cancellation", + done: true, + fields: fields{ + db: s, + }, + args: args{ + id: "credentials/responses/hello", + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + + opData, err := json.Marshal(opstorage.StoredOperation{ + ID: "credentials/responses/hello", + Done: tt.done, + }) + require.NoError(t, err) + require.NoError(t, s.Write(namespace.FromParent("credentials/responses"), "credentials/responses/hello", opData)) + b := Storage{ db: tt.fields.db, }