From 3741ac54f2271794eb91b7718a8b1ac1edc3e5e5 Mon Sep 17 00:00:00 2001 From: Denise Li Date: Fri, 24 May 2024 23:37:39 -0400 Subject: [PATCH] comments --- backend/controller/dal/database_resolver.go | 60 +++++++------------ .../controller/dal/database_resolver_test.go | 4 +- backend/controller/sql/models.go | 8 +-- backend/controller/sql/querier.go | 6 +- backend/controller/sql/queries.sql | 11 ++-- backend/controller/sql/queries.sql.go | 27 +++++---- backend/controller/sql/schema/001_init.sql | 8 +-- common/configuration/db_provider.go | 11 +++- common/configuration/db_provider_test.go | 4 +- 9 files changed, 69 insertions(+), 70 deletions(-) diff --git a/backend/controller/dal/database_resolver.go b/backend/controller/dal/database_resolver.go index c36ff82266..6012898a16 100644 --- a/backend/controller/dal/database_resolver.go +++ b/backend/controller/dal/database_resolver.go @@ -6,7 +6,7 @@ import ( "github.com/TBD54566975/ftl/backend/controller/sql" "github.com/TBD54566975/ftl/common/configuration" - "github.com/alecthomas/types/optional" + "github.com/TBD54566975/ftl/internal/slices" ) // DatabaseResolver loads values a project's configuration from the given database. @@ -26,56 +26,42 @@ func (d *DAL) NewConfigResolver() DatabaseResolver { } func (d DatabaseResolver) Get(ctx context.Context, ref configuration.Ref) (*url.URL, error) { - module, moduleOk := ref.Module.Get() - if !moduleOk { - module = "" // global - } - accessor, err := d.db.GetConfig(ctx, module, ref.Name) + value, err := d.db.GetConfig(ctx, ref.Module, ref.Name) if err != nil { - // If we could not find this config within the module, then try getting - // again with scope set to global - if moduleOk { - return d.Get(ctx, configuration.Ref{ - Module: optional.None[string](), - Name: ref.Name, - }) - } return nil, configuration.ErrNotFound } - return url.Parse(accessor) + p := configuration.DBProvider{true} + return p.Store(ctx, ref, []byte(value)) } func (d DatabaseResolver) List(ctx context.Context) ([]configuration.Entry, error) { configs, err := d.db.ListConfigs(ctx) if err != nil { - return nil, err + return nil, translatePGError(err) } - entries := []configuration.Entry{} - for _, c := range configs { - u, err := url.Parse(c.Accessor) - if err != nil { - return nil, err - } - entries = append(entries, configuration.Entry{ - Ref: configuration.NewRef(c.Module, c.Name), + p := configuration.DBProvider{true} + return slices.Map(configs, func(c sql.Config) configuration.Entry { + ref := configuration.Ref{c.Module, c.Name} + // err can be ignored because Store always returns a nil error + u, _ := p.Store(ctx, ref, c.Value) + return configuration.Entry{ + Ref: ref, Accessor: u, - }) - } - return entries, nil + } + }), nil } func (d DatabaseResolver) Set(ctx context.Context, ref configuration.Ref, key *url.URL) error { - return d.db.SetConfig(ctx, moduleAsString(ref), ref.Name, key.String()) + p := configuration.DBProvider{true} + value, err := p.Load(ctx, ref, key) + if err != nil { + return err + } + err = d.db.SetConfig(ctx, ref.Module, ref.Name, value) + return translatePGError(err) } func (d DatabaseResolver) Unset(ctx context.Context, ref configuration.Ref) error { - return d.db.UnsetConfig(ctx, moduleAsString(ref), ref.Name) -} - -func moduleAsString(ref configuration.Ref) string { - module, ok := ref.Module.Get() - if !ok { - return "" - } - return module + err := d.db.UnsetConfig(ctx, ref.Module, ref.Name) + return translatePGError(err) } diff --git a/backend/controller/dal/database_resolver_test.go b/backend/controller/dal/database_resolver_test.go index 2b4192848c..88aafd7ba4 100644 --- a/backend/controller/dal/database_resolver_test.go +++ b/backend/controller/dal/database_resolver_test.go @@ -46,14 +46,14 @@ func TestDBResolverRoundTrip(t *testing.T) { } ctx, cr := setup(t) - u := URL("db://asdfasdf") + u := URL("db://ImFzZGYi") // asdf for _, test := range tests { t.Run(test.TestName, func(t *testing.T) { if test.PresetGlobal { err := cr.Set(ctx, configuration.Ref{ Module: optional.None[string](), Name: "configname", - }, URL("db://qwerty")) + }, URL("db://InF3ZXJ0eSI")) // qwerty assert.NoError(t, err) } err := cr.Set(ctx, configuration.Ref{ diff --git a/backend/controller/sql/models.go b/backend/controller/sql/models.go index 26cf9580c1..a535f25d4f 100644 --- a/backend/controller/sql/models.go +++ b/backend/controller/sql/models.go @@ -383,10 +383,10 @@ type AsyncCall struct { } type Config struct { - ID int64 - Module string - Name string - Accessor string + ID int64 + Module optional.Option[string] + Name string + Value []byte } type Controller struct { diff --git a/backend/controller/sql/querier.go b/backend/controller/sql/querier.go index 733bf5e8cb..b239ed6e31 100644 --- a/backend/controller/sql/querier.go +++ b/backend/controller/sql/querier.go @@ -39,7 +39,7 @@ type Querier interface { GetArtefactContentRange(ctx context.Context, start int32, count int32, iD int64) ([]byte, error) // Return the digests that exist in the database. GetArtefactDigests(ctx context.Context, digests [][]byte) ([]GetArtefactDigestsRow, error) - GetConfig(ctx context.Context, module string, name string) (string, error) + GetConfig(ctx context.Context, module optional.Option[string], name string) ([]byte, error) GetCronJobs(ctx context.Context) ([]GetCronJobsRow, error) GetDeployment(ctx context.Context, key model.DeploymentKey) (GetDeploymentRow, error) // Get all artefacts matching the given digests. @@ -83,10 +83,10 @@ type Querier interface { // // "key" is the unique identifier for the FSM execution. SendFSMEvent(ctx context.Context, arg SendFSMEventParams) (int64, error) - SetConfig(ctx context.Context, module string, name string, accessor string) error + SetConfig(ctx context.Context, module optional.Option[string], name string, value []byte) error SetDeploymentDesiredReplicas(ctx context.Context, key model.DeploymentKey, minReplicas int32) error StartCronJobs(ctx context.Context, keys []string) ([]StartCronJobsRow, error) - UnsetConfig(ctx context.Context, module string, name string) error + UnsetConfig(ctx context.Context, module optional.Option[string], name string) error UpsertController(ctx context.Context, key model.ControllerKey, endpoint string) (int64, error) UpsertModule(ctx context.Context, language string, name string) (int64, error) // Upsert a runner and return the deployment ID that it is assigned to, if any. diff --git a/backend/controller/sql/queries.sql b/backend/controller/sql/queries.sql index 9dc8bf5bc7..21e3b68a5d 100644 --- a/backend/controller/sql/queries.sql +++ b/backend/controller/sql/queries.sql @@ -547,10 +547,13 @@ VALUES ((SELECT id FROM execution), (SELECT id FROM call)) RETURNING id; -- name: GetConfig :one -SELECT accessor +SELECT value FROM configs -WHERE module = @module - AND name = @name; +WHERE + (module IS NULL OR module = @module) + AND name = @name +ORDER BY module NULLS LAST +LIMIT 1; -- name: ListConfigs :many SELECT * @@ -558,7 +561,7 @@ FROM configs ORDER BY module, name; -- name: SetConfig :exec -INSERT INTO configs (module, name, accessor) +INSERT INTO configs (module, name, value) VALUES ($1, $2, $3); -- name: UnsetConfig :exec diff --git a/backend/controller/sql/queries.sql.go b/backend/controller/sql/queries.sql.go index c83ef30eba..4a57b336ff 100644 --- a/backend/controller/sql/queries.sql.go +++ b/backend/controller/sql/queries.sql.go @@ -575,17 +575,20 @@ func (q *Queries) GetArtefactDigests(ctx context.Context, digests [][]byte) ([]G } const getConfig = `-- name: GetConfig :one -SELECT accessor +SELECT value FROM configs -WHERE module = $1 +WHERE + (module IS NULL OR module = $1) AND name = $2 +ORDER BY module NULLS LAST +LIMIT 1 ` -func (q *Queries) GetConfig(ctx context.Context, module string, name string) (string, error) { +func (q *Queries) GetConfig(ctx context.Context, module optional.Option[string], name string) ([]byte, error) { row := q.db.QueryRow(ctx, getConfig, module, name) - var accessor string - err := row.Scan(&accessor) - return accessor, err + var value []byte + err := row.Scan(&value) + return value, err } const getCronJobs = `-- name: GetCronJobs :many @@ -1558,7 +1561,7 @@ func (q *Queries) KillStaleRunners(ctx context.Context, timeout time.Duration) ( } const listConfigs = `-- name: ListConfigs :many -SELECT id, module, name, accessor +SELECT id, module, name, value FROM configs ORDER BY module, name ` @@ -1576,7 +1579,7 @@ func (q *Queries) ListConfigs(ctx context.Context) ([]Config, error) { &i.ID, &i.Module, &i.Name, - &i.Accessor, + &i.Value, ); err != nil { return nil, err } @@ -1750,12 +1753,12 @@ func (q *Queries) SendFSMEvent(ctx context.Context, arg SendFSMEventParams) (int } const setConfig = `-- name: SetConfig :exec -INSERT INTO configs (module, name, accessor) +INSERT INTO configs (module, name, value) VALUES ($1, $2, $3) ` -func (q *Queries) SetConfig(ctx context.Context, module string, name string, accessor string) error { - _, err := q.db.Exec(ctx, setConfig, module, name, accessor) +func (q *Queries) SetConfig(ctx context.Context, module optional.Option[string], name string, value []byte) error { + _, err := q.db.Exec(ctx, setConfig, module, name, value) return err } @@ -1842,7 +1845,7 @@ DELETE FROM configs WHERE module = $1 AND name = $2 ` -func (q *Queries) UnsetConfig(ctx context.Context, module string, name string) error { +func (q *Queries) UnsetConfig(ctx context.Context, module optional.Option[string], name string) error { _, err := q.db.Exec(ctx, unsetConfig, module, name) return err } diff --git a/backend/controller/sql/schema/001_init.sql b/backend/controller/sql/schema/001_init.sql index a98f772689..0b809ac923 100644 --- a/backend/controller/sql/schema/001_init.sql +++ b/backend/controller/sql/schema/001_init.sql @@ -449,10 +449,10 @@ CREATE TABLE fsm_transitions ( CREATE TABLE configs ( - id BIGINT NOT NULL GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, - module TEXT NOT NULL, - name TEXT NOT NULL, - accessor TEXT NOT NULL, + id BIGINT NOT NULL GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, + module TEXT, + name TEXT NOT NULL, + value JSONB NOT NULL, UNIQUE (module, name) ); diff --git a/common/configuration/db_provider.go b/common/configuration/db_provider.go index a67173a5bc..5dade872cb 100644 --- a/common/configuration/db_provider.go +++ b/common/configuration/db_provider.go @@ -2,6 +2,8 @@ package configuration import ( "context" + "encoding/base64" + "fmt" "net/url" ) @@ -18,11 +20,16 @@ func (DBProvider) Key() string { return "db" } func (d DBProvider) Writer() bool { return d.DB } func (DBProvider) Load(ctx context.Context, ref Ref, key *url.URL) ([]byte, error) { - return []byte(key.Host), nil + data, err := base64.RawURLEncoding.DecodeString(key.Host) + if err != nil { + return nil, fmt.Errorf("invalid base64 data in db configuration: %w", err) + } + return data, nil } func (DBProvider) Store(ctx context.Context, ref Ref, value []byte) (*url.URL, error) { - return &url.URL{Scheme: "db", Host: string(value)}, nil + b64 := base64.RawURLEncoding.EncodeToString(value) + return &url.URL{Scheme: "db", Host: b64}, nil } func (DBProvider) Delete(ctx context.Context, ref Ref) error { diff --git a/common/configuration/db_provider_test.go b/common/configuration/db_provider_test.go index e0e76147c2..e6f6ff12f6 100644 --- a/common/configuration/db_provider_test.go +++ b/common/configuration/db_provider_test.go @@ -10,8 +10,8 @@ import ( func TestDBProvider(t *testing.T) { ctx := context.Background() - u := URL("db://asdf") - b := []byte("asdf") + u := URL("db://ImFzZGYi") + b := []byte(`"asdf"`) ref := Ref{ Module: optional.None[string](), Name: "name",