Skip to content

Only apply the schema definition prefix when explicitly specified #715

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
josephschorr wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Only apply the schema definition prefix when explicitly specified #715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/zed.md
View file
Open in desktop

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

7 changes: 2 additions & 5 deletions internal/cmd/import.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,7 @@ func registerImportCmd(rootCmd *cobra.Command) {
if err != nil {
return err
}
prefix, err := determinePrefixForSchema(cmd.Context(), cobrautil.MustGetString(cmd, "schema-definition-prefix"), client, nil)
if err != nil {
return err
}
prefix := cobrautil.MustGetString(cmd, "schema-definition-prefix")
log.Trace().Msgf("using prefix: %s", prefix)
return importCmdFunc(cmd, client, client, prefix, args[0])
},
Expand All @@ -71,7 +68,7 @@ func registerImportCmd(rootCmd *cobra.Command) {
importCmd.Flags().Int("workers", 1, "number of concurrent batching workers")
importCmd.Flags().Bool("schema", true, "import schema")
importCmd.Flags().Bool("relationships", true, "import relationships")
importCmd.Flags().String("schema-definition-prefix", "", "prefix to add to the schema's definition(s) before importing")
importCmd.Flags().String("schema-definition-prefix", "", "prefix to add to the schema's definition(s) before importing; no prefix is added unless specified")
}

