Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
jaredoconnell committed Mar 20, 2024
1 parent 28b929e commit 9b551cb
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 97 deletions.
4 changes: 1 addition & 3 deletions schema/any.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,7 @@ func (a *AnySchema) Serialize(data any) (any, error) {
return a.checkAndConvert(data)
}

func (a *AnySchema) ApplyScope(scope Scope, namespace string) {

}
func (a *AnySchema) ApplyScope(_ Scope, _ string) {}

func (a *AnySchema) ValidateReferences() error {
// No references in this type. No work to do.
Expand Down
4 changes: 1 addition & 3 deletions schema/bool.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,7 @@ func (b BoolSchema) SerializeType(data bool) (any, error) {
return b.Serialize(data)
}

func (b BoolSchema) ApplyScope(scope Scope, namespace string) {

}
func (b BoolSchema) ApplyScope(_ Scope, _ string) {}

func (b BoolSchema) ValidateReferences() error {
// No references in this type. No work to do.
Expand Down
3 changes: 1 addition & 2 deletions schema/enum.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ func (e EnumSchema[T]) ValidValues() map[T]*DisplayValue {
return e.ValidValuesMap
}

func (e EnumSchema[T]) ApplyScope(scope Scope, namespace string) {
}
func (e EnumSchema[T]) ApplyScope(_ Scope, _ string) {}

func (e EnumSchema[T]) ValidateReferences() error {
// No references in this type. No work to do.
Expand Down
3 changes: 1 addition & 2 deletions schema/float.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ func (f FloatSchema) Max() *float64 {
func (f FloatSchema) Units() *UnitsDefinition {
return f.UnitsValue
}
func (f FloatSchema) ApplyScope(scope Scope, namespace string) {
}
func (f FloatSchema) ApplyScope(_ Scope, _ string) {}

func (f FloatSchema) ValidateReferences() error {
// No references in this type. No work to do.
Expand Down
3 changes: 1 addition & 2 deletions schema/int.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ func (i IntSchema) ReflectedType() reflect.Type {
return reflect.TypeOf(int64(0))
}

func (i IntSchema) ApplyScope(scope Scope, namespace string) {
}
func (i IntSchema) ApplyScope(_ Scope, _ string) {}

func (i IntSchema) ValidateReferences() error {
// No references in this type. No work to do.
Expand Down
3 changes: 1 addition & 2 deletions schema/pattern.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ func (p PatternSchema) TypeID() TypeID {
return TypeIDPattern
}

func (p PatternSchema) ApplyScope(scope Scope, namespace string) {
}
func (p PatternSchema) ApplyScope(_ Scope, _ string) {}

func (p PatternSchema) ValidateReferences() error {
// No references in this type. No work to do.
Expand Down
61 changes: 33 additions & 28 deletions schema/ref.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,7 @@ type Ref interface {

// NewRefSchema creates a new reference to an object in a wrapping Scope by ID.
func NewRefSchema(id string, display Display) *RefSchema {
return &RefSchema{
id,
display,
DEFAULT_NAMESPACE,
nil,
}
return NewNamespacedRefSchema(id, DEFAULT_NAMESPACE, display)
}

// NewNamespacedRefSchema creates a new reference to an object in a wrapping Scope by ID and namespace.
Expand All @@ -47,7 +42,9 @@ type RefSchema struct {
func (r *RefSchema) Properties() map[string]*PropertySchema {
if r.referencedObjectCache == nil {
panic(BadArgumentError{
Message: fmt.Sprintf("Properties was called before ApplyScope with namespace %q", r.ObjectNamespace),
Message: fmt.Sprintf(
"ref type not linked to its object with ID %q in Properties; scope with namespace %q was not applied successfully",
r.IDValue, r.ObjectNamespace),
})
}
return r.referencedObjectCache.Properties()
Expand All @@ -56,7 +53,9 @@ func (r *RefSchema) Properties() map[string]*PropertySchema {
func (r *RefSchema) GetDefaults() map[string]any {
if r.referencedObjectCache == nil {
panic(BadArgumentError{
Message: fmt.Sprintf("GetDefaults was called before ApplyScope with namespace %q", r.ObjectNamespace),
Message: fmt.Sprintf(
"ref type not linked to its object with ID %q in GetDefaults; scope with namespace %q was not applied successfully",
r.IDValue, r.ObjectNamespace),
})
}
return r.referencedObjectCache.GetDefaults()
Expand All @@ -69,7 +68,9 @@ func (r *RefSchema) TypeID() TypeID {
func (r *RefSchema) GetObject() Object {
if r.referencedObjectCache == nil {
panic(BadArgumentError{
Message: fmt.Sprintf("GetObject was called before ApplyScope with namespace %q", r.ObjectNamespace),
Message: fmt.Sprintf(
"ref type not linked to its object with ID %q in GetObject; scope with namespace %q was not applied successfully",
r.IDValue, r.ObjectNamespace),
})
}
return r.referencedObjectCache
Expand All @@ -78,7 +79,9 @@ func (r *RefSchema) GetObject() Object {
func (r *RefSchema) ReflectedType() reflect.Type {
if r.referencedObjectCache == nil {
panic(BadArgumentError{
Message: fmt.Sprintf("ReflectedType was called before ApplyScope with namespace %q", r.ObjectNamespace),
Message: fmt.Sprintf(
"ref type not linked to its object with ID %q in ReflectedType; scope with namespace %q was not applied successfully",
r.IDValue, r.ObjectNamespace),
})
}
return r.referencedObjectCache.ReflectedType()
Expand Down Expand Up @@ -107,25 +110,27 @@ func (r *RefSchema) ApplyScope(scope Scope, namespace string) {
}

func (r *RefSchema) ValidateReferences() error {
if r.referencedObjectCache == nil {
return BadArgumentError{
Message: fmt.Sprintf(
"Ref object reference could not find an object with ID %q in namespace %q",
r.IDValue,
r.ObjectNamespace,
),
}
if r.referencedObjectCache != nil {
return nil // Success
}
// The only way, unless there is a bug, for it to get here is if ApplyScope was not called with the
// correct namespace, or if the code disregards the error returned by ApplyScope. ApplyScope should
// always set referencedObjectCache or return an error if the correct namespace is applied.
return BadArgumentError{
Message: fmt.Sprintf(
"Ref object reference missing its link to object with ID %q in namespace %q. Is that a valid namespace?",
r.IDValue,
r.ObjectNamespace,
),
}

return nil
}

func (r *RefSchema) Unserialize(data any) (any, error) {
if r.referencedObjectCache == nil {
panic(BadArgumentError{
Message: fmt.Sprintf(
"Unserialize called before ApplyScope. Did you add your RefType to a scope with the namespace %q?",
r.ObjectNamespace,
"ref type not linked to its object with ID %q in Unserialize; scope with namespace %q was not applied successfully",
r.IDValue, r.ObjectNamespace,
),
})
}
Expand All @@ -136,8 +141,8 @@ func (r *RefSchema) Validate(data any) error {
if r.referencedObjectCache == nil {
panic(BadArgumentError{
Message: fmt.Sprintf(
"Validate called before ApplyScope. Did you add your RefType to a scope with the namespace %q?",
r.ObjectNamespace,
"ref type not linked to its object with ID %q in Validate; scope with namespace %q was not applied successfully",
r.IDValue, r.ObjectNamespace,
),
})
}
Expand All @@ -148,8 +153,8 @@ func (r *RefSchema) ValidateCompatibility(typeOrData any) error {
if r.referencedObjectCache == nil {
panic(BadArgumentError{
Message: fmt.Sprintf(
"ValidateCompatibility called before ApplyScope. Did you add your RefType to a scope with the namespace %q?",
r.ObjectNamespace,
"ref type not linked to its object with ID %q in ValidateCompatibility; scope with namespace %q was not applied successfully",
r.IDValue, r.ObjectNamespace,
),
})
}
Expand All @@ -164,8 +169,8 @@ func (r *RefSchema) Serialize(data any) (any, error) {
if r.referencedObjectCache == nil {
panic(BadArgumentError{
Message: fmt.Sprintf(
"Serialize called before ApplyScope. Did you add your RefType to a scope with the namespace %q?",
r.ObjectNamespace,
"ref type not linked to its object with ID %q in Serialize; scope with namespace %q was not applied successfully",
r.IDValue, r.ObjectNamespace,
),
})
}
Expand Down
131 changes: 78 additions & 53 deletions schema/scope_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,54 +318,74 @@ func TestSelfSerialization(t *testing.T) {
func TestApplyingExternalNamespace(t *testing.T) {
// This test tests applying a scope to a schema that contains scopes,
// properties, objects, maps, and lists.
// The applied scope must be passed through all of those types, validating
// that the scope gets applied all the way down and that errors are propagated up.
var testScope = schema.NewScopeSchema(
schema.NewObjectSchema(
"scopeTestObjectA",
map[string]*schema.PropertySchema{
"ref-b": schema.NewPropertySchema(
schema.NewNamespacedRefSchema("scopeTestObjectB", "test-namespace", nil),
nil,
true,
nil,
nil,
nil,
nil,
nil,
),
"list-type": schema.NewPropertySchema(
schema.NewListSchema(
schema.NewNamespacedRefSchema("scopeTestObjectB", "test-namespace", nil),
nil,
nil,
),
nil,
true,
nil,
nil,
nil,
nil,
nil,
),
"map-type": schema.NewPropertySchema(
schema.NewMapSchema(
schema.NewIntSchema(nil, nil, nil),
schema.NewNamespacedRefSchema("scopeTestObjectB", "test-namespace", nil),
nil,
nil,
),
nil,
true,
nil,
nil,
nil,
nil,
nil,
),
},
// The applied scope must be passed down to all of those types, validating
// that the scope gets applied down and that errors are propagated up.
refProperty := schema.NewPropertySchema(
schema.NewNamespacedRefSchema("scopeTestObjectB", "test-namespace", nil),
nil,
true,
nil,
nil,
nil,
nil,
nil,
)
listProperty := schema.NewPropertySchema(
schema.NewListSchema(
schema.NewNamespacedRefSchema("scopeTestObjectB", "test-namespace", nil),
nil,
nil,
),
nil,
true,
nil,
nil,
nil,
nil,
nil,
)
mapProperty := schema.NewPropertySchema(
schema.NewMapSchema(
schema.NewIntSchema(nil, nil, nil),
schema.NewNamespacedRefSchema("scopeTestObjectB", "test-namespace", nil),
nil,
nil,
),
nil,
true,
nil,
nil,
nil,
nil,
nil,
)
testScopes := map[string]*schema.ScopeSchema{
"withRef": schema.NewScopeSchema(
schema.NewObjectSchema(
"scopeTestObjectA",
map[string]*schema.PropertySchema{
"ref-b": refProperty,
},
),
),
"withList": schema.NewScopeSchema(
schema.NewObjectSchema(
"scopeTestObjectA",
map[string]*schema.PropertySchema{
"list-type": listProperty,
},
),
),
"withMap": schema.NewScopeSchema(
schema.NewObjectSchema(
"scopeTestObjectA",
map[string]*schema.PropertySchema{
"map-type": mapProperty,
},
),
),
}

var externalScope = schema.NewScopeSchema(
schema.NewObjectSchema(
"scopeTestObjectB",
Expand All @@ -383,11 +403,16 @@ func TestApplyingExternalNamespace(t *testing.T) {
},
),
)
// Not applied yet
err := testScope.ValidateReferences()
assert.Error(t, err)
assert.Contains(t, err.Error(), "could not find an object")
testScope.ApplyScope(externalScope, "test-namespace")
// Now it's applied, so the error should be resolved.
assert.NoError(t, testScope.ValidateReferences())
for testName, tc := range testScopes {
testScope := tc
t.Run(testName, func(t *testing.T) {
// Not applied yet
err := testScope.ValidateReferences()
assert.Error(t, err)
assert.Contains(t, err.Error(), "missing its link")
testScope.ApplyScope(externalScope, "test-namespace")
// Now it's applied, so the error should be resolved.
assert.NoError(t, testScope.ValidateReferences())
})
}
}
3 changes: 1 addition & 2 deletions schema/string.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ func (s StringSchema) Pattern() *regexp.Regexp {
return s.PatternValue
}

func (s StringSchema) ApplyScope(scope Scope, namespace string) {
}
func (s StringSchema) ApplyScope(_ Scope, _ string) {}

func (s StringSchema) ValidateReferences() error {
// No references in this type. No work to do.
Expand Down

0 comments on commit 9b551cb

Please sign in to comment.