From b4699e7598af91017c21b6d7ba41c720654c43d5 Mon Sep 17 00:00:00 2001 From: Vadim Voitenko Date: Mon, 18 Mar 2024 16:51:16 +0200 Subject: [PATCH] Fixed delete operation * Implemented adhoc Walk function * Removed artifacts * Added DeleteAll method for Storager interface * Added DeleteAll integration test for S3 storage * Renamed integration tests package --- .../cmd/delete_backup/delete_dump.go | 21 ++---- internal/storages/directory/directory.go | 19 ++++++ internal/storages/s3/s3.go | 27 +++++++- internal/storages/storager.go | 2 + internal/storages/utils.go | 31 +++++++++ tests/demodb/dump.sql | 12 ---- tests/integration/{toc => greenmask}/args.go | 2 +- .../backward_compatibility_test.go | 2 +- .../{toc => greenmask}/main_test.go | 2 +- .../{toc => greenmask}/toc_readwriter_test.go | 2 +- tests/integration/storages/s3_test.go | 46 +++++++++++++ tests/migration/test.sql | 66 ------------------- 12 files changed, 132 insertions(+), 100 deletions(-) create mode 100644 internal/storages/utils.go delete mode 100644 tests/demodb/dump.sql rename tests/integration/{toc => greenmask}/args.go (99%) rename tests/integration/{toc => greenmask}/backward_compatibility_test.go (99%) rename tests/integration/{toc => greenmask}/main_test.go (97%) rename tests/integration/{toc => greenmask}/toc_readwriter_test.go (99%) delete mode 100644 tests/migration/test.sql diff --git a/cmd/greenmask/cmd/delete_backup/delete_dump.go b/cmd/greenmask/cmd/delete_backup/delete_dump.go index c49c1504..b78d8f3c 100644 --- a/cmd/greenmask/cmd/delete_backup/delete_dump.go +++ b/cmd/greenmask/cmd/delete_backup/delete_dump.go @@ -17,6 +17,7 @@ package delete_backup import ( "context" "fmt" + "slices" "github.com/greenmaskio/greenmask/internal/storages" "github.com/rs/zerolog/log" @@ -59,24 +60,14 @@ func deleteDump(dumpId string) error { log.Fatal().Err(err).Msg("") } - var found bool - var backupDirStorage storages.Storager - for _, b := range dirs { - if dumpId == b.Dirname() { - found = true - backupDirStorage = b - } - } - - if !found { + if !slices.ContainsFunc(dirs, func(sst storages.Storager) bool { + return dumpId == sst.Dirname() + }) { return fmt.Errorf("dump with id %s was not found", dumpId) } - files, _, err := backupDirStorage.ListDir(ctx) - for _, f := range files { - if err = backupDirStorage.Delete(ctx, f); err != nil { - return fmt.Errorf("storage error: %s", err) - } + if err = st.DeleteAll(ctx, dumpId); err != nil { + return fmt.Errorf("storage error: %s", err) } return nil diff --git a/internal/storages/directory/directory.go b/internal/storages/directory/directory.go index 41e37498..9e263b67 100644 --- a/internal/storages/directory/directory.go +++ b/internal/storages/directory/directory.go @@ -147,6 +147,25 @@ func (d *Storage) Delete(ctx context.Context, filePaths ...string) error { return nil } +func (d *Storage) DeleteAll(ctx context.Context, pathPrefix string) error { + fileInfo, err := os.Stat(path.Join(d.cwd, pathPrefix)) + if err != nil { + return err + } + if fileInfo.IsDir() { + err = os.RemoveAll(path.Join(d.cwd, pathPrefix)) + if err != nil { + return fmt.Errorf(`error deliting directory %s: %w`, pathPrefix, err) + } + } else { + err = os.Remove(path.Join(d.cwd, pathPrefix)) + if err != nil { + return fmt.Errorf(`error deliting file %s: %w`, pathPrefix, err) + } + } + return nil +} + func (d *Storage) Exists(ctx context.Context, fileName string) (bool, error) { _, err := os.Stat(path.Join(d.cwd, fileName)) if err != nil { diff --git a/internal/storages/s3/s3.go b/internal/storages/s3/s3.go index 5412511d..091eb9fd 100644 --- a/internal/storages/s3/s3.go +++ b/internal/storages/s3/s3.go @@ -148,7 +148,7 @@ func NewStorage(ctx context.Context, cfg *Config, logLevel string) (*Storage, er ) return &Storage{ - prefix: path.Join(cfg.Bucket, cfg.Prefix) + "/", + prefix: fixPrefix(path.Join(cfg.Bucket, cfg.Prefix)), session: ses, config: cfg, service: service, @@ -175,7 +175,7 @@ func (s *Storage) ListDir(ctx context.Context) (files []string, dirs []storages. session: s.session, service: s.service, uploader: s.uploader, - prefix: *prefix.Prefix, + prefix: fixPrefix(*prefix.Prefix), }, ) } @@ -272,10 +272,24 @@ func (s *Storage) Delete(ctx context.Context, filePaths ...string) error { return nil } +func (s *Storage) DeleteAll(ctx context.Context, pathPrefix string) error { + pathPrefix = fixPrefix(pathPrefix) + ss := s.SubStorage(pathPrefix, true) + filesList, err := storages.Walk(ctx, ss, "") + if err != nil { + return fmt.Errorf("error walking through storage: %w", err) + } + + if err = ss.Delete(ctx, filesList...); err != nil { + return fmt.Errorf("error deleting files: %w", err) + } + return nil +} + func (s *Storage) SubStorage(subPath string, relative bool) storages.Storager { prefix := subPath if relative { - prefix = path.Join(s.prefix, prefix) + prefix = fixPrefix(path.Join(s.prefix, prefix)) } return &Storage{ config: s.config, @@ -303,3 +317,10 @@ func (s *Storage) Exists(ctx context.Context, fileName string) (bool, error) { } return true, nil } + +func fixPrefix(prefix string) string { + if prefix != "" && prefix[len(prefix)-1] != '/' { + prefix = prefix + "/" + } + return prefix +} diff --git a/internal/storages/storager.go b/internal/storages/storager.go index 52353616..d19e592f 100644 --- a/internal/storages/storager.go +++ b/internal/storages/storager.go @@ -32,6 +32,8 @@ type Storager interface { PutObject(ctx context.Context, filePath string, body io.Reader) error // Delete - delete list of objects by the provided paths Delete(ctx context.Context, filePaths ...string) error + // DeleteAll - delete all objects by the provided path prefix + DeleteAll(ctx context.Context, pathPrefix string) error // Exists - check object existence Exists(ctx context.Context, fileName string) (bool, error) // SubStorage - get new Storage instance with the samo config but change current cwd via subPath diff --git a/internal/storages/utils.go b/internal/storages/utils.go new file mode 100644 index 00000000..d9228534 --- /dev/null +++ b/internal/storages/utils.go @@ -0,0 +1,31 @@ +package storages + +import ( + "context" + "fmt" + "path" +) + +func Walk(ctx context.Context, st Storager, parent string) (res []string, err error) { + var files []string + files, dirs, err := st.ListDir(ctx) + if err != nil { + return nil, fmt.Errorf("error listing directory: %w", err) + } + for _, f := range files { + res = append(res, path.Join(parent, f)) + } + if len(dirs) > 0 { + for _, d := range dirs { + subFiles, err := Walk(ctx, d, d.Dirname()) + if err != nil { + return nil, fmt.Errorf("error walking through directory: %w", err) + } + for _, f := range subFiles { + res = append(res, path.Join(parent, f)) + } + } + } + + return +} diff --git a/tests/demodb/dump.sql b/tests/demodb/dump.sql deleted file mode 100644 index 74db3e10..00000000 --- a/tests/demodb/dump.sql +++ /dev/null @@ -1,12 +0,0 @@ -create table public.flights -( - id INT PRIMARY KEY, - flight_no TEXT NOT NULL UNIQUE, - departure TIMESTAMPTZ NOT NULL, - arrival TIMESTAMPTZ NOT NULL CHECK ( arrival > departure ) -); - - -INSERT INTO public.flights (id, flight_no, departure, arrival) -SELECT generate_series, format('ABCD%s', generate_series), now() - INTERVAL '1 hour', now() -FROM generate_series(1, 100); diff --git a/tests/integration/toc/args.go b/tests/integration/greenmask/args.go similarity index 99% rename from tests/integration/toc/args.go rename to tests/integration/greenmask/args.go index 5f6c9243..ae9ee12d 100644 --- a/tests/integration/toc/args.go +++ b/tests/integration/greenmask/args.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package toc +package greenmask import ( "flag" diff --git a/tests/integration/toc/backward_compatibility_test.go b/tests/integration/greenmask/backward_compatibility_test.go similarity index 99% rename from tests/integration/toc/backward_compatibility_test.go rename to tests/integration/greenmask/backward_compatibility_test.go index 9db86668..f9e4eae3 100644 --- a/tests/integration/toc/backward_compatibility_test.go +++ b/tests/integration/greenmask/backward_compatibility_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package toc +package greenmask import ( "context" diff --git a/tests/integration/toc/main_test.go b/tests/integration/greenmask/main_test.go similarity index 97% rename from tests/integration/toc/main_test.go rename to tests/integration/greenmask/main_test.go index 307495a9..da76bbd2 100644 --- a/tests/integration/toc/main_test.go +++ b/tests/integration/greenmask/main_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package toc +package greenmask import ( "testing" diff --git a/tests/integration/toc/toc_readwriter_test.go b/tests/integration/greenmask/toc_readwriter_test.go similarity index 99% rename from tests/integration/toc/toc_readwriter_test.go rename to tests/integration/greenmask/toc_readwriter_test.go index 7a7f3d72..20f7da36 100644 --- a/tests/integration/toc/toc_readwriter_test.go +++ b/tests/integration/greenmask/toc_readwriter_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package toc +package greenmask import ( "errors" diff --git a/tests/integration/storages/s3_test.go b/tests/integration/storages/s3_test.go index b9afe2ad..987a8c14 100644 --- a/tests/integration/storages/s3_test.go +++ b/tests/integration/storages/s3_test.go @@ -19,7 +19,9 @@ import ( "context" "io" "path" + "slices" + "github.com/greenmaskio/greenmask/internal/storages" "github.com/rs/zerolog" "github.com/stretchr/testify/suite" @@ -115,4 +117,48 @@ func (suite *S3StorageSuite) TestS3Ops() { suite.Require().NotContains(files, "test_to_del.txt") }) + suite.Run("delete_all", func() { + buf := bytes.NewBuffer([]byte("1234567890")) + err := suite.st.PutObject(context.Background(), "/test_to_del.txt", buf) + suite.Require().NoError(err) + + buf = bytes.NewBuffer([]byte("1234567890")) + err = suite.st.PutObject(context.Background(), "/dir1/test_to_del2.txt", buf) + suite.Require().NoError(err) + + buf = bytes.NewBuffer([]byte("1234567890")) + err = suite.st.PutObject(context.Background(), "/dir1/subdir2/test_to_del3.txt", buf) + suite.Require().NoError(err) + + files, dirs, err := suite.st.ListDir(context.Background()) + suite.Require().NoError(err) + suite.Require().Contains(files, "test_to_del.txt") + idx := slices.IndexFunc(dirs, func(s storages.Storager) bool { + return s.Dirname() == "dir1" + }) + suite.Require().True(idx != -1) + + files, dirs, err = dirs[idx].ListDir(context.Background()) + suite.Require().NoError(err) + suite.Require().Contains(files, "test_to_del2.txt") + suite.Require().Len(dirs, 1) + idx = slices.IndexFunc(dirs, func(s storages.Storager) bool { + return s.Dirname() == "subdir2" + }) + suite.Require().True(idx != -1) + + files, dirs, err = dirs[idx].ListDir(context.Background()) + suite.Require().NoError(err) + suite.Require().Contains(files, "test_to_del3.txt") + suite.Require().Empty(dirs) + + err = suite.st.DeleteAll(context.Background(), "/") + suite.Require().NoError(err) + + files, dirs, err = suite.st.ListDir(context.Background()) + suite.Require().NoError(err) + suite.Require().NotContains(files, "test_to_del.txt") + suite.Require().Empty(dirs) + }) + } diff --git a/tests/migration/test.sql b/tests/migration/test.sql deleted file mode 100644 index 92449d27..00000000 --- a/tests/migration/test.sql +++ /dev/null @@ -1,66 +0,0 @@ --- Copyright 2023 Greenmask --- --- Licensed under the Apache License, Version 2.0 (the "License"); --- you may not use this file except in compliance with the License. --- You may obtain a copy of the License at --- --- http://www.apache.org/licenses/LICENSE-2.0 --- --- Unless required by applicable law or agreed to in writing, software --- distributed under the License is distributed on an "AS IS" BASIS, --- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. --- See the License for the specific language governing permissions and --- limitations under the License. - -create schema typetest; - -create table typetest.test_bool -( - id SERIAL PRIMARY KEY, - at1 BOOLEAN DEFAULT TRUE, - at2 BOOLEAN NOT NULL DEFAULT FALSE -); - -INSERT INTO typetest.test_bool -SELECT -FROM generate_series(1, 100000); - - -create table typetest.test_int -( - id SERIAL PRIMARY KEY, - at1 INT2 DEFAULT 1, - at2 INT4 DEFAULT 2, - at3 INT8 DEFAULT 3, - at4 INT2 NOT NULL DEFAULT 4, - at5 INT4 NOT NULL DEFAULT 5, - at6 INT8 NOT NULL DEFAULT 6 -); - -INSERT INTO typetest.test_int -SELECT -FROM generate_series(1, 100000); - - -CREATE DOMAIN us_postal_code AS TEXT - CHECK ( - VALUE ~ '^\d{5}$' - OR VALUE ~ '^\d{5}-\d{4}$' - ); -CREATE DOMAIN us_postal_code_v2 AS us_postal_code; - - -ALTER TABLE bookings.flights ADD COLUMN post_code us_postal_code_v2 DEFAULT '12345' NOT NULL ; - -CREATE DOMAIN int_dom AS INT - CHECK ( VALUE > 10 AND VALUE < 100); - -CREATE DOMAIN int_dom_v2 AS int_dom; - -ALTER TABLE bookings.flights ADD COLUMN test_dom int_dom_v2 DEFAULT 11 NOT NULL ; - -select * from bookings.flights; - - -DROP DATABASE demo_restore; -CREATE DATABASE demo_restore;