encoding/openapi: bug fixes in expandReference

- indirect the reference, but avoid premature
  expansion of a disjunction

- intercept infinite recursion so that a proper
  error message (with location) can be given.

- allow "type" to be unspecified. This was
  incorrectly assumed to be a required field.

Issue #56

Change-Id: Icc543cb626e3533305e73a0867a567c19daf8d2e
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/2682
Reviewed-by: Marcel van Lohuizen <[email protected]>

Author: mpvl

encoding/openapi/build.go

@@ -39,6 +39,7 @@ type buildContext struct {
 	nameFunc    func(inst *cue.Instance, path []string) string
 	descFunc    func(v cue.Value) string
 	fieldFilter *regexp.Regexp
+	evalDepth   int // detect cycles when resolving references
 
 	schemas *OrderedMap
 
@@ -142,11 +143,6 @@ func (c *buildContext) isInternal(name string) bool {
 	return strings.HasSuffix(name, "_value")
 }
 
-// shouldExpand reports is the given identifier is not exported.
-func (c *buildContext) shouldExpand(p *cue.Instance, ref []string) bool {
-	return c.expandRefs
-}
-
 func (b *builder) failf(v cue.Value, format string, args ...interface{}) {
 	panic(&openapiError{
 		errors.NewMessage(format, args),
@@ -187,13 +183,31 @@ func (b *builder) schema(name string, v cue.Value) *oaSchema {
 	return schema
 }
 
+func resolve(v cue.Value) cue.Value {
+	switch op, a := v.Expr(); op {
+	case cue.SelectorOp:
+		field, _ := a[1].String()
+		v = resolve(a[0]).Lookup(field)
+	}
+	return v
+}
+
 func (b *builder) value(v cue.Value, f typeFunc) (isRef bool) {
 	count := 0
 	disallowDefault := false
 	var values cue.Value
-	p, a := v.Reference()
-	if b.ctx.shouldExpand(p, a) {
-		values = v
+	if b.ctx.expandRefs {
+		// Cycles are not allowed when expanding references. Right now we just
+		// cap the depth of evaluation at 30.
+		// TODO: do something more principled.
+		const maxDepth = 30
+		if b.ctx.evalDepth > maxDepth {
+			b.failf(v, "maximum stack depth of %d reached", maxDepth)
+		}
+		b.ctx.evalDepth++
+		defer func() { b.ctx.evalDepth-- }()
+
+		values = resolve(v)
 		count = 1
 	} else {
 		dedup := map[string]bool{}
@@ -844,12 +858,10 @@ func (b *builder) setNot(key string, value interface{}) {
 func (b *builder) finish() *oaSchema {
 	switch len(b.allOf) {
 	case 0:
-		if b.typ == "" {
-			b.failf(cue.Value{}, "no type specified at finish")
-			return nil
-		}
 		t := &OrderedMap{}
-		setType(t, b)
+		if b.typ != "" {
+			setType(t, b)
+		}
 		return t
 
 	case 1:

encoding/openapi/openapi.go

@@ -49,7 +49,8 @@ type Generator struct {
 	FieldFilter string
 
 	// ExpandReferences replaces references with actual objects when generating
-	// OpenAPI Schema. It is an error for an CUE value to refer to itself.
+	// OpenAPI Schema. It is an error for an CUE value to refer to itself
+	// if this option is used.
 	ExpandReferences bool
 }
 

encoding/openapi/openapi_test.go

@@ -62,6 +62,10 @@ func TestParseDefinitions(t *testing.T) {
 		"oneof.cue",
 		"oneof.json",
 		defaultConfig,
+	}, {
+		"oneof.cue",
+		"oneof-resolve.json",
+		resolveRefs,
 	}, {
 		"openapi.cue",
 		"openapi.json",
@@ -107,7 +111,7 @@ func TestParseDefinitions(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			if d := diff.Diff(string(b), out.String()); d != "" {
+			if d := diff.Diff(out.String(), string(b)); d != "" {
 				t.Errorf("files differ:\n%v", d)
 			}
 		})

encoding/openapi/testdata/oneof-resolve.json

@@ -0,0 +1,114 @@
+{
+   "openapi": "3.0.0",
+   "info": {
+      "title": "test",
+      "version": "v1"
+   },
+   "components": {
+      "schemas": {
+         "MyString": {
+            "oneOf": [
+               {
+                  "type": "object",
+                  "required": [
+                     "exact"
+                  ],
+                  "properties": {
+                     "exact": {
+                        "type": "string",
+                        "format": "string"
+                     }
+                  }
+               },
+               {
+                  "type": "object",
+                  "required": [
+                     "regex"
+                  ],
+                  "properties": {
+                     "regex": {
+                        "type": "string",
+                        "format": "string"
+                     }
+                  }
+               }
+            ]
+         },
+         "MyInt": {
+            "type": "integer"
+         },
+         "Foo": {
+            "type": "object",
+            "required": [
+               "include",
+               "exclude",
+               "count"
+            ],
+            "properties": {
+               "include": {
+                  "oneOf": [
+                     {
+                        "type": "object",
+                        "required": [
+                           "exact"
+                        ],
+                        "properties": {
+                           "exact": {
+                              "type": "string",
+                              "format": "string"
+                           }
+                        }
+                     },
+                     {
+                        "type": "object",
+                        "required": [
+                           "regex"
+                        ],
+                        "properties": {
+                           "regex": {
+                              "type": "string",
+                              "format": "string"
+                           }
+                        }
+                     }
+                  ]
+               },
+               "exclude": {
+                  "type": "array",
+                  "items": {
+                     "oneOf": [
+                        {
+                           "type": "object",
+                           "required": [
+                              "exact"
+                           ],
+                           "properties": {
+                              "exact": {
+                                 "type": "string",
+                                 "format": "string"
+                              }
+                           }
+                        },
+                        {
+                           "type": "object",
+                           "required": [
+                              "regex"
+                           ],
+                           "properties": {
+                              "regex": {
+                                 "type": "string",
+                                 "format": "string"
+                              }
+                           }
+                        }
+                     ]
+                  }
+               },
+               "count": {
+                  "type": "integer"
+               }
+            }
+         }
+      }
+   }
+}