Skip to content

Commit

Permalink
Fix small TODO in type system with a small code move
Browse files Browse the repository at this point in the history
  • Loading branch information
josephschorr committed Feb 21, 2024
1 parent 40d6692 commit 7629abf
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 32 deletions.
9 changes: 4 additions & 5 deletions internal/graph/reachableresources.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/authzed/spicedb/internal/dispatch"
datastoremw "github.com/authzed/spicedb/internal/middleware/datastore"
"github.com/authzed/spicedb/internal/namespace"
"github.com/authzed/spicedb/pkg/datastore"
"github.com/authzed/spicedb/pkg/datastore/options"
core "github.com/authzed/spicedb/pkg/proto/core/v1"
Expand Down Expand Up @@ -107,12 +106,12 @@ func (crr *CursoredReachableResources) afterSameType(
// Load the type system and reachability graph to find the entrypoints for the reachability.
ds := datastoremw.MustFromContext(ctx)
reader := ds.SnapshotReader(req.Revision)
_, typeSystem, err := namespace.ReadNamespaceAndTypes(ctx, req.ResourceRelation.Namespace, reader)
_, typeSystem, err := typesystem.ReadNamespaceAndTypes(ctx, req.ResourceRelation.Namespace, reader)
if err != nil {
return err
}

rg := typesystem.ReachabilityGraphFor(typeSystem.AsValidated())
rg := typesystem.ReachabilityGraphFor(typeSystem)
entrypoints, err := rg.OptimizedEntrypointsForSubjectToResource(ctx, &core.RelationReference{
Namespace: req.SubjectRelation.Namespace,
Relation: req.SubjectRelation.Relation,
Expand Down Expand Up @@ -174,7 +173,7 @@ func (crr *CursoredReachableResources) lookupRelationEntrypoint(
return err
}

_, relTypeSystem, err := namespace.ReadNamespaceAndTypes(ctx, relationReference.Namespace, reader)
_, relTypeSystem, err := typesystem.ReadNamespaceAndTypes(ctx, relationReference.Namespace, reader)
if err != nil {
return err
}
Expand Down Expand Up @@ -349,7 +348,7 @@ func (crr *CursoredReachableResources) lookupTTUEntrypoint(ctx context.Context,
) error {
containingRelation := entrypoint.ContainingRelationOrPermission()

_, ttuTypeSystem, err := namespace.ReadNamespaceAndTypes(ctx, containingRelation.Namespace, reader)
_, ttuTypeSystem, err := typesystem.ReadNamespaceAndTypes(ctx, containingRelation.Namespace, reader)
if err != nil {
return err
}
Expand Down
16 changes: 0 additions & 16 deletions internal/namespace/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/authzed/spicedb/pkg/datastore"
"github.com/authzed/spicedb/pkg/genutil/mapz"
core "github.com/authzed/spicedb/pkg/proto/core/v1"
"github.com/authzed/spicedb/pkg/typesystem"
)

// ReadNamespaceAndRelation checks that the specified namespace and relation exist in the
Expand Down Expand Up @@ -129,21 +128,6 @@ func CheckNamespaceAndRelation(
return NewRelationNotFoundErr(namespace, relation)
}

// ReadNamespaceAndTypes reads a namespace definition, version, and type system and returns it if found.
func ReadNamespaceAndTypes(
ctx context.Context,
nsName string,
ds datastore.Reader,
) (*core.NamespaceDefinition, *typesystem.TypeSystem, error) {
nsDef, _, err := ds.ReadNamespaceByName(ctx, nsName)
if err != nil {
return nil, nil, err
}

ts, terr := typesystem.NewNamespaceTypeSystem(nsDef, typesystem.ResolverForDatastoreReader(ds))
return nsDef, ts, terr
}

// ListReferencedNamespaces returns the names of all namespaces referenced in the
// given namespace definitions. This includes the namespaces themselves, as well as
// any found in type information on relations.
Expand Down
4 changes: 2 additions & 2 deletions internal/services/integrationtesting/consistency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/authzed/spicedb/internal/developmentmembership"
"github.com/authzed/spicedb/internal/dispatch"
"github.com/authzed/spicedb/internal/graph"
"github.com/authzed/spicedb/internal/namespace"
"github.com/authzed/spicedb/internal/services/integrationtesting/consistencytestutil"
"github.com/authzed/spicedb/pkg/datastore"
"github.com/authzed/spicedb/pkg/development"
Expand All @@ -31,6 +30,7 @@ import (
devinterface "github.com/authzed/spicedb/pkg/proto/developer/v1"
dispatchv1 "github.com/authzed/spicedb/pkg/proto/dispatch/v1"
"github.com/authzed/spicedb/pkg/tuple"
"github.com/authzed/spicedb/pkg/typesystem"
"github.com/authzed/spicedb/pkg/validationfile"
"github.com/authzed/spicedb/pkg/validationfile/blocks"
)
Expand Down Expand Up @@ -78,7 +78,7 @@ func runConsistencyTestSuiteForFile(t *testing.T, filePath string, useCachingDis
require.NoError(t, err)

for _, nsDef := range cad.Populated.NamespaceDefinitions {
_, ts, err := namespace.ReadNamespaceAndTypes(
_, ts, err := typesystem.ReadNamespaceAndTypes(
cad.Ctx,
nsDef.Name,
cad.DataStore.SnapshotReader(headRevision),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import (
"github.com/authzed/spicedb/internal/dispatch/graph"
"github.com/authzed/spicedb/internal/dispatch/keys"
datastoremw "github.com/authzed/spicedb/internal/middleware/datastore"
"github.com/authzed/spicedb/internal/namespace"
"github.com/authzed/spicedb/internal/testserver"
"github.com/authzed/spicedb/pkg/datastore"
"github.com/authzed/spicedb/pkg/typesystem"
"github.com/authzed/spicedb/pkg/validationfile"
)

Expand Down Expand Up @@ -59,7 +59,7 @@ func BuildDataAndCreateClusterForTesting(t *testing.T, consistencyTestFilePath s

// Validate the type system for each namespace.
for _, nsDef := range populated.NamespaceDefinitions {
_, ts, err := namespace.ReadNamespaceAndTypes(
_, ts, err := typesystem.ReadNamespaceAndTypes(
dsCtx,
nsDef.Name,
ds.SnapshotReader(revision),
Expand Down
29 changes: 22 additions & 7 deletions pkg/typesystem/typesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"

"github.com/authzed/spicedb/pkg/datastore"
"github.com/authzed/spicedb/pkg/genutil/mapz"
"github.com/authzed/spicedb/pkg/graph"
nspkg "github.com/authzed/spicedb/pkg/namespace"
Expand Down Expand Up @@ -365,13 +366,6 @@ func (nts *TypeSystem) computeReferencesWildcardType(ctx context.Context, relati
return nil, nil
}

// AsValidated returns the current type system marked as validated. This method should *only* be
// called for type systems read from storage.
// TODO(jschorr): Maybe have the namespaces loaded from datastore do this automatically?
func (nts *TypeSystem) AsValidated() *ValidatedNamespaceTypeSystem {
return &ValidatedNamespaceTypeSystem{nts}
}

// Validate runs validation on the type system for the namespace to ensure it is consistent.
func (nts *TypeSystem) Validate(ctx context.Context) (*ValidatedNamespaceTypeSystem, error) {
for _, relation := range nts.relationMap {
Expand Down Expand Up @@ -625,3 +619,24 @@ func NewTypeErrorWithSource(wrapped error, withSource nspkg.WithSourcePosition,
0,
))
}

// ReadNamespaceAndTypes reads a namespace definition, version, and type system and returns it if found.
func ReadNamespaceAndTypes(
ctx context.Context,
nsName string,
ds datastore.Reader,
) (*core.NamespaceDefinition, *ValidatedNamespaceTypeSystem, error) {
nsDef, _, err := ds.ReadNamespaceByName(ctx, nsName)
if err != nil {
return nil, nil, err
}

ts, terr := NewNamespaceTypeSystem(nsDef, ResolverForDatastoreReader(ds))
if terr != nil {
return nil, nil, terr
}

// NOTE: since the type system was read from the datastore, it must have been validated
// on the way in, so it is safe for us to return it as a validated type system.
return nsDef, &ValidatedNamespaceTypeSystem{ts}, nil
}

0 comments on commit 7629abf

Please sign in to comment.