Skip to content

Commit a816903

Browse files
benluddytimflannagan
benluddy
authored and
timflannagan
committed
Return empty properties and dependencies in ListBundles responses.
Bundles that don't have any properties/dependencies should return nil or [] for those fields, but due to an error in handling NULL column values, a single zero-valued entry was being returned. Upstream-repository: operator-registry Upstream-commit: 00b48a4c4bbe75582b08623c0cc758f0e79b626f
1 parent 3774ef8 commit a816903

File tree

5 files changed

+720
-64
lines changed

5 files changed

+720
-64
lines changed

‎staging/operator-registry/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ require (
4747
golang.org/x/mod v0.2.0
4848
golang.org/x/net v0.0.0-20200625001655-4c5254603344
4949
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45
50+
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e
5051
golang.org/x/sys v0.0.0-20200625212154-ddb9806d33ae // indirect
5152
golang.org/x/text v0.3.3 // indirect
5253
google.golang.org/genproto v0.0.0-20200701001935-0939c5918c31 // indirect

‎staging/operator-registry/pkg/sqlite/query.go

Lines changed: 83 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -o sqlitefakes/fake_rowscanner.go . RowScanner
2+
//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -o sqlitefakes/fake_querier.go . Querier
13
package sqlite
24

35
import (
@@ -13,10 +15,28 @@ import (
1315
"github.com/operator-framework/operator-registry/pkg/registry"
1416
)
1517

16-
type SQLQuerier struct {
18+
type RowScanner interface {
19+
Next() bool
20+
Close() error
21+
Scan(dest ...interface{}) error
22+
}
23+
24+
type Querier interface {
25+
QueryContext(ctx context.Context, query string, args ...interface{}) (RowScanner, error)
26+
}
27+
28+
type dbQuerierAdapter struct {
1729
db *sql.DB
1830
}
1931

32+
func (a dbQuerierAdapter) QueryContext(ctx context.Context, query string, args ...interface{}) (RowScanner, error) {
33+
return a.db.QueryContext(ctx, query, args...)
34+
}
35+
36+
type SQLQuerier struct {
37+
db Querier
38+
}
39+
2040
var _ registry.Query = &SQLQuerier{}
2141

2242
func NewSQLLiteQuerier(dbFilename string) (*SQLQuerier, error) {
@@ -25,11 +45,15 @@ func NewSQLLiteQuerier(dbFilename string) (*SQLQuerier, error) {
2545
return nil, err
2646
}
2747

28-
return &SQLQuerier{db}, nil
48+
return &SQLQuerier{dbQuerierAdapter{db}}, nil
2949
}
3050

3151
func NewSQLLiteQuerierFromDb(db *sql.DB) *SQLQuerier {
32-
return &SQLQuerier{db}
52+
return &SQLQuerier{dbQuerierAdapter{db}}
53+
}
54+
55+
func NewSQLLiteQuerierFromDBQuerier(q Querier) *SQLQuerier {
56+
return &SQLQuerier{q}
3357
}
3458

3559
func (s *SQLQuerier) ListTables(ctx context.Context) ([]string, error) {
@@ -900,7 +924,7 @@ func (s *SQLQuerier) GetCurrentCSVNameForChannel(ctx context.Context, pkgName, c
900924
return "", nil
901925
}
902926

903-
func (s *SQLQuerier) ListBundles(ctx context.Context) (bundles []*api.Bundle, err error) {
927+
func (s *SQLQuerier) ListBundles(ctx context.Context) ([]*api.Bundle, error) {
904928
query := `SELECT DISTINCT channel_entry.entry_id, operatorbundle.bundle, operatorbundle.bundlepath,
905929
channel_entry.operatorbundle_name, channel_entry.package_name, channel_entry.channel_name, operatorbundle.replaces, operatorbundle.skips,
906930
operatorbundle.version, operatorbundle.skiprange,
@@ -918,23 +942,25 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) (bundles []*api.Bundle, er
918942
}
919943
defer rows.Close()
920944

921-
bundles = []*api.Bundle{}
945+
var bundles []*api.Bundle
922946
bundlesMap := map[string]*api.Bundle{}
923947
for rows.Next() {
924-
var entryID sql.NullInt64
925-
var bundle sql.NullString
926-
var bundlePath sql.NullString
927-
var bundleName sql.NullString
928-
var pkgName sql.NullString
929-
var channelName sql.NullString
930-
var replaces sql.NullString
931-
var skips sql.NullString
932-
var version sql.NullString
933-
var skipRange sql.NullString
934-
var depType sql.NullString
935-
var depValue sql.NullString
936-
var propType sql.NullString
937-
var propValue sql.NullString
948+
var (
949+
entryID sql.NullInt64
950+
bundle sql.NullString
951+
bundlePath sql.NullString
952+
bundleName sql.NullString
953+
pkgName sql.NullString
954+
channelName sql.NullString
955+
replaces sql.NullString
956+
skips sql.NullString
957+
version sql.NullString
958+
skipRange sql.NullString
959+
depType sql.NullString
960+
depValue sql.NullString
961+
propType sql.NullString
962+
propValue sql.NullString
963+
)
938964
if err := rows.Scan(&entryID, &bundle, &bundlePath, &bundleName, &pkgName, &channelName, &replaces, &skips, &version, &skipRange, &depType, &depValue, &propType, &propValue); err != nil {
939965
return nil, err
940966
}
@@ -946,29 +972,18 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) (bundles []*api.Bundle, er
946972
bundleKey := fmt.Sprintf("%s/%s/%s/%s", bundleName.String, version.String, bundlePath.String, channelName.String)
947973
bundleItem, ok := bundlesMap[bundleKey]
948974
if ok {
949-
// Create new dependency object
950975
if depType.Valid && depValue.Valid {
951-
dep := &api.Dependency{}
952-
dep.Type = depType.String
953-
dep.Value = depValue.String
954-
955-
// Add new dependency to the existing list
956-
existingDeps := bundleItem.Dependencies
957-
existingDeps = append(existingDeps, dep)
958-
bundleItem.Dependencies = existingDeps
976+
bundleItem.Dependencies = append(bundleItem.Dependencies, &api.Dependency{
977+
Type: depType.String,
978+
Value: depValue.String,
979+
})
959980
}
960981

961-
962-
// Create new property object
963982
if propType.Valid && propValue.Valid {
964-
prop := &api.Property{}
965-
prop.Type = propType.String
966-
prop.Value = propValue.String
967-
968-
// Add new property to the existing list
969-
existingProps := bundleItem.Properties
970-
existingProps = append(existingProps, prop)
971-
bundleItem.Properties = existingProps
983+
bundleItem.Properties = append(bundleItem.Properties, &api.Property{
984+
Type: propType.String,
985+
Value: propValue.String,
986+
})
972987
}
973988
} else {
974989
// Create new bundle
@@ -987,30 +1002,34 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) (bundles []*api.Bundle, er
9871002
out.Version = version.String
9881003
out.SkipRange = skipRange.String
9891004
out.Replaces = replaces.String
990-
out.Skips = strings.Split(skips.String, ",")
1005+
if skips.Valid {
1006+
out.Skips = strings.Split(skips.String, ",")
1007+
}
9911008

9921009
provided, required, err := s.GetApisForEntry(ctx, entryID.Int64)
9931010
if err != nil {
9941011
return nil, err
9951012
}
996-
out.ProvidedApis = provided
997-
out.RequiredApis = required
998-
999-
// Create new dependency and dependency list
1000-
dep := &api.Dependency{}
1001-
dependencies := []*api.Dependency{}
1002-
dep.Type = depType.String
1003-
dep.Value = depValue.String
1004-
dependencies = append(dependencies, dep)
1005-
out.Dependencies = dependencies
1006-
1007-
// Create new property and property list
1008-
prop := &api.Property{}
1009-
properties := []*api.Property{}
1010-
prop.Type = propType.String
1011-
prop.Value = propValue.String
1012-
properties = append(properties, prop)
1013-
out.Properties = properties
1013+
if len(provided) > 0 {
1014+
out.ProvidedApis = provided
1015+
}
1016+
if len(required) > 0 {
1017+
out.RequiredApis = required
1018+
}
1019+
1020+
if depType.Valid && depValue.Valid {
1021+
out.Dependencies = []*api.Dependency{{
1022+
Type: depType.String,
1023+
Value: depValue.String,
1024+
}}
1025+
}
1026+
1027+
if propType.Valid && propValue.Valid {
1028+
out.Properties = []*api.Property{{
1029+
Type: propType.String,
1030+
Value: propValue.String,
1031+
}}
1032+
}
10141033

10151034
bundlesMap[bundleKey] = out
10161035
}
@@ -1028,29 +1047,29 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) (bundles []*api.Bundle, er
10281047
bundles = append(bundles, v)
10291048
}
10301049

1031-
return
1050+
return bundles, nil
10321051
}
10331052

10341053
func unique(deps []*api.Dependency) []*api.Dependency {
1035-
keys := make(map[string]bool)
1036-
list := []*api.Dependency{}
1054+
keys := make(map[string]struct{})
1055+
var list []*api.Dependency
10371056
for _, entry := range deps {
10381057
depKey := fmt.Sprintf("%s/%s", entry.Type, entry.Value)
10391058
if _, value := keys[depKey]; !value {
1040-
keys[depKey] = true
1059+
keys[depKey] = struct{}{}
10411060
list = append(list, entry)
10421061
}
10431062
}
10441063
return list
10451064
}
10461065

10471066
func uniqueProps(props []*api.Property) []*api.Property {
1048-
keys := make(map[string]bool)
1049-
list := []*api.Property{}
1067+
keys := make(map[string]struct{})
1068+
var list []*api.Property
10501069
for _, entry := range props {
10511070
propKey := fmt.Sprintf("%s/%s", entry.Type, entry.Value)
10521071
if _, value := keys[propKey]; !value {
1053-
keys[propKey] = true
1072+
keys[propKey] = struct{}{}
10541073
list = append(list, entry)
10551074
}
10561075
}

0 commit comments

Comments
 (0)