Skip to content

Commit

Permalink
feat: write to UUID mapper and relation tuples in one SQL transaction (
Browse files Browse the repository at this point in the history
…#1340)

* fix: lint

* feat: wrap an SQL transaction around the UUID mapper's and the relation tuple manager's write operations
  • Loading branch information
alnr committed Jun 15, 2023
1 parent 48d1050 commit eeeecf6
Show file tree
Hide file tree
Showing 29 changed files with 176 additions and 97 deletions.
2 changes: 0 additions & 2 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@ linters:
enable:
- gosec
- goimports
- deadcode
- errcheck
- gosimple
- ineffassign
- staticcheck
- typecheck
- unused
- varcheck

linters-settings:
goimports:
Expand Down
7 changes: 4 additions & 3 deletions cmd/client/grpc_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const (

EnvReadRemote = "KETO_READ_REMOTE"
EnvWriteRemote = "KETO_WRITE_REMOTE"
EnvAuthToken = "KETO_BEARER_TOKEN" // nosec G101 -- just the key, not the value
EnvAuthToken = "KETO_BEARER_TOKEN" //nolint:gosec // just the key, not the value
EnvAuthority = "KETO_AUTHORITY"

ContextKeyTimeout contextKeys = "timeout"
Expand All @@ -51,7 +51,8 @@ func (d *connectionDetails) dialOptions() (opts []grpc.DialOption) {
if d.token != "" {
opts = append(opts,
grpc.WithPerRPCCredentials(
oauth.NewOauthAccess(&oauth2.Token{AccessToken: d.token})))
oauth.TokenSource{oauth2.StaticTokenSource(&oauth2.Token{AccessToken: d.token})},
))
}
if d.authority != "" {
opts = append(opts, grpc.WithAuthority(d.authority))
Expand All @@ -63,7 +64,7 @@ func (d *connectionDetails) dialOptions() (opts []grpc.DialOption) {
opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
case d.skipHostVerification:
opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{
// nolint explicity set through scary flag
//nolint:gosec // explicity set through scary flag
InsecureSkipVerify: true,
})))
default:
Expand Down
5 changes: 4 additions & 1 deletion doc_swagger.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import "github.com/ory/herodot"
// The standard Ory JSON API error format.
//
// swagger:model errorGeneric
//
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
type errorGeneric struct {
// Contains error details
//
Expand All @@ -20,5 +22,6 @@ type errorGeneric struct {
// An empty response
//
// swagger:response emptyResponse
// nolint:deadcode,unused
//
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
type emptyResponse struct{}
2 changes: 1 addition & 1 deletion internal/check/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func TestEngine(t *testing.T) {

insertFixtures(t, reg.RelationTupleManager(), tuples)
e := check.NewEngine(reg)
reg.Config(ctx).Set(config.KeyLimitMaxReadDepth, 5)
require.NoError(t, reg.Config(ctx).Set(config.KeyLimitMaxReadDepth, 5))

cases := []struct {
tuple string
Expand Down
16 changes: 10 additions & 6 deletions internal/check/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ func (h *Handler) getCheckNoStatus(w http.ResponseWriter, r *http.Request, _ htt
// Check Permission Or Error Request Parameters
//
// swagger:parameters checkPermissionOrError
// nolint:deadcode,unused
//
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
type checkPermissionOrError struct {
// in: query
MaxDepth int `json:"max-depth"`
Expand Down Expand Up @@ -177,7 +178,8 @@ func (h *Handler) getCheck(ctx context.Context, q url.Values) (bool, error) {
// Check Permission using Post Request Parameters
//
// swagger:parameters postCheckPermission
// nolint:deadcode,unused
//
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
type postCheckPermission struct {
// in: query
MaxDepth int `json:"max-depth"`
Expand All @@ -189,7 +191,8 @@ type postCheckPermission struct {
// Check Permission using Post Request Body
//
// swagger:model postCheckPermissionBody
// nolint:deadcode,unused
//
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
type postCheckPermissionBody struct {
ketoapi.RelationQuery
}
Expand Down Expand Up @@ -224,9 +227,9 @@ func (h *Handler) postCheckNoStatus(w http.ResponseWriter, r *http.Request, _ ht
// Post Check Permission Or Error Request Parameters
//
// swagger:parameters postCheckPermissionOrError
// nolint:deadcode,unused
//
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
type postCheckPermissionOrError struct {
// nolint:deadcode,unused
// in: query
MaxDepth int `json:"max-depth"`

Expand All @@ -237,7 +240,8 @@ type postCheckPermissionOrError struct {
// Post Check Permission Or Error Body
//
// swagger:model postCheckPermissionOrErrorBody
// nolint:deadcode,unused
//
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
type postCheckPermissionOrErrorBody struct {
ketoapi.RelationQuery
}
Expand Down
4 changes: 3 additions & 1 deletion internal/driver/config/namespace_memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ func (s *memoryNamespaceManager) GetNamespaceByConfigID(_ context.Context, id in
defer s.RUnlock()

for _, n := range s.byName {
if n.ID == id { // nolint ignore deprecated method
//lint:ignore SA1019 backwards compatibility
//nolint:staticcheck
if n.ID == id {
return n, nil
}
}
Expand Down
4 changes: 3 additions & 1 deletion internal/driver/config/namespace_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,9 @@ func (nw *NamespaceWatcher) GetNamespaceByConfigID(_ context.Context, id int32)
defer nw.RUnlock()

for _, nspace := range nw.namespaces {
if nspace.namespace.ID == id { // nolint ignore deprecated ID
//lint:ignore SA1019 backwards compatibility
//nolint:staticcheck
if nspace.namespace.ID == id {
return nspace.namespace, nil
}
}
Expand Down
4 changes: 2 additions & 2 deletions internal/driver/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func (r *RegistryDefault) serveOPLSyntax(ctx context.Context, done chan<- struct
func (r *RegistryDefault) serveMetrics(ctx context.Context, done chan<- struct{}) func() error {
ctx, cancel := context.WithCancel(ctx)

// nolint: gosec,G112 graceful.WithDefaults already sets a timeout
//nolint:gosec // graceful.WithDefaults already sets a timeout
s := graceful.WithDefaults(&http.Server{
Handler: r.metricsRouter(ctx),
Addr: r.Config(ctx).MetricsListenOn(),
Expand Down Expand Up @@ -237,7 +237,7 @@ func multiplexPort(ctx context.Context, log *logrusx.Logger, addr string, router
grpcL := m.MatchWithWriters(cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc"))
httpL := m.Match(cmux.HTTP1())

// nolint: gosec,G112 graceful.WithDefaults already sets a timeout
//nolint:gosec // graceful.WithDefaults already sets a timeout
restS := graceful.WithDefaults(&http.Server{
Handler: router,
})
Expand Down
8 changes: 8 additions & 0 deletions internal/driver/registry_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,3 +344,11 @@ func (r *RegistryDefault) Init(ctx context.Context) (err error) {
})
return
}

var _ x.TransactorProvider = (*RegistryDefault)(nil)

func (r *RegistryDefault) Transactor() interface {
Transaction(ctx context.Context, f func(ctx context.Context) error) error
} {
return r.Persister()
}
16 changes: 12 additions & 4 deletions internal/httpclient/api/openapi.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 16 additions & 10 deletions internal/httpclient/api_metadata.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions internal/httpclient/docs/MetadataApi.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions internal/persistence/definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type (

Connection(ctx context.Context) *pop.Connection
NetworkID(ctx context.Context) uuid.UUID
Transaction(ctx context.Context, f func(ctx context.Context) error) error
}
Migrator interface {
MigrationBox(ctx context.Context) (*popx.MigrationBox, error)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ func TestMigrations(t *testing.T) {
var oldRTs []*tuplesBeforeUUID
require.NoError(t, p.Connection(ctx).
Select("subject_id", "object").
// lint:ignore SA1019
//lint:ignore SA1019 backwards compatibility
//nolint:staticcheck
Where("namespace_id = ?", namespaces[1].ID).
All(&oldRTs))
assert.Equalf(t, "user", oldRTs[0].SubjectID.String, "%+v", oldRTs[0])
Expand All @@ -180,7 +181,8 @@ func TestMigrations(t *testing.T) {
for i := 0; i < len(oldRTs); i++ {
oldRTs[i] = tuplesBeforeUUID{
NetworkID: p.NetworkID(ctx),
// lint:ignore SA1019
//lint:ignore SA1019 backwards compatibility
//nolint:staticcheck
NamespaceID: namespaces[1].ID,
Object: "object-" + strconv.Itoa(i),
Relation: "pagination-works",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"github.com/ory/keto/internal/namespace"
)

// lint:file-ignore SA1019 as we migrate legacy stuff
//lint:file-ignore SA1019 as we migrate legacy stuff

// We copy the definitions of OldRelationTuple and UUIDMapping here so that the
// migration will always work on the same definitions.
Expand Down Expand Up @@ -123,14 +123,15 @@ func (rt *OldRelationTuple) ToUUID(s string) uuid.UUID {
}

func namespaceIDtoName(n namespace.Manager, id int32) (string, error) {
//nolint:staticcheck
ns, err := n.GetNamespaceByConfigID(context.Background(), id)
if err != nil {
return "", err
}
return ns.Name, nil
}

func (rt *OldRelationTuple) ToNew(n namespace.Manager) (err error, newRT *NewRelationTuple, objectMapping *UUIDMapping, subjectMapping *UUIDMapping) {
func (rt *OldRelationTuple) ToNew(n namespace.Manager) (newRT *NewRelationTuple, objectMapping *UUIDMapping, subjectMapping *UUIDMapping, err error) {
newRT = &NewRelationTuple{
ID: rt.ID,
NetworkID: rt.NetworkID,
Expand Down Expand Up @@ -209,7 +210,7 @@ var (
mappings := make([]*UUIDMapping, len(relationTuples)*2)
newTuples := make([]*NewRelationTuple, len(relationTuples))
for i := range relationTuples {
err, newTuples[i], mappings[i*2], mappings[i*2+1] = relationTuples[i].ToNew(namespaces)
newTuples[i], mappings[i*2], mappings[i*2+1], err = relationTuples[i].ToNew(namespaces)
if err != nil {
return errors.WithStack(err)
}
Expand Down Expand Up @@ -273,13 +274,14 @@ var (
if err != nil {
return fmt.Errorf("could not get namespace: %w", err)
}
ot.NamespaceID = namespace.ID
ot.NamespaceID = namespace.ID //nolint:staticcheck

if rt.SubjectSetNamespace.Valid {
subjectSetNamespace, err := namespaces.GetNamespaceByName(ctx, rt.SubjectSetNamespace.String)
if err != nil {
return fmt.Errorf("could not get subject namespace: %w", err)
}
//nolint:staticcheck
if err = ot.SubjectSetNamespaceID.Scan(subjectSetNamespace.ID); err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions internal/persistence/sql/persister.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ func (p *Persister) queryWithNetwork(ctx context.Context) *pop.Query {
return p.Connection(ctx).Where("nid = ?", p.NetworkID(ctx))
}

func (p *Persister) transaction(ctx context.Context, f func(ctx context.Context, c *pop.Connection) error) error {
return popx.Transaction(ctx, p.conn.WithContext(ctx), f)
func (p *Persister) Transaction(ctx context.Context, f func(ctx context.Context) error) error {
return popx.Transaction(ctx, p.conn.WithContext(ctx), func(ctx context.Context, _ *pop.Connection) error { return f(ctx) })
}

func (p *Persister) NetworkID(ctx context.Context) uuid.UUID {
Expand Down

0 comments on commit eeeecf6

Please sign in to comment.