Skip to content

Commit

Permalink
hotfix(state) multiple fixes for ServiceVersion
Browse files Browse the repository at this point in the history 
- Add an option to ignore foreign entities while doing comparison in the
  diff engine
- Index ServiceVersions by package.ID and version name, since version
  name will not be unique globally
  • Loading branch information
@hbagdi
hbagdi committed Mar 6, 2021
1 parent 934e025 commit cf44212
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 60 deletions.
10 changes: 8 additions & 2 deletions state/konnect_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,26 @@ func (s1 *ServiceVersion) Console() string {

// Equal returns true if s1 and s2 are equal.
func (s1 *ServiceVersion) Equal(s2 *ServiceVersion) bool {
return s1.EqualWithOpts(s2, false, false)
return s1.EqualWithOpts(s2, false, false,false)
}

// EqualWithOpts returns true if s1 and s2 are equal.
// If ignoreID is set to true, IDs will be ignored while comparison.
// If ignoreTS is set to true, timestamp fields will be ignored.
func (s1 *ServiceVersion) EqualWithOpts(s2 *ServiceVersion,
ignoreID bool, ignoreTS bool) bool {
ignoreID, ignoreTS, ignoreForeign bool) bool {
s1Copy := s1.ServiceVersion.DeepCopy()
s2Copy := s2.ServiceVersion.DeepCopy()

if ignoreID {
s1Copy.ID = nil
s2Copy.ID = nil
}
if ignoreForeign {
s1Copy.ServicePackage = nil
s1Copy.ControlPlaneServiceRelation = nil
s2Copy.ServicePackage = nil
s2Copy.ControlPlaneServiceRelation = nil
}
return reflect.DeepEqual(s1Copy, s2Copy)
}
90 changes: 54 additions & 36 deletions state/service_version.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,20 @@ package state

import (
"fmt"

"github.com/hashicorp/go-memdb"
"github.com/kong/deck/state/indexers"
"github.com/kong/deck/utils"
"github.com/pkg/errors"
)

const (
serviceVersionTableName = "service-version"
versionsByServicePackageID = "serviceVersionsByServicePackageID"
)

var errInvalidPackage = errors.New("servicePackage.ID is required in ServiceVersion")

var serviceVersionTableSchema = &memdb.TableSchema{
Name: serviceVersionTableName,
Indexes: map[string]*memdb.IndexSchema{
Expand All @@ -20,11 +24,6 @@ var serviceVersionTableSchema = &memdb.TableSchema{
Unique: true,
Indexer: &memdb.StringFieldIndex{Field: "ID"},
},
"version": {
Name: "version",
Unique: true,
Indexer: &memdb.StringFieldIndex{Field: "Version"},
},
all: allIndex,
// foreign
versionsByServicePackageID: {
Expand All @@ -41,6 +40,14 @@ var serviceVersionTableSchema = &memdb.TableSchema{
},
}

func validatePackage(version ServiceVersion) error {
if version.ServicePackage == nil ||
utils.Empty(version.ServicePackage.ID) {
return errInvalidPackage
}
return nil
}

// ServiceVersionsCollection stores and indexes Service Versions.
type ServiceVersionsCollection collection

Expand All @@ -52,6 +59,10 @@ func (k *ServiceVersionsCollection) Add(serviceVersion ServiceVersion) error {
return errIDRequired
}

if err := validatePackage(serviceVersion); err != nil {
return err
}

txn := k.db.Txn(true)
defer txn.Abort()

Expand All @@ -60,7 +71,7 @@ func (k *ServiceVersionsCollection) Add(serviceVersion ServiceVersion) error {
if !utils.Empty(serviceVersion.Version) {
searchBy = append(searchBy, *serviceVersion.Version)
}
_, err := getServiceVersion(txn, searchBy...)
_, err := getServiceVersion(txn, *serviceVersion.ServicePackage.ID, searchBy...)
if err == nil {
return fmt.Errorf("inserting serviceVersion %v: %w", serviceVersion.Console(), ErrAlreadyExists)
} else if err != ErrNotFound {
Expand All @@ -75,35 +86,34 @@ func (k *ServiceVersionsCollection) Add(serviceVersion ServiceVersion) error {
return nil
}

func getServiceVersion(txn *memdb.Txn, IDs ...string) (*ServiceVersion, error) {
for _, id := range IDs {
res, err := multiIndexLookupUsingTxn(txn, serviceVersionTableName,
[]string{"version", "id"}, id)
if err == ErrNotFound {
continue
}
if err != nil {
return nil, err
}
func getServiceVersion(txn *memdb.Txn, packageID string, IDs ...string) (*ServiceVersion, error) {
if packageID == "" {
return nil, errors.New("packageID is required")
}
versions, err := getAllByPackageID(txn, packageID)
if err != nil {
return nil, err
}

serviceVersion, ok := res.(*ServiceVersion)
if !ok {
panic(unexpectedType)
for _, id := range IDs {
for _, version := range versions {
if id == *version.ID || id == *version.Version {
return &ServiceVersion{ServiceVersion: *version.DeepCopy()}, nil
}
}
return &ServiceVersion{ServiceVersion: *serviceVersion.DeepCopy()}, nil
}
return nil, ErrNotFound
}

// Get gets a Service Version by name or ID.
func (k *ServiceVersionsCollection) Get(nameOrID string) (*ServiceVersion, error) {
func (k *ServiceVersionsCollection) Get(packageID, nameOrID string) (*ServiceVersion, error) {
if nameOrID == "" {
return nil, errIDRequired
}

txn := k.db.Txn(false)
defer txn.Abort()
serviceVersion, err := getServiceVersion(txn, nameOrID)
serviceVersion, err := getServiceVersion(txn, packageID, nameOrID)
if err != nil {
return nil, err
}
Expand All @@ -116,11 +126,14 @@ func (k *ServiceVersionsCollection) Update(serviceVersion ServiceVersion) error
if utils.Empty(serviceVersion.ID) {
return errIDRequired
}
if err := validatePackage(serviceVersion); err != nil {
return err
}

txn := k.db.Txn(true)
defer txn.Abort()

err := deleteServiceVersion(txn, *serviceVersion.ID)
err := deleteServiceVersion(txn, *serviceVersion.ServicePackage.ID, *serviceVersion.ID)
if err != nil {
return err
}
Expand All @@ -134,8 +147,8 @@ func (k *ServiceVersionsCollection) Update(serviceVersion ServiceVersion) error
return nil
}

func deleteServiceVersion(txn *memdb.Txn, nameOrID string) error {
serviceVersion, err := getServiceVersion(txn, nameOrID)
func deleteServiceVersion(txn *memdb.Txn, packageID, nameOrID string) error {
serviceVersion, err := getServiceVersion(txn, packageID, nameOrID)
if err != nil {
return err
}
Expand All @@ -148,15 +161,15 @@ func deleteServiceVersion(txn *memdb.Txn, nameOrID string) error {
}

// Delete deletes a serviceVersion by name or ID.
func (k *ServiceVersionsCollection) Delete(nameOrID string) error {
func (k *ServiceVersionsCollection) Delete(packageID, nameOrID string) error {
if nameOrID == "" {
return errIDRequired
}

txn := k.db.Txn(true)
defer txn.Abort()

err := deleteServiceVersion(txn, nameOrID)
err := deleteServiceVersion(txn, packageID, nameOrID)
if err != nil {
return err
}
Expand Down Expand Up @@ -187,21 +200,26 @@ func (k *ServiceVersionsCollection) GetAll() ([]*ServiceVersion, error) {
return res, nil
}

// GetAllByServicePackageID returns all serviceVersions for a servicePackage id.
func (k *ServiceVersionsCollection) GetAllByServicePackageID(id string) ([]*ServiceVersion,
error) {
txn := k.db.Txn(false)
iter, err := txn.Get(serviceVersionTableName, versionsByServicePackageID, id)
func getAllByPackageID(txn *memdb.Txn, packageID string) ([]*ServiceVersion, error) {
iter, err := txn.Get(serviceVersionTableName, versionsByServicePackageID, packageID)
if err != nil {
return nil, err
}
var res []*ServiceVersion

var versions []*ServiceVersion
for el := iter.Next(); el != nil; el = iter.Next() {
r, ok := el.(*ServiceVersion)
v, ok := el.(*ServiceVersion)
if !ok {
panic(unexpectedType)
}
res = append(res, &ServiceVersion{ServiceVersion: *r.DeepCopy()})
versions = append(versions, &ServiceVersion{ServiceVersion: *v.DeepCopy()})
}
return res, nil
return versions, nil
}

// GetAllByServicePackageID returns all serviceVersions for a servicePackage id.
func (k *ServiceVersionsCollection) GetAllByServicePackageID(id string) ([]*ServiceVersion,
error) {
txn := k.db.Txn(false)
return getAllByPackageID(txn, id)
}
49 changes: 27 additions & 22 deletions state/service_version_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,16 @@ func TestServiceVersionCollection_Add(t *testing.T) {

func TestServiceVersionCollection_Get(t *testing.T) {
type args struct {
nameOrID string
nameOrID string
packageID string
}
sv1 := ServiceVersion{
ServiceVersion: konnect.ServiceVersion{
ID: kong.String("foo-id"),
Version: kong.String("foo-name"),
ServicePackage: &konnect.ServicePackage{
ID: kong.String("id1"),
},
},
}
sv2 := ServiceVersion{
Expand All @@ -151,25 +155,27 @@ func TestServiceVersionCollection_Get(t *testing.T) {
}{

{
name: "gets a serviceVersion by ID",
name: "gets a serviceVersion by package and version ID",
args: args{
nameOrID: "foo-id",
nameOrID: "foo-id",
packageID: "id1",
},
want: &sv1,
wantErr: false,
},
{
name: "gets a serviceVersion by Version",
name: "returns an error when only version is specified",
args: args{
nameOrID: "bar-name",
},
want: &sv2,
wantErr: false,
want: nil,
wantErr: true,
},
{
name: "returns an ErrNotFound when no serviceVersion found",
args: args{
nameOrID: "baz-id",
nameOrID: "baz-id",
packageID: "id1",
},
want: nil,
wantErr: true,
Expand All @@ -188,8 +194,7 @@ func TestServiceVersionCollection_Get(t *testing.T) {
k.Add(sv2)
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got, err := k.Get(tt.args.nameOrID)
got, err := k.Get(tt.args.packageID, tt.args.nameOrID)
if (err != nil) != tt.wantErr {
t.Errorf("ServiceVersionCollection.Get() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down Expand Up @@ -233,10 +238,10 @@ func TestServiceVersionCollection_Update(t *testing.T) {
serviceVersion ServiceVersion
}
tests := []struct {
name string
args args
wantErr bool
updatedRoute *ServiceVersion
name string
args args
wantErr bool
updatedVersion *ServiceVersion
}{
{
name: "update errors if serviceVersion.ID is nil",
Expand Down Expand Up @@ -268,8 +273,8 @@ func TestServiceVersionCollection_Update(t *testing.T) {
args: args{
serviceVersion: sv3,
},
wantErr: false,
updatedRoute: &sv3,
wantErr: false,
updatedVersion: &sv3,
},
}
k := serviceVersionCollection()
Expand All @@ -282,10 +287,10 @@ func TestServiceVersionCollection_Update(t *testing.T) {
t.Errorf("ServiceVersionCollection.Update() error = %v, wantErr %v", err, tt.wantErr)
}
if !tt.wantErr {
got, _ := k.Get(*tt.updatedRoute.ID)
got, _ := k.Get(*tt.updatedVersion.ServicePackage.ID, *tt.updatedVersion.ID)

if !reflect.DeepEqual(got, tt.updatedRoute) {
t.Errorf("update serviceVersion, got = %#v, want %#v", got, tt.updatedRoute)
if !reflect.DeepEqual(got, tt.updatedVersion) {
t.Errorf("update serviceVersion, got = %#v, want %#v", got, tt.updatedVersion)
}
}
})
Expand All @@ -300,19 +305,19 @@ func TestServiceVersionDelete(t *testing.T) {
serviceVersion.Version = kong.String("my-serviceVersion")
serviceVersion.ID = kong.String("first")
serviceVersion.ServicePackage = &konnect.ServicePackage{
ID: kong.String("id1"),
ID: kong.String("package-id1"),
}
err := collection.Add(serviceVersion)
assert.Nil(err)

re, err := collection.Get("my-serviceVersion")
re, err := collection.Get("package-id1", "my-serviceVersion")
assert.Nil(err)
assert.NotNil(re)

err = collection.Delete(*re.ID)
err = collection.Delete("package-id1", *re.ID)
assert.Nil(err)

err = collection.Delete(*re.ID)
err = collection.Delete("package-id1", *re.ID)
assert.NotNil(err)
}

Expand Down

0 comments on commit cf44212

Please sign in to comment.