func importCmdFunc(cmd *cobra.Command, schemaClient v1.SchemaServiceClient, relationshipsClient v1.PermissionsServiceClient, prefix, filename string) error {
Expand Down
75 changes: 4 additions & 71 deletions internal/cmd/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ import (
"io"
"os"
"path/filepath"
"strings"

"github.com/ccoveille/go-safecast/v2"
"github.com/jzelinskie/cobrautil/v2"
"github.com/jzelinskie/stringz"
"github.com/rs/zerolog/log"
"github.com/spf13/cobra"
"golang.org/x/term"
Expand Down Expand Up @@ -95,11 +93,11 @@ func registerAdditionalSchemaCmds(schemaCmd *cobra.Command) {

schemaCmd.AddCommand(schemaCopyCmd)
schemaCopyCmd.Flags().Bool("json", false, "output as JSON")
schemaCopyCmd.Flags().String("schema-definition-prefix", "", "prefix to add to the schema's definition(s) before writing")
schemaCopyCmd.Flags().String("schema-definition-prefix", "", "prefix to add to the schema's definition(s) before writing; no prefix is added unless specified")

schemaCmd.AddCommand(schemaWriteCmd)
schemaWriteCmd.Flags().Bool("json", false, "output as JSON")
schemaWriteCmd.Flags().String("schema-definition-prefix", "", "prefix to add to the schema's definition(s) before writing")
schemaWriteCmd.Flags().String("schema-definition-prefix", "", "prefix to add to the schema's definition(s) before writing; no prefix is added unless specified")

schemaCmd.AddCommand(schemaDiffCmd)

Expand Down Expand Up @@ -230,12 +228,7 @@ func schemaCopyInner(ctx context.Context, srcClient, destClient v1.SchemaService
}
log.Trace().Interface("response", readResp).Msg("read schema")

prefix, err := determinePrefixForSchema(ctx, definitionPrefix, nil, &readResp.SchemaText)
if err != nil {
return nil, err
}

schemaText, err := rewriteSchema(ctx, readResp.SchemaText, prefix)
schemaText, err := rewriteSchema(ctx, readResp.SchemaText, definitionPrefix)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -284,11 +277,7 @@ func schemaWriteCmdImpl(cmd *cobra.Command, args []string, client v1.SchemaServi
return errors.New("attempted to write empty schema")
}

prefix, err := determinePrefixForSchema(cmd.Context(), cobrautil.MustGetString(cmd, "schema-definition-prefix"), client, nil)
if err != nil {
return err
}

prefix := cobrautil.MustGetString(cmd, "schema-definition-prefix")
schemaText, err := rewriteSchema(cmd.Context(), string(schemaBytes), prefix)
if err != nil {
return err
Expand Down Expand Up @@ -334,62 +323,6 @@ func rewriteSchema(ctx context.Context, existingSchemaText string, definitionPre
return generated, err
}

// determinePrefixForSchema determines the prefix to be applied to a schema that will be written.
//
// If specifiedPrefix is non-empty, it is returned immediately.
// If existingSchema is non-nil, it is parsed for the prefix.
// Otherwise, the client is used to retrieve the existing schema (if any), and the prefix is retrieved from there.
func determinePrefixForSchema(ctx context.Context, specifiedPrefix string, client v1.SchemaServiceClient, existingSchema *string) (string, error) {
if specifiedPrefix != "" {
return specifiedPrefix, nil
}

var schemaText string
if existingSchema != nil {
schemaText = *existingSchema
} else {
readSchemaText, err := commands.ReadSchema(ctx, client)
if err != nil {
return "", nil
}
schemaText = readSchemaText
}

// If there is no schema found, return the empty string.
if schemaText == "" {
return "", nil
}

// Otherwise, compile the schema and grab the prefixes of the namespaces defined.
found, err := compiler.Compile(
compiler.InputSchema{Source: input.Source("schema"), SchemaString: schemaText},
compiler.AllowUnprefixedObjectType(),
compiler.SkipValidation(),
)
if err != nil {
return "", err
}

foundPrefixes := make([]string, 0, len(found.OrderedDefinitions))
for _, def := range found.OrderedDefinitions {
if strings.Contains(def.GetName(), "/") {
parts := strings.Split(def.GetName(), "/")
foundPrefixes = append(foundPrefixes, parts[0])
} else {
foundPrefixes = append(foundPrefixes, "")
}
}

prefixes := stringz.Dedup(foundPrefixes)
if len(prefixes) == 1 {
prefix := prefixes[0]
log.Debug().Str("prefix", prefix).Msg("found schema definition prefix")
return prefix, nil
}

return "", nil
}

func schemaCompileOuter(cmd *cobra.Command, args []string) (bool, error) {
outputFilepath := cobrautil.MustGetString(cmd, "out")

Expand Down
81 changes: 20 additions & 61 deletions internal/cmd/schema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,64 +23,6 @@ import (
"github.com/authzed/zed/internal/zedtesting"
)

func TestDeterminePrefixForSchema(t *testing.T) {
tests := []struct {
name string
existingSchema string
specifiedPrefix string
expectedPrefix string
}{
{
"empty schema",
"",
"",
"",
},
{
"no prefix, none specified",
`definition user {}`,
"",
"",
},
{
"no prefix, one specified",
`definition user {}`,
"test",
"test",
},
{
"prefix found",
`definition test/user {}`,
"",
"test",
},
{
"multiple prefixes found",
`definition test/user {}

definition something/resource {}`,
"",
"",
},
{
"multiple prefixes found, one specified",
`definition test/user {}

definition something/resource {}`,
"foobar",
"foobar",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
found, err := determinePrefixForSchema(t.Context(), test.specifiedPrefix, nil, &test.existingSchema)
require.NoError(t, err)
require.Equal(t, test.expectedPrefix, found)
})
}
}

func TestRewriteSchema(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -366,6 +308,7 @@ func TestSchemaWrite(t *testing.T) {
testCases := map[string]struct {
schemaMakerFn func() ([]string, error)
terminalChecker *mockTermChecker
existingSchema string
expectErr string
expectSchemaWritten string
}{
Expand Down Expand Up @@ -397,6 +340,20 @@ definition resource {
terminalChecker: &mockTermChecker{returnVal: false},
expectSchemaWritten: "definition user{}\ndefinition document { relation read: user }",
},
`existing_prefixed_schema_does_not_prefix`: {
schemaMakerFn: func() ([]string, error) {
return []string{
filepath.Join("write-schema-test", "basic.zed"),
}, nil
},
existingSchema: `definition someprefix/user {}`,
expectSchemaWritten: `definition user {}
definition resource {
relation view: user
permission viewer = view
}`,
terminalChecker: &mockTermChecker{returnVal: false},
},
`schema_from_stdin_but_terminal`: {
schemaMakerFn: func() ([]string, error) {
schemaContent := "definition user{}\ndefinition document { relation read: user }"
Expand Down Expand Up @@ -450,11 +407,13 @@ definition resource {
defer ctrl.Finish()
mockClient := NewMockSchemaServiceClient(ctrl)

// ReadSchema is always called at least once
// Writing never consults the existing schema: the prefix is applied only when
// explicitly specified via --schema-definition-prefix. Serving the existing schema
// here ensures a reintroduced prefix inference would be caught by the assertions below.
mockClient.EXPECT().
ReadSchema(gomock.Any(), gomock.Any()).
Return(&v1.ReadSchemaResponse{SchemaText: ""}, nil).
MaxTimes(2) // sometimes we read for prefix determination
Return(&v1.ReadSchemaResponse{SchemaText: tc.existingSchema}, nil).
AnyTimes()

// Set up WriteSchema expectations based on test case
var receivedSchema string
Expand Down
23 changes: 0 additions & 23 deletions internal/commands/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
package commands

import (
"context"

"github.com/jzelinskie/cobrautil/v2"
"github.com/jzelinskie/stringz"
"github.com/rs/zerolog/log"
"github.com/spf13/cobra"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"

Expand Down Expand Up @@ -63,22 +59,3 @@ func schemaReadCmdFunc(cmd *cobra.Command, _ []string) error {
console.Println(stringz.Join("\n\n", resp.SchemaText))
return nil
}

// ReadSchema calls read schema for the client and returns the schema found.
func ReadSchema(ctx context.Context, client v1.SchemaServiceClient) (string, error) {
request := &v1.ReadSchemaRequest{}
log.Trace().Interface("request", request).Msg("requesting schema read")

resp, err := client.ReadSchema(ctx, request)
if err != nil {
errStatus, ok := status.FromError(err)
if !ok || errStatus.Code() != codes.NotFound {
return "", err
}

log.Debug().Msg("no schema defined")
return "", nil
}

return resp.SchemaText, nil
}
Loading