From 5710411c7e2b65ab88dfcc1ecbd647f3dcbc92f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Garc=C3=ADa=20Crespo?= Date: Tue, 6 Feb 2024 10:11:57 +0000 Subject: [PATCH] Rewrite location config tests using tables --- internal/storage/types/location_config.go | 2 +- .../storage/types/location_config_test.go | 336 +++++++++--------- 2 files changed, 174 insertions(+), 164 deletions(-) diff --git a/internal/storage/types/location_config.go b/internal/storage/types/location_config.go index 678b6653f..339e3e8c1 100644 --- a/internal/storage/types/location_config.go +++ b/internal/storage/types/location_config.go @@ -51,7 +51,7 @@ func (c *LocationConfig) UnmarshalJSON(blob []byte) error { keys := map[string]json.RawMessage{} err := json.Unmarshal(blob, &keys) if err != nil { - return err + return errors.New("undefined configuration format") } if len(keys) > 1 { return errors.New("multiple config values have been assigned") diff --git a/internal/storage/types/location_config_test.go b/internal/storage/types/location_config_test.go index 43e4caf7a..f72169301 100644 --- a/internal/storage/types/location_config_test.go +++ b/internal/storage/types/location_config_test.go @@ -11,183 +11,193 @@ import ( "github.com/artefactual-sdps/enduro/internal/storage/types" ) -func TestLocationConfig(t *testing.T) { +func TestLocationConfigEncoding(t *testing.T) { t.Parallel() - t.Run("Encoding", func(t *testing.T) { - t.Parallel() - - // Invalid config. - cfg := types.LocationConfig{} - blob, err := json.Marshal(cfg) - assert.DeepEqual(t, blob, []byte(nil)) - assert.Error(t, err, "json: error calling MarshalJSON for type types.LocationConfig: unsupported config type: ") - - // Valid S3 config. - cfg = types.LocationConfig{ - Value: &types.S3Config{ - Bucket: "perma-aips-1", - Region: "eu-west-1", + type test struct { + config types.LocationConfig + want string + wantErr string + wantInvalid bool + } + for name, tt := range map[string]test{ + "Encodes valid S3 config": { + config: types.LocationConfig{ + Value: &types.S3Config{ + Bucket: "perma-aips-1", + Region: "eu-west-1", + }, }, - } - blob, err = json.Marshal(cfg) - assert.NilError(t, err) - assert.Equal(t, string(blob), `{"s3":{"bucket":"perma-aips-1","region":"eu-west-1"}}`) - assert.Equal(t, cfg.Value.Valid(), true) - - // Valid SFTP config. - cfg = types.LocationConfig{ - Value: &types.SFTPConfig{ - Address: "sftp:22", - Username: "user", - Password: "secret", - Directory: "upload", + want: `{"s3":{"bucket":"perma-aips-1","region":"eu-west-1"}}`, + }, + "Encodes valid SFTP config": { + config: types.LocationConfig{ + Value: &types.SFTPConfig{ + Address: "sftp:22", + Username: "user", + Password: "secret", + Directory: "upload", + }, }, - } - blob, err = json.Marshal(cfg) - assert.NilError(t, err) - assert.Equal(t, string(blob), `{"sftp":{"address":"sftp:22","username":"user","password":"secret","directory":"upload"}}`) - assert.Equal(t, cfg.Value.Valid(), true) - - // Invalid S3 config. - cfg = types.LocationConfig{ - Value: &types.S3Config{ - Bucket: "perma-aips-1", + want: `{"sftp":{"address":"sftp:22","username":"user","password":"secret","directory":"upload"}}`, + }, + "Encodes valid URL config": { + config: types.LocationConfig{ + Value: &types.URLConfig{ + URL: "mem://", + }, }, - } - blob, err = json.Marshal(cfg) - assert.NilError(t, err) - assert.Equal(t, string(blob), `{"s3":{"bucket":"perma-aips-1","region":""}}`) - assert.Equal(t, cfg.Value.Valid(), false) - - // Invalid SFTP config. - cfg = types.LocationConfig{ - Value: &types.SFTPConfig{ - Address: "sftp:22", - Username: "user", - Directory: "upload", + want: `{"url":{"url":"mem://"}}`, + }, + "Rejects invalid S3 config": { + config: types.LocationConfig{ + Value: &types.S3Config{ + Bucket: "perma-aips-1", + }, }, - } - blob, err = json.Marshal(cfg) - assert.NilError(t, err) - assert.Equal(t, string(blob), `{"sftp":{"address":"sftp:22","username":"user","password":"","directory":"upload"}}`) - assert.Equal(t, cfg.Value.Valid(), false) - }) - - t.Run("Decoding", func(t *testing.T) { - t.Parallel() - - // S3 config. - blob := []byte(`{"s3":{"bucket":"perma-aips-1","region":"eu-west-1"}}`) - cfg := types.LocationConfig{} - err := json.Unmarshal(blob, &cfg) - assert.NilError(t, err) - assert.DeepEqual(t, cfg, types.LocationConfig{ - Value: &types.S3Config{ - Bucket: "perma-aips-1", - Region: "eu-west-1", + want: `{"s3":{"bucket":"perma-aips-1","region":""}}`, + wantInvalid: true, + }, + "Rejects invalid SFTP config": { + config: types.LocationConfig{ + Value: &types.SFTPConfig{}, }, - }) - - // SFTP Config - blob = []byte(`{"sftp":{"address":"sftp:22","username":"user","password":"secret","directory":"upload"}}`) - cfg = types.LocationConfig{} - err = json.Unmarshal(blob, &cfg) - assert.NilError(t, err) - assert.DeepEqual(t, cfg, types.LocationConfig{ - Value: &types.SFTPConfig{ - Address: "sftp:22", - Username: "user", - Password: "secret", - Directory: "upload", + want: `{"sftp":{"address":"","username":"","password":"","directory":""}}`, + wantInvalid: true, + }, + "Rejects invalid URL config": { + config: types.LocationConfig{ + Value: &types.URLConfig{}, }, + want: `{"url":{"url":""}}`, + wantInvalid: true, + }, + "Rejects invalid config": { + config: types.LocationConfig{}, + wantErr: "json: error calling MarshalJSON for type types.LocationConfig: unsupported config type: ", + }, + } { + tt := tt + t.Run(name, func(t *testing.T) { + t.Parallel() + + blob, err := json.Marshal(tt.config) + + if tt.wantErr != "" { + assert.Error(t, err, tt.wantErr) + assert.Assert(t, blob == nil) + return + } + + assert.NilError(t, err) + assert.DeepEqual(t, string(blob), tt.want) + assert.Equal(t, tt.config.Value.Valid(), !tt.wantInvalid) }) - - // Unknown config. - blob = []byte(`{"xxxxxx":{"bucket":"perma-aips-1","region":"eu-west-1"}}`) - cfg = types.LocationConfig{} - err = json.Unmarshal(blob, &cfg) - assert.Error(t, err, "undefined configuration document") - assert.DeepEqual(t, cfg, types.LocationConfig{}) - - // It rejects multiple configs. - blob = []byte(`{ - "s3":{"bucket":"perma-aips-1","region":"eu-west-1"}, - "sftp":{"address":"sftp:22","username":"user","password":"secret","directory":"upload"} - }`) - cfg = types.LocationConfig{} - err = json.Unmarshal(blob, &cfg) - assert.Error(t, err, "multiple config values have been assigned") - assert.DeepEqual(t, cfg, types.LocationConfig{}) - - // It rejects multiple configs (2). - blob = []byte(`{ - "sftp":{"address":"sftp:22","username":"user","password":"secret","directory":"upload"}, - "s3":{"bucket":"perma-aips-1","region":"eu-west-1"} - }`) - cfg = types.LocationConfig{} - err = json.Unmarshal(blob, &cfg) - assert.Error(t, err, "multiple config values have been assigned") - assert.DeepEqual(t, cfg, types.LocationConfig{}) - }) + } } -func TestURLConfig(t *testing.T) { +func TestLocationConfigDecoding(t *testing.T) { t.Parallel() - t.Run("Encodes a URL config", func(t *testing.T) { - t.Parallel() - - cfg := types.LocationConfig{ - Value: &types.URLConfig{ - URL: "mem:///test-bucket", + type test struct { + blob string + want types.LocationConfig + extra func(c types.LocationConfig) + wantErr string + } + for name, tt := range map[string]test{ + "Decodes S3 config": { + blob: `{"s3":{"bucket":"perma-aips-1","region":"eu-west-1"}}`, + want: types.LocationConfig{ + Value: &types.S3Config{ + Bucket: "perma-aips-1", + Region: "eu-west-1", + }, }, - } - blob, err := json.Marshal(cfg) - - assert.NilError(t, err) - assert.Equal(t, string(blob), `{"url":{"url":"mem:///test-bucket"}}`) - assert.Equal(t, cfg.Value.Valid(), true) - }) - - t.Run("Decodes a URL config", func(t *testing.T) { - t.Parallel() - - blob := []byte(`{"url":{"url":"mem:///test-bucket"}}`) - cfg := types.LocationConfig{} - err := json.Unmarshal(blob, &cfg) - - assert.NilError(t, err) - assert.DeepEqual(t, cfg, types.LocationConfig{ - Value: &types.URLConfig{ - URL: "mem:///test-bucket", + }, + "Decodes SFTP config": { + blob: `{"sftp":{"address":"sftp:22","username":"user","password":"secret","directory":"upload"}}`, + want: types.LocationConfig{ + Value: &types.SFTPConfig{ + Address: "sftp:22", + Username: "user", + Password: "secret", + Directory: "upload", + }, + }, + }, + "Decodes URL config": { + blob: `{"url": {"url": "mem://test-bucket"}}`, + want: types.LocationConfig{ + Value: &types.URLConfig{ + URL: "mem://test-bucket", + }, + }, + extra: func(c types.LocationConfig) { + b, err := c.Value.OpenBucket(context.Background()) + assert.NilError(t, err) + defer b.Close() + + y, err := b.IsAccessible(context.Background()) + assert.NilError(t, err) + assert.Equal(t, y, true) + }, + }, + "Rejects URL config if invalid": { + blob: `{"url": {"url": "foo://test-bucket"}}`, + want: types.LocationConfig{ + Value: &types.URLConfig{ + URL: "foo://test-bucket", + }, + }, + extra: func(c types.LocationConfig) { + _, err := c.Value.OpenBucket(context.Background()) + assert.Error(t, err, `open bucket by URL: open blob.Bucket: no driver registered for "foo" for URL "foo://test-bucket"; available schemes: mem, s3, sftp`) }, + }, + "Rejects unknown config": { + blob: `{"xxxxxx":{}}`, + wantErr: "undefined configuration document", + }, + "Rejects unexpected JSON": { + blob: `[1, 2, 3]`, + wantErr: "undefined configuration format", + }, + "Rejects multiple configs (1)": { + blob: `{ + "s3":{"bucket":"perma-aips-1","region":"eu-west-1"}, + "sftp":{"address":"sftp:22","username":"user","password":"secret","directory":"upload"} + }`, + wantErr: "multiple config values have been assigned", + }, + "Rejects multiple configs (2)": { + blob: `{ + "sftp":{"address":"sftp:22","username":"user","password":"secret","directory":"upload"}, + "s3":{"bucket":"perma-aips-1","region":"eu-west-1"} + }`, + wantErr: "multiple config values have been assigned", + }, + } { + tt := tt + t.Run(name, func(t *testing.T) { + t.Parallel() + + cfg := types.LocationConfig{} + err := json.Unmarshal([]byte(tt.blob), &cfg) + + if tt.wantErr != "" { + assert.Error(t, err, tt.wantErr) + assert.DeepEqual(t, cfg, types.LocationConfig{}) + return + } + + assert.NilError(t, err) + assert.DeepEqual(t, cfg, tt.want) + assert.Equal(t, cfg.Value.Valid(), true) + + if tt.extra != nil { + tt.extra(cfg) + } }) - }) - - t.Run("Opens a URL Config bucket", func(t *testing.T) { - t.Parallel() - - ctx := context.Background() - c := types.URLConfig{URL: "mem:///test-bucket"} - - b, err := c.OpenBucket(ctx) - assert.NilError(t, err) - defer b.Close() - - y, err := b.IsAccessible(ctx) - assert.NilError(t, err) - assert.Equal(t, y, true) - }) - - t.Run("Errors if URL Config is invalid", func(t *testing.T) { - t.Parallel() - - ctx := context.Background() - c := types.URLConfig{URL: "foo:///test-bucket"} - _, err := c.OpenBucket(ctx) - assert.ErrorContains(t, err, - `open bucket by URL: open blob.Bucket: no driver registered for "foo"`, - ) - }) + } }