test: fix failing master pipeline (#3283)

ory · Oct 6, 2022 · f979adb · f979adb
1 parent 1d13be6
commit f979adb
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 44 deletions.
diff --git a/driver/config/provider.go b/driver/config/provider.go
@@ -75,7 +75,7 @@ const (
 	KeyAdminURL                                  = "urls.self.admin"
 	KeyIssuerURL                                 = "urls.self.issuer"
 	KeyAccessTokenStrategy                       = "strategies.access_token"
-	KeyDbIgnoreUnknownTableColumns               = "db.ignore_unknown_table_columns"
+	KeyDBIgnoreUnknownTableColumns               = "db.ignore_unknown_table_columns"
 	KeySubjectIdentifierAlgorithmSalt            = "oidc.subject_identifiers.pairwise.salt"
 	KeyPublicAllowDynamicRegistration            = "oidc.dynamic_client_registration.enabled"
 	KeyPKCEEnforced                              = "oauth2.pkce.enforced"
@@ -420,7 +420,7 @@ func (p *DefaultProvider) TokenRefreshHookURL(ctx context.Context) *url.URL {
 }
 
 func (p *DefaultProvider) DbIgnoreUnknownTableColumns() bool {
-	return p.p.Bool(KeyDbIgnoreUnknownTableColumns)
+	return p.p.Bool(KeyDBIgnoreUnknownTableColumns)
 }
 
 func (p *DefaultProvider) SubjectIdentifierAlgorithmSalt(ctx context.Context) string {

diff --git a/driver/registry_sql_test.go b/driver/registry_sql_test.go
@@ -6,6 +6,8 @@ import (
 	"strconv"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/stretchr/testify/assert"
 
 	"github.com/ory/hydra/client"
@@ -35,51 +37,41 @@ func TestDefaultKeyManager_HsmDisabled(t *testing.T) {
 }
 
 func TestDbUnknownTableColumns(t *testing.T) {
-	tests := []struct {
-		name         string
-		flagValue    string
-		expectError  bool
-		expectedSize int
-	}{
-		{name: "with unsafe", flagValue: "true", expectError: false, expectedSize: 1},
-		{name: "without unsafe", flagValue: "false", expectError: true, expectedSize: 0},
-	}
+	ctx := context.Background()
+	l := logrusx.New("", "")
+	c := config.MustNew(ctx, l, configx.SkipValidation())
+	postgresDsn := dockertest.RunTestPostgreSQL(t)
+	c.MustSet(ctx, config.KeyDSN, postgresDsn)
+	reg, err := NewRegistryFromDSN(ctx, c, l, false, true, &contextx.Default{})
+	require.NoError(t, err)
 
-	for _, test := range tests {
-		t.Run(
-			test.name, func(t *testing.T) {
-				ctx := context.Background()
-				l := logrusx.New("", "")
-				c := config.MustNew(ctx, l, configx.SkipValidation())
-				postgresDsn := dockertest.RunTestPostgreSQL(t)
-				c.MustSet(ctx, config.KeyDSN, postgresDsn)
-				c.MustSet(ctx, config.KeyDbIgnoreUnknownTableColumns, test.flagValue)
-				reg, err := NewRegistryFromDSN(ctx, c, l, false, true, &contextx.Default{})
-				assert.NoError(t, err)
+	statement := "ALTER TABLE \"hydra_client\" ADD COLUMN \"temp_column\" VARCHAR(128) NOT NULL DEFAULT '';"
+	require.NoError(t, reg.Persister().Connection(ctx).RawQuery(statement).Exec())
 
-				statement := "ALTER TABLE \"hydra_client\" ADD COLUMN \"temp_column\" VARCHAR(128) NOT NULL DEFAULT '';"
-				err = reg.Persister().Connection(ctx).RawQuery(statement).Exec()
-				assert.NoError(t, err)
+	cl := &client.Client{
+		LegacyClientID: strconv.Itoa(rand.Int()),
+	}
+	require.NoError(t, reg.Persister().CreateClient(ctx, cl))
+	getClients := func(reg Registry) ([]client.Client, error) {
+		readClients := make([]client.Client, 0)
+		return readClients, reg.Persister().Connection(ctx).RawQuery("SELECT * FROM \"hydra_client\"").All(&readClients)
+	}
 
-				cl := &client.Client{
-					LegacyClientID: strconv.Itoa(rand.Int()),
-				}
+	t.Run("with ignore disabled (default behavior)", func(t *testing.T) {
+		_, err := getClients(reg)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "missing destination name temp_column")
+	})
 
-				err = reg.Persister().CreateClient(ctx, cl)
-				assert.NoError(t, err)
+	t.Run("with ignore enabled", func(t *testing.T) {
+		c.MustSet(ctx, config.KeyDBIgnoreUnknownTableColumns, true)
+		reg, err := NewRegistryFromDSN(ctx, c, l, false, true, &contextx.Default{})
+		require.NoError(t, err)
 
-				readClients := make([]client.Client, 0)
-				err = reg.Persister().Connection(ctx).RawQuery("SELECT * FROM \"hydra_client\"").All(&readClients)
-				if test.expectError {
-					assert.Error(t, err)
-					assert.Contains(t, err.Error(), "missing destination name temp_column")
-				} else {
-					assert.NoError(t, err)
-				}
-				assert.Len(t, readClients, test.expectedSize)
-			},
-		)
-	}
+		actual, err := getClients(reg)
+		require.NoError(t, err)
+		assert.Len(t, actual, 1)
+	})
 }
 
 func sussessfulPing() func(r *RegistrySQL) error {

diff --git a/persistence/sql/persister_client.go b/persistence/sql/persister_client.go
@@ -19,7 +19,10 @@ func (p *Persister) GetConcreteClient(ctx context.Context, id string) (*client.C
 	defer span.End()
 
 	var cl client.Client
-	return &cl, sqlcon.HandleError(p.QueryWithNetwork(ctx).Where("id = ?", id).First(&cl))
+	if err := p.QueryWithNetwork(ctx).Where("id = ?", id).First(&cl); err != nil {
+		return nil, sqlcon.HandleError(err)
+	}
+	return &cl, nil
 }
 
 func (p *Persister) GetClient(ctx context.Context, id string) (fosite.Client, error) {
@@ -129,7 +132,10 @@ func (p *Persister) GetClients(ctx context.Context, filters client.Filter) ([]cl
 		query.Where("owner = ?", filters.Owner)
 	}
 
-	return cs, sqlcon.HandleError(query.All(&cs))
+	if err := query.All(&cs); err != nil {
+		return nil, sqlcon.HandleError(err)
+	}
+	return cs, nil
 }
 
 func (p *Persister) CountClients(ctx context.Context) (int, error) {