Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug where uninstalling any version disables all bundles of same type #107

Merged
merged 1 commit into from
Sep 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dataaccess/dataaccess.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type DataAccess interface {

BundleCreate(ctx context.Context, bundle data.Bundle) error
BundleDelete(ctx context.Context, name string, version string) error
BundleDisable(ctx context.Context, name string) error
BundleDisable(ctx context.Context, name string, version string) error
BundleEnable(ctx context.Context, name string, version string) error
BundleEnabledVersion(ctx context.Context, name string) (string, error)
BundleExists(ctx context.Context, name string, version string) (bool, error)
Expand Down
6 changes: 3 additions & 3 deletions dataaccess/memory/bundle-access.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,15 @@ func (da *InMemoryDataAccess) BundleDelete(ctx context.Context, name, version st
}

// BundleDisable TBD
func (da *InMemoryDataAccess) BundleDisable(ctx context.Context, name string) error {
func (da *InMemoryDataAccess) BundleDisable(ctx context.Context, name, version string) error {
if name == "" {
return errs.ErrEmptyBundleName
}

foundMatch := false

for n, b := range da.bundles {
if n != name {
for _, b := range da.bundles {
if b.Name != name || b.Version != version {
continue
}

Expand Down
30 changes: 30 additions & 0 deletions dataaccess/memory/bundle-access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/getgort/gort/data"
"github.com/getgort/gort/dataaccess/errs"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func testBundleAccess(t *testing.T) {
Expand All @@ -33,6 +34,7 @@ func testBundleAccess(t *testing.T) {
t.Run("testBundleEnableTwo", testBundleEnableTwo)
t.Run("testBundleExists", testBundleExists)
t.Run("testBundleDelete", testBundleDelete)
t.Run("testBundleDeleteDoesntDisable", testBundleDeleteDoesntDisable)
t.Run("testBundleGet", testBundleGet)
t.Run("testBundleList", testBundleList)
t.Run("testBundleVersionList", testBundleVersionList)
Expand Down Expand Up @@ -268,6 +270,34 @@ func testBundleDelete(t *testing.T) {
}
}

func testBundleDeleteDoesntDisable(t *testing.T) {
var err error

bundle, _ := getTestBundle()
bundle.Name = "test-delete2"
bundle.Version = "0.0.1"
err = da.BundleCreate(ctx, bundle)
require.NoError(t, err)
defer da.BundleDelete(ctx, bundle.Name, bundle.Version)

bundle2, _ := getTestBundle()
bundle2.Name = "test-delete2"
bundle2.Version = "0.0.2"
err = da.BundleCreate(ctx, bundle2)
require.NoError(t, err)
defer da.BundleDelete(ctx, bundle2.Name, bundle2.Version)

err = da.BundleEnable(ctx, bundle2.Name, bundle2.Version)
require.NoError(t, err)

err = da.BundleDelete(ctx, bundle.Name, bundle.Version)
require.NoError(t, err)

bundle2, err = da.BundleGet(ctx, bundle2.Name, bundle2.Version)
require.NoError(t, err)
assert.True(t, bundle2.Enabled)
}

func testBundleGet(t *testing.T) {
var err error

Expand Down
12 changes: 6 additions & 6 deletions dataaccess/postgres/bundle-access.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func (da PostgresDataAccess) BundleDelete(ctx context.Context, name, version str
return errs.ErrNoSuchBundle
}

err = da.doBundleDisable(ctx, tx, name)
err = da.doBundleDisable(ctx, tx, name, version)
if err != nil {
tx.Rollback()
return gerr.Wrap(errs.ErrDataAccess, err)
Expand All @@ -157,7 +157,7 @@ func (da PostgresDataAccess) BundleDelete(ctx context.Context, name, version str
}

// BundleDisable TBD
func (da PostgresDataAccess) BundleDisable(ctx context.Context, name string) error {
func (da PostgresDataAccess) BundleDisable(ctx context.Context, name, version string) error {
tr := otel.GetTracerProvider().Tracer(telemetry.ServiceName)
ctx, sp := tr.Start(ctx, "postgres.BundleDisable")
defer sp.End()
Expand All @@ -173,7 +173,7 @@ func (da PostgresDataAccess) BundleDisable(ctx context.Context, name string) err
return gerr.Wrap(errs.ErrDataAccess, err)
}

err = da.doBundleDisable(ctx, tx, name)
err = da.doBundleDisable(ctx, tx, name, version)
if err != nil {
tx.Rollback()
return gerr.Wrap(errs.ErrDataAccess, err)
Expand Down Expand Up @@ -521,10 +521,10 @@ func (da PostgresDataAccess) doBundleDelete(ctx context.Context, tx *sql.Tx, nam
}

// doBundleDisable TBD
func (da PostgresDataAccess) doBundleDisable(ctx context.Context, tx *sql.Tx, name string) error {
query := `DELETE FROM bundle_enabled WHERE bundle_name=$1`
func (da PostgresDataAccess) doBundleDisable(ctx context.Context, tx *sql.Tx, name string, version string) error {
query := `DELETE FROM bundle_enabled WHERE bundle_name=$1 AND bundle_version=$2`

_, err := tx.ExecContext(ctx, query, name)
_, err := tx.ExecContext(ctx, query, name, version)
if err != nil {
return gerr.Wrap(errs.ErrDataAccess, err)
}
Expand Down
30 changes: 30 additions & 0 deletions dataaccess/postgres/bundle-access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/getgort/gort/data"
"github.com/getgort/gort/dataaccess/errs"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func testBundleAccess(t *testing.T) {
Expand All @@ -33,6 +34,7 @@ func testBundleAccess(t *testing.T) {
t.Run("testBundleEnableTwo", testBundleEnableTwo)
t.Run("testBundleExists", testBundleExists)
t.Run("testBundleDelete", testBundleDelete)
t.Run("testBundleDeleteDoesntDisable", testBundleDeleteDoesntDisable)
t.Run("testBundleGet", testBundleGet)
t.Run("testBundleList", testBundleList)
t.Run("testBundleVersionList", testBundleVersionList)
Expand Down Expand Up @@ -268,6 +270,34 @@ func testBundleDelete(t *testing.T) {
}
}

func testBundleDeleteDoesntDisable(t *testing.T) {
var err error

bundle, _ := getTestBundle()
bundle.Name = "test-delete2"
bundle.Version = "0.0.1"
err = da.BundleCreate(ctx, bundle)
require.NoError(t, err)
defer da.BundleDelete(ctx, bundle.Name, bundle.Version)

bundle2, _ := getTestBundle()
bundle2.Name = "test-delete2"
bundle2.Version = "0.0.2"
err = da.BundleCreate(ctx, bundle2)
require.NoError(t, err)
defer da.BundleDelete(ctx, bundle2.Name, bundle2.Version)

err = da.BundleEnable(ctx, bundle2.Name, bundle2.Version)
require.NoError(t, err)

err = da.BundleDelete(ctx, bundle.Name, bundle.Version)
require.NoError(t, err)

bundle2, err = da.BundleGet(ctx, bundle2.Name, bundle2.Version)
require.NoError(t, err)
assert.True(t, bundle2.Enabled)
}

func testBundleGet(t *testing.T) {
var err error

Expand Down
2 changes: 1 addition & 1 deletion service/bundle-handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func handlePatchBundleVersion(w http.ResponseWriter, r *http.Request) {
if enabledValue[0] == 'T' {
err = dataAccessLayer.BundleEnable(r.Context(), name, version)
} else if enabledValue[0] == 'F' {
err = dataAccessLayer.BundleDisable(r.Context(), name)
err = dataAccessLayer.BundleDisable(r.Context(), name, version)
}
if err != nil {
respondAndLogError(r.Context(), w, err)
Expand Down