From 5052aa1c125cc52ecf3ecbbc20fafa59451e8f52 Mon Sep 17 00:00:00 2001 From: Stefan Benz <46600784+stebenz@users.noreply.github.com> Date: Thu, 8 Sep 2022 08:39:38 +0100 Subject: [PATCH] fix(assets): correct type column in assets (#4295) * fix(asssets): correct remove asset objects with text column * fix(assets): type asset_type, correct and add unit tests * fix(assets): set unspecified objecttype to empty string Co-authored-by: Livio Spring --- internal/static/database/crdb.go | 4 +- internal/static/database/crdb_test.go | 116 +++++++++++++++++++++++++- internal/static/storage.go | 13 ++- 3 files changed, 127 insertions(+), 6 deletions(-) diff --git a/internal/static/database/crdb.go b/internal/static/database/crdb.go index 4fde9781fc..e03be31145 100644 --- a/internal/static/database/crdb.go +++ b/internal/static/database/crdb.go @@ -44,7 +44,7 @@ func (c *crdbStorage) PutObject(ctx context.Context, instanceID, location, resou } stmt, args, err := squirrel.Insert(assetsTable). Columns(AssetColInstanceID, AssetColResourceOwner, AssetColName, AssetColType, AssetColContentType, AssetColData, AssetColUpdatedAt). - Values(instanceID, resourceOwner, name, objectType, contentType, data, "now()"). + Values(instanceID, resourceOwner, name, objectType.String(), contentType, data, "now()"). Suffix(fmt.Sprintf( "ON CONFLICT (%s, %s, %s) DO UPDATE"+ " SET %s = $5, %s = $6"+ @@ -167,7 +167,7 @@ func (c *crdbStorage) RemoveObjects(ctx context.Context, instanceID, resourceOwn Where(squirrel.Eq{ AssetColInstanceID: instanceID, AssetColResourceOwner: resourceOwner, - AssetColType: objectType, + AssetColType: objectType.String(), }). PlaceholderFormat(squirrel.Dollar). ToSql() diff --git a/internal/static/database/crdb_test.go b/internal/static/database/crdb_test.go index 2a5735e1e0..49cb5c029c 100644 --- a/internal/static/database/crdb_test.go +++ b/internal/static/database/crdb_test.go @@ -21,12 +21,20 @@ var ( ) const ( - objectStmt = "INSERT INTO system.assets" + + createObjectStmt = "INSERT INTO system.assets" + " (instance_id,resource_owner,name,asset_type,content_type,data,updated_at)" + " VALUES ($1,$2,$3,$4,$5,$6,$7)" + " ON CONFLICT (instance_id, resource_owner, name) DO UPDATE SET" + " content_type = $5, data = $6" + " RETURNING hash" + removeObjectStmt = "DELETE FROM system.assets" + + " WHERE instance_id = $1" + + " AND name = $2" + + " AND resource_owner = $3" + removeObjectsStmt = "DELETE FROM system.assets" + + " WHERE asset_type = $1" + + " AND instance_id = $2" + + " AND resource_owner = $3" ) func Test_crdbStorage_CreateObject(t *testing.T) { @@ -56,7 +64,7 @@ func Test_crdbStorage_CreateObject(t *testing.T) { fields{ client: prepareDB(t, expectQuery( - objectStmt, + createObjectStmt, []string{ "hash", "updated_at", @@ -70,7 +78,7 @@ func Test_crdbStorage_CreateObject(t *testing.T) { "instanceID", "resourceOwner", "name", - static.ObjectTypeUserAvatar, + static.ObjectTypeUserAvatar.String(), "contentType", []byte("test"), "now()", @@ -83,6 +91,7 @@ func Test_crdbStorage_CreateObject(t *testing.T) { resourceOwner: "resourceOwner", name: "name", contentType: "contentType", + objectType: static.ObjectTypeUserAvatar, data: bytes.NewReader([]byte("test")), objectSize: 4, }, @@ -115,6 +124,107 @@ func Test_crdbStorage_CreateObject(t *testing.T) { } } +func Test_crdbStorage_RemoveObject(t *testing.T) { + type fields struct { + client db + } + type args struct { + ctx context.Context + instanceID string + resourceOwner string + name string + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + { + "remove ok", + fields{ + client: prepareDB(t, + expectExec( + removeObjectStmt, + nil, + "instanceID", + "name", + "resourceOwner", + )), + }, + args{ + ctx: context.Background(), + instanceID: "instanceID", + resourceOwner: "resourceOwner", + name: "name", + }, + false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &crdbStorage{ + client: tt.fields.client.db, + } + err := c.RemoveObject(tt.args.ctx, tt.args.instanceID, tt.args.resourceOwner, tt.args.name) + if (err != nil) != tt.wantErr { + t.Errorf("RemoveObject() error = %v, wantErr %v", err, tt.wantErr) + return + } + }) + } +} + +func Test_crdbStorage_RemoveObjects(t *testing.T) { + type fields struct { + client db + } + type args struct { + ctx context.Context + instanceID string + resourceOwner string + objectType static.ObjectType + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + { + "remove ok", + fields{ + client: prepareDB(t, + expectExec( + removeObjectsStmt, + nil, static.ObjectTypeUserAvatar.String(), + "instanceID", + "resourceOwner", + )), + }, + args{ + ctx: context.Background(), + instanceID: "instanceID", + resourceOwner: "resourceOwner", + objectType: static.ObjectTypeUserAvatar, + }, + false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &crdbStorage{ + client: tt.fields.client.db, + } + err := c.RemoveObjects(tt.args.ctx, tt.args.instanceID, tt.args.resourceOwner, tt.args.objectType) + if (err != nil) != tt.wantErr { + t.Errorf("RemoveObjects() error = %v, wantErr %v", err, tt.wantErr) + return + } + }) + } +} + type db struct { mock sqlmock.Sqlmock db *sql.DB diff --git a/internal/static/storage.go b/internal/static/storage.go index ba42575edb..1a39b4455e 100644 --- a/internal/static/storage.go +++ b/internal/static/storage.go @@ -21,10 +21,21 @@ type Storage interface { type ObjectType int32 const ( - ObjectTypeUserAvatar = iota + ObjectTypeUserAvatar ObjectType = iota ObjectTypeStyling ) +func (o ObjectType) String() string { + switch o { + case ObjectTypeUserAvatar: + return "0" + case ObjectTypeStyling: + return "1" + default: + return "" + } +} + type Asset struct { InstanceID string ResourceOwner string