Skip to content

Commit 779c4a8

Browse files
committed
Decouple schema parsing from validation parsing
1 parent 2146357 commit 779c4a8

21 files changed

+272
-38
lines changed

Diff for: .golangci.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ linters:
2929
- "rowserrcheck"
3030
- "staticcheck"
3131
- "stylecheck"
32-
- "tenv"
3332
- "typecheck"
3433
- "unconvert"
3534
- "unused"
35+
- "usetesting"
3636
- "wastedassign"
3737
- "whitespace"

Diff for: internal/client/client_test.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414

1515
func TestGetTokenWithCLIOverride(t *testing.T) {
1616
require := require.New(t)
17-
testCert, err := os.CreateTemp("", "")
17+
testCert, err := os.CreateTemp(t.TempDir(), "")
1818
require.NoError(err)
1919
_, err = testCert.Write([]byte("hi"))
2020
require.NoError(err)
@@ -100,10 +100,9 @@ func TestGetCurrentTokenWithCLIOverrideWithoutSecretFile(t *testing.T) {
100100

101101
bTrue := true
102102

103-
tmpDir, err := os.MkdirTemp("", "")
104-
require.NoError(err)
103+
tmpDir := t.TempDir()
105104
configPath := path.Join(tmpDir, "config.json")
106-
err = os.WriteFile(configPath, []byte("{}"), 0o600)
105+
err := os.WriteFile(configPath, []byte("{}"), 0o600)
107106
require.NoError(err)
108107

109108
configStore := &storage.JSONConfigStore{ConfigPath: tmpDir}

Diff for: internal/cmd/backup_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ func TestBackupParseRelsCmdFunc(t *testing.T) {
168168

169169
cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, zedtesting.StringFlag{FlagName: "prefix-filter", FlagValue: tt.filter})
170170
backupName := createTestBackup(t, tt.schema, tt.relationships)
171-
f, err := os.CreateTemp("", "parse-output")
171+
f, err := os.CreateTemp(t.TempDir(), "parse-output")
172172
require.NoError(t, err)
173173
defer func() {
174174
_ = f.Close()
@@ -189,7 +189,7 @@ func TestBackupParseRelsCmdFunc(t *testing.T) {
189189
func TestBackupParseRevisionCmdFunc(t *testing.T) {
190190
cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, zedtesting.StringFlag{FlagName: "prefix-filter", FlagValue: "test"})
191191
backupName := createTestBackup(t, testSchema, testRelationships)
192-
f, err := os.CreateTemp("", "parse-output")
192+
f, err := os.CreateTemp(t.TempDir(), "parse-output")
193193
require.NoError(t, err)
194194
defer func() {
195195
_ = f.Close()
@@ -249,7 +249,7 @@ func TestBackupParseSchemaCmdFunc(t *testing.T) {
249249
zedtesting.StringFlag{FlagName: "prefix-filter", FlagValue: tt.filter},
250250
zedtesting.BoolFlag{FlagName: "rewrite-legacy", FlagValue: tt.rewriteLegacy})
251251
backupName := createTestBackup(t, tt.schema, nil)
252-
f, err := os.CreateTemp("", "parse-output")
252+
f, err := os.CreateTemp(t.TempDir(), "parse-output")
253253
require.NoError(t, err)
254254
defer func() {
255255
_ = f.Close()

Diff for: internal/cmd/helpers_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func readLines(t *testing.T, fileName string) []string {
4242
func createTestBackup(t *testing.T, schema string, relationships []string) string {
4343
t.Helper()
4444

45-
f, err := os.CreateTemp("", "test-backup")
45+
f, err := os.CreateTemp(t.TempDir(), "test-backup")
4646
require.NoError(t, err)
4747
defer f.Close()
4848
t.Cleanup(func() {

Diff for: internal/cmd/import_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func TestImportCmdHappyPath(t *testing.T) {
2323
zedtesting.IntFlag{FlagName: "batch-size", FlagValue: 100},
2424
zedtesting.IntFlag{FlagName: "workers", FlagValue: 1},
2525
)
26-
f := filepath.Join("test", "happy-path-validation-file.yaml")
26+
f := filepath.Join("import-test", "happy-path-validation-file.yaml")
2727

2828
// Set up client
2929
ctx, cancel := context.WithCancel(context.Background())
+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
definition resource {
2+
relation user: user
3+
permission view = user
4+
}
+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
definition user {}

Diff for: internal/cmd/validate-test/external-schema.yaml

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
schemaFile: "./external-schema.zed"
3+
relationships: >-
4+
resource:1#user@user:1
5+
assertions:
6+
assertTrue:
7+
- "resource:1#user@user:1"
8+
assertFalse:
9+
- "resource:1#user@user:2"

Diff for: internal/cmd/validate-test/external-schema.zed

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
definition user {}
2+
3+
definition resource {
4+
relation user: user
5+
permission view = user
6+
}

Diff for: internal/cmd/validate-test/invalid-schema.zed

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
something something {}

Diff for: internal/cmd/validate-test/missing-schema.yaml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
relationships: >-
3+
resource:1#user@user:1
4+
assertions:
5+
assertTrue:
6+
- "resource:1#user@user:1"
7+
assertFalse:
8+
- "resource:1#user@user:2"
+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
partial foo {}
2+
3+
definition bar {
4+
...foo
5+
}

Diff for: internal/cmd/validate-test/only-passes-standard.zed

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// "or" is a reserved keyword in composable schemas
2+
definition or {}

Diff for: internal/cmd/validate-test/schema-only.zed

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
definition user {}
2+
3+
definition resource {
4+
relation user: user
5+
permission view = user
6+
}

Diff for: internal/cmd/validate-test/standard-validation.yaml

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
schema: |-
3+
definition user {}
4+
5+
definition resource {
6+
relation user: user
7+
permission view = user
8+
}
9+
relationships: >-
10+
resource:1#user@user:1
11+
assertions:
12+
assertTrue:
13+
- "resource:1#user@user:1"
14+
assertFalse:
15+
- "resource:1#user@user:2"

Diff for: internal/cmd/validate.go

+18-6
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"errors"
55
"fmt"
66
"net/url"
7-
"os"
87
"strings"
98

109
"github.com/ccoveille/go-safecast"
@@ -94,6 +93,12 @@ func validatePreRunE(cmd *cobra.Command, _ []string) error {
9493
return nil
9594
}
9695

96+
/*
97+
Okay so
98+
TODO: add flag to force one kind of resolution or the other
99+
TODO: display error output from composable schema if both fail
100+
*/
101+
97102
func validateCmdFunc(cmd *cobra.Command, filenames []string) error {
98103
// Initialize variables for multiple files
99104
var (
@@ -122,7 +127,7 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) error {
122127
if err != nil {
123128
var errWithSource spiceerrors.WithSourceError
124129
if errors.As(err, &errWithSource) {
125-
ouputErrorWithSource(validateContents, errWithSource)
130+
outputErrorWithSource(validateContents, errWithSource)
126131
}
127132
return err
128133
}
@@ -145,13 +150,17 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) error {
145150
return err
146151
}
147152
if devErrs != nil {
153+
// TODO: this depends on where `schema:` is in the file
148154
schemaOffset := 1 /* for the 'schema:' */
149155
if isOnlySchema {
150156
schemaOffset = 0
151157
}
152158

153159
// Output errors
154160
outputDeveloperErrorsWithLineOffset(validateContents, devErrs.InputErrors, schemaOffset)
161+
// TODO: this may not make sense
162+
// Return the first error up the chain
163+
return errors.New(devErrs.InputErrors[0].Message)
155164
}
156165
// Run assertions
157166
adevErrs, aerr := development.RunAllAssertions(devCtx, &parsed.Assertions)
@@ -160,6 +169,9 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) error {
160169
}
161170
if adevErrs != nil {
162171
outputDeveloperErrors(validateContents, adevErrs)
172+
// TODO: this may not make sense
173+
// Return the first of the errors up the chain
174+
return errors.New(adevErrs[0].Message)
163175
}
164176
successfullyValidatedFiles++
165177

@@ -170,6 +182,9 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) error {
170182
}
171183
if erDevErrs != nil {
172184
outputDeveloperErrors(validateContents, erDevErrs)
185+
// TODO: this may not make sense
186+
// Return the first of the errors up the chain
187+
return errors.New(erDevErrs[0].Message)
173188
}
174189
// Print out any warnings for all files
175190
warnings, err := development.GetWarnings(ctx, devCtx)
@@ -203,10 +218,9 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) error {
203218
return nil
204219
}
205220

206-
func ouputErrorWithSource(validateContents []byte, errWithSource spiceerrors.WithSourceError) {
221+
func outputErrorWithSource(validateContents []byte, errWithSource spiceerrors.WithSourceError) {
207222
console.Printf("%s%s\n", errorPrefix(), errorMessageStyle().Render(errWithSource.Error()))
208223
outputForLine(validateContents, errWithSource.LineNumber, errWithSource.SourceCodeString, 0) // errWithSource.LineNumber is 1-indexed
209-
os.Exit(1)
210224
}
211225

212226
func outputForLine(validateContents []byte, oneIndexedLineNumber uint64, sourceCodeString string, oneIndexedColumnPosition uint64) {
@@ -234,8 +248,6 @@ func outputDeveloperErrorsWithLineOffset(validateContents []byte, devErrors []*d
234248
for _, devErr := range devErrors {
235249
outputDeveloperError(devErr, lines, lineOffset)
236250
}
237-
238-
os.Exit(1)
239251
}
240252

241253
func outputDeveloperError(devError *devinterface.DeveloperError, lines []string, lineOffset int) {

0 commit comments

Comments
 (0)