From 15f6aabfad10dbd8e84b2fedbbb2cd254148f173 Mon Sep 17 00:00:00 2001 From: "Ariel Shaqed (Scolnicov)" Date: Wed, 10 Apr 2024 14:17:18 +0300 Subject: [PATCH] Fix middleware and some changes to accessing the changed Go APIs Includes 7e0c91c6e73bb563dcb743e34f82fd5ed6da7528, thanks @Jonathan-Rosenberg! --- esti/hooks_test.go | 4 +-- esti/import_test.go | 6 ++-- esti/merge_test.go | 4 +-- esti/presign_multipart_test.go | 4 +-- esti/system_test.go | 12 ++++---- pkg/api/controller_test.go | 41 +++++++++++++--------------- pkg/api/tmpl/chi/chi-middleware.tmpl | 1 + pkg/auth/service_test.go | 6 ++-- 8 files changed, 36 insertions(+), 42 deletions(-) diff --git a/esti/hooks_test.go b/esti/hooks_test.go index 7a1a1f44b83..5de39d7213a 100644 --- a/esti/hooks_test.go +++ b/esti/hooks_test.go @@ -141,7 +141,7 @@ func testCommitMerge(t *testing.T, ctx context.Context, repo string) { BranchID: branch, Committer: commitRecord.Committer, CommitMessage: commitRecord.Message, - Metadata: commitRecord.Metadata.AdditionalProperties, + Metadata: *commitRecord.Metadata, }, preCommitEvent) require.NotNil(t, webhookData.queryParams) require.Contains(t, webhookData.queryParams, "check_env_vars") @@ -166,7 +166,7 @@ func testCommitMerge(t *testing.T, ctx context.Context, repo string) { CommitID: commitRecord.Id, Committer: commitRecord.Committer, CommitMessage: commitRecord.Message, - Metadata: commitRecord.Metadata.AdditionalProperties, + Metadata: *commitRecord.Metadata, }, postCommitEvent) mergeResp, err := client.MergeIntoBranchWithResponse(ctx, repo, branch, mainBranch, apigen.MergeIntoBranchJSONRequestBody{}) diff --git a/esti/import_test.go b/esti/import_test.go index 3aee101eebf..23329b6e6c4 100644 --- a/esti/import_test.go +++ b/esti/import_test.go @@ -172,7 +172,7 @@ func TestImport(t *testing.T) { require.NoError(t, err) require.NotNil(t, metadataResp.JSON200) - repoMetadata := metadataResp.JSON200.AdditionalProperties + repoMetadata := *metadataResp.JSON200 require.NotNil(t, repoMetadata) importMetadata, ok := repoMetadata[graveler.MetadataKeyLastImportTimeStamp] require.True(t, ok) @@ -264,7 +264,7 @@ func testImportNew(t testing.TB, ctx context.Context, repoName, importBranch str Force: swag.Bool(force), } if len(metadata) > 0 { - body.Commit.Metadata = &apigen.CommitCreation_Metadata{AdditionalProperties: metadata} + body.Commit.Metadata = &metadata } importResp, err := client.ImportStartWithResponse(ctx, repoName, importBranch, body) @@ -314,7 +314,7 @@ func TestImportCancel(t *testing.T) { importResp, err := client.ImportStartWithResponse(ctx, repoName, branch, apigen.ImportStartJSONRequestBody{ Commit: apigen.CommitCreation{ Message: "created by import", - Metadata: &apigen.CommitCreation_Metadata{AdditionalProperties: map[string]string{"created_by": "import"}}, + Metadata: &map[string]string{"created_by": "import"}, }, Paths: []apigen.ImportLocation{{ Destination: importTargetPrefix, diff --git a/esti/merge_test.go b/esti/merge_test.go index 8f05ae2cdf7..0d43d6f413e 100644 --- a/esti/merge_test.go +++ b/esti/merge_test.go @@ -76,8 +76,8 @@ func doMergeAndListIteration(t *testing.T, logger logging.Logger, ctx context.Co res, err := client.GetCommitWithResponse(ctx, repo, mergeRes.JSON200.Reference) require.NoError(t, err, "failed to get commit") require.NotNil(t, res.JSON200) - metadata := res.JSON200.Metadata - val, ok := metadata.AdditionalProperties[graveler.MergeStrategyMetadataKey] + metadata := *res.JSON200.Metadata + val, ok := metadata[graveler.MergeStrategyMetadataKey] require.True(t, ok) require.Equal(t, strategy, val) diff --git a/esti/presign_multipart_test.go b/esti/presign_multipart_test.go index 3c822cc2a2b..e7de8ddfd92 100644 --- a/esti/presign_multipart_test.go +++ b/esti/presign_multipart_test.go @@ -234,9 +234,7 @@ func TestCompletePresignMultipartUpload(t *testing.T) { ContentType: swag.String("application/octet-stream"), Parts: parts, PhysicalAddress: respCreate.JSON201.PhysicalAddress, - UserMetadata: &apigen.CompletePresignMultipartUpload_UserMetadata{ - AdditionalProperties: map[string]string{"foo": "bar"}, - }, + UserMetadata: &map[string]string{"foo": "bar"}, }) require.NoError(t, err, "CompletePresignMultipartUpload should succeed") require.Equalf(t, http.StatusOK, resp.StatusCode(), "CompletePresignMultipartUpload status code mismatch: %s - %s", resp.Status(), resp.Body) diff --git a/esti/system_test.go b/esti/system_test.go index 2d64255ab7d..48e8ed900e1 100644 --- a/esti/system_test.go +++ b/esti/system_test.go @@ -223,13 +223,11 @@ func uploadContentDirect(ctx context.Context, client apigen.ClientWithResponsesI resp, err := client.LinkPhysicalAddressWithResponse(ctx, repoID, branchID, &apigen.LinkPhysicalAddressParams{ Path: objPath, }, apigen.LinkPhysicalAddressJSONRequestBody{ - Checksum: stats.ETag, - SizeBytes: stats.Size, - Staging: *stagingLocation, - UserMetadata: &apigen.StagingMetadata_UserMetadata{ - AdditionalProperties: metadata, - }, - ContentType: &contentType, + Checksum: stats.ETag, + SizeBytes: stats.Size, + Staging: *stagingLocation, + UserMetadata: &metadata, + ContentType: &contentType, }) if err != nil { return nil, fmt.Errorf("link object to backing store: %w", err) diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 70c87ea8e55..aef095a9c48 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -828,14 +828,13 @@ func TestController_GetCommitHandler(t *testing.T) { if len(commit.Id) == 0 { t.Errorf("GetCommit initial commit missing ID: %+v", commit) } - metadata := apigen.Commit_Metadata{} expectedCommit := &apigen.Commit{ Committer: "", CreationDate: commit.CreationDate, Id: commit.Id, Message: graveler.FirstCommitMsg, MetaRangeId: "", - Metadata: &metadata, + Metadata: nil, Parents: []string{}, Generation: swag.Int64(1), Version: swag.Int(1), @@ -1300,15 +1299,15 @@ func TestController_SetRepositoryMetadataHandler(t *testing.T) { }) verifyResponseOK(t, createResp, err) - resp, err := clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: apigen.RepositoryMetadataSet_Metadata{AdditionalProperties: tt1.properties}}) + resp, err := clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: tt1.properties}) verifyResponseOK(t, resp, err) - resp, err = clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: apigen.RepositoryMetadataSet_Metadata{AdditionalProperties: tt1.appendProps}}) + resp, err = clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: tt1.appendProps}) verifyResponseOK(t, resp, err) getResp, err := clt.GetRepositoryMetadataWithResponse(ctx, repoName) verifyResponseOK(t, resp, err) - if diff := deep.Equal(getResp.JSON200.AdditionalProperties, tt1.expected); diff != nil { + if diff := deep.Equal(*getResp.JSON200, apigen.RepositoryMetadata(tt1.expected)); diff != nil { t.Fatal("Get repository metadata results diff:", diff) } }) @@ -1328,13 +1327,13 @@ func TestController_SetRepositoryMetadataHandler(t *testing.T) { }) verifyResponseOK(t, createResp, err) - resp, err := clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: apigen.RepositoryMetadataSet_Metadata{AdditionalProperties: map[string]string{"foo": "bar"}}}) + resp, err := clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: map[string]string{"foo": "bar"}}) verifyResponseOK(t, resp, err) }) t.Run("set repository metadata not exist", func(t *testing.T) { repoName := testUniqueRepoName() - resp, err := clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: apigen.RepositoryMetadataSet_Metadata{AdditionalProperties: map[string]string{"foo": "bar"}}}) + resp, err := clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: map[string]string{"foo": "bar"}}) require.NoError(t, err) require.NotNil(t, resp.JSON404) }) @@ -1352,7 +1351,7 @@ func TestController_SetRepositoryMetadataHandler(t *testing.T) { creds := createUserWithDefaultGroup(t, clt) // create a client with the user regClt := setupClientByEndpoint(t, deps.server.URL, creds.AccessKeyID, creds.SecretAccessKey) - resp, err := regClt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: apigen.RepositoryMetadataSet_Metadata{AdditionalProperties: map[string]string{"foo": "bar"}}}) + resp, err := regClt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: map[string]string{"foo": "bar"}}) require.NoError(t, err) require.NotNil(t, resp.JSON401) }) @@ -1404,7 +1403,7 @@ func TestController_DeleteRepositoryMetadataHandler(t *testing.T) { }) verifyResponseOK(t, createResp, err) - resp, err := clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: apigen.RepositoryMetadataSet_Metadata{AdditionalProperties: tt.properties}}) + resp, err := clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: tt.properties}) verifyResponseOK(t, resp, err) deleteResp, err := clt.DeleteRepositoryMetadataWithResponse(ctx, repoName, apigen.DeleteRepositoryMetadataJSONRequestBody{Keys: tt.deleteProps}) @@ -1412,7 +1411,7 @@ func TestController_DeleteRepositoryMetadataHandler(t *testing.T) { getResp, err := clt.GetRepositoryMetadataWithResponse(ctx, repoName) verifyResponseOK(t, getResp, err) - if diff := deep.Equal(getResp.JSON200.AdditionalProperties, tt.expected); diff != nil { + if diff := deep.Equal(*getResp.JSON200, apigen.RepositoryMetadata(tt.expected)); diff != nil { t.Fatal("Get repository metadata results diff:", diff) } }) @@ -1432,13 +1431,13 @@ func TestController_DeleteRepositoryMetadataHandler(t *testing.T) { }) verifyResponseOK(t, createResp, err) - resp, err := clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: apigen.RepositoryMetadataSet_Metadata{AdditionalProperties: map[string]string{"foo": "bar"}}}) + resp, err := clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: map[string]string{"foo": "bar"}}) verifyResponseOK(t, resp, err) }) t.Run("delete repository metadata, repository not exist", func(t *testing.T) { repoName := testUniqueRepoName() - resp, err := clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: apigen.RepositoryMetadataSet_Metadata{AdditionalProperties: map[string]string{"foo": "bar"}}}) + resp, err := clt.SetRepositoryMetadataWithResponse(ctx, repoName, apigen.SetRepositoryMetadataJSONRequestBody{Metadata: map[string]string{"foo": "bar"}}) require.NoError(t, err) require.NotNil(t, resp.JSON404) }) @@ -1477,8 +1476,7 @@ func TestController_GetRepositoryMetadataHandler(t *testing.T) { resp, err := clt.GetRepositoryMetadataWithResponse(ctx, repoName) verifyResponseOK(t, resp, err) - require.NotNil(t, resp.JSON200) - require.Nil(t, resp.JSON200.AdditionalProperties) + require.Empty(t, resp.JSON200, 0) }) t.Run("get metadata bare repo", func(t *testing.T) { @@ -1496,8 +1494,7 @@ func TestController_GetRepositoryMetadataHandler(t *testing.T) { resp, err := clt.GetRepositoryMetadataWithResponse(ctx, repoName) verifyResponseOK(t, resp, err) - require.NotNil(t, resp.JSON200) - require.Nil(t, resp.JSON200.AdditionalProperties) + require.Empty(t, resp.JSON200) }) t.Run("get repository metadata not exist", func(t *testing.T) { @@ -2328,7 +2325,7 @@ func TestController_ObjectsStatObjectHandler(t *testing.T) { if objectStats.PhysicalAddress != onBlock(deps, "some-bucket/")+entry.PhysicalAddress { t.Fatalf("expected correct PhysicalAddress, got %s", objectStats.PhysicalAddress) } - if diff := deep.Equal(objectStats.Metadata.AdditionalProperties, map[string]string(entry.Metadata)); diff != nil { + if diff := deep.Equal(*objectStats.Metadata, apigen.ObjectUserMetadata(entry.Metadata)); diff != nil { t.Fatalf("expected to get back user-defined metadata: %s", diff) } contentType := apiutil.Value(objectStats.ContentType) @@ -2341,8 +2338,8 @@ func TestController_ObjectsStatObjectHandler(t *testing.T) { resp, err = clt.StatObjectWithResponse(ctx, repo, "main", &apigen.StatObjectParams{Path: "foo/bar", UserMetadata: &getUserMetadata}) verifyResponseOK(t, resp, err) objectStatsNoMetadata := resp.JSON200 - if objectStatsNoMetadata.Metadata == nil || len(objectStatsNoMetadata.Metadata.AdditionalProperties) != 0 { - t.Fatalf("expected to not get back empty user-defined metadata, got %+v", objectStatsNoMetadata.Metadata.AdditionalProperties) + if objectStatsNoMetadata.Metadata == nil || len(*objectStatsNoMetadata.Metadata) != 0 { + t.Fatalf("expected to not get back empty user-defined metadata, got %+v", objectStatsNoMetadata.Metadata) } }) @@ -2439,7 +2436,7 @@ func TestController_ObjectsListObjectsHandler(t *testing.T) { results := resp.JSON200.Results for _, obj := range results { if obj.Metadata != nil { - t.Fatalf("expected no user-defined metadata, got %+v", obj.Metadata.AdditionalProperties) + t.Fatalf("expected no user-defined metadata, got %+v", obj.Metadata) } } }) @@ -5536,7 +5533,7 @@ func TestController_CreateCommitRecord(t *testing.T) { CreationDate: 1e9, Generation: 1, Message: "message", - Metadata: &apigen.CommitRecordCreation_Metadata{AdditionalProperties: map[string]string{"key": "value"}}, + Metadata: &map[string]string{"key": "value"}, MetarangeId: "metarangeId", Parents: []string{"parent1", "parent2"}, Version: 1, @@ -5558,7 +5555,7 @@ func TestController_CreateCommitRecord(t *testing.T) { Committer: body.Committer, Message: body.Message, CreationDate: time.Unix(body.CreationDate, 0).UTC(), - Metadata: body.Metadata.AdditionalProperties, + Metadata: *body.Metadata, MetaRangeID: body.MetarangeId, Parents: body.Parents, Generation: catalog.CommitGeneration(body.Generation), diff --git a/pkg/api/tmpl/chi/chi-middleware.tmpl b/pkg/api/tmpl/chi/chi-middleware.tmpl index cb126ddb31b..c3038ee25e5 100644 --- a/pkg/api/tmpl/chi/chi-middleware.tmpl +++ b/pkg/api/tmpl/chi/chi-middleware.tmpl @@ -4,6 +4,7 @@ type ServerInterfaceWrapper struct { Handler ServerInterface HandlerMiddlewares []MiddlewareFunc + ErrorHandlerFunc func(w http.ResponseWriter, r *http.Request, err error) } type MiddlewareFunc func(http.HandlerFunc) http.HandlerFunc diff --git a/pkg/auth/service_test.go b/pkg/auth/service_test.go index f88d1f715a8..e8da2fb2b3a 100644 --- a/pkg/auth/service_test.go +++ b/pkg/auth/service_test.go @@ -1907,7 +1907,7 @@ func TestAPIAuthService_WritePolicy(t *testing.T) { CreatedAt: creationTime, Statement: []model.Statement{{ Action: tt.firstStatementAction, - Effect: tt.firstStatementEffect, + Effect: string(tt.firstStatementEffect), Resource: tt.firstStatementResource, }}, }, tt.overwrite) @@ -1963,7 +1963,7 @@ func TestAPIAuthService_GetPolicy(t *testing.T) { Statement: []auth.Statement{ { Action: tt.firstStatementAction, - Effect: tt.firstStatementEffect, + Effect: auth.StatementEffect(tt.firstStatementEffect), Resource: tt.firstStatementResource, }, }, @@ -2011,7 +2011,7 @@ func statementEquals(t *testing.T, authStatement []auth.Statement, modalStatemen return } for i, authS := range authStatement { - if authS.Effect != modalStatement[i].Effect { + if string(authS.Effect) != modalStatement[i].Effect { t.Errorf("Effect (authStatement)%s != (modelStatement)%s", modalStatement[i].Effect, authS.Effect) } if authS.Resource != modalStatement[i].Resource {