Revert fix go-openapi#146

A more thorough investigation on loading specs from a generated server
from paths containing special characters (such as blank spaces,
parenthesis) did not conclude that it was necessary to alter how cache
entries are matched.

Indeed, this change deeply altered how keys in the cache are managed
and was therefore at best, a risky bet.

* reverted commit e8e27ff
* added check to ensure that the root pseudo-document is preloaded in
  cache under all conditions
* added more unit tests to check the behavior of the normalizers against
  escaped characters
* closes go-openapi#145

Signed-off-by: Frederic BIDON <[email protected]>

.gitignore

@@ -1,2 +1 @@
-secrets.yml
-coverage.out
+*.out

expander.go

@@ -102,15 +102,21 @@ const rootBase = ".root"
 
 // baseForRoot loads in the cache the root document and produces a fake ".root" base path entry
 // for further $ref resolution
-//
-// Setting the cache is optional and this parameter may safely be left to nil.
 func baseForRoot(root interface{}, cache ResolutionCache) string {
+	// cache the root document to resolve $ref's
+	normalizedBase := normalizeBase(rootBase)
+
 	if root == nil {
-		return ""
+		// ensure that we never leave a nil root: always cache the root base pseudo-document
+		cachedRoot, found := cache.Get(normalizedBase)
+		if found && cachedRoot != nil {
+			// the cache is already preloaded with a root
+			return normalizedBase
+		}
+
+		root = map[string]interface{}{}
 	}
 
-	// cache the root document to resolve $ref's
-	normalizedBase := normalizeBase(rootBase)
 	cache.Set(normalizedBase, root)
 
 	return normalizedBase

expander_test.go

@@ -1098,6 +1098,53 @@ func TestExpand_ExtraItems(t *testing.T) {
 			 }`, jazon)
 }
 
+func TestExpand_Issue145(t *testing.T) {
+	cwd, err := os.Getwd()
+	require.NoError(t, err)
+	pseudoRoot := normalizeBase(filepath.Join(cwd, rootBase))
+
+	// assert the internal behavior of baseForRoot()
+	t.Run("with nil root, empty cache", func(t *testing.T) {
+		cache := defaultResolutionCache()
+		require.Equal(t, pseudoRoot, baseForRoot(nil, cache))
+
+		t.Run("empty root is cached", func(t *testing.T) {
+			value, ok := cache.Get(pseudoRoot)
+			require.True(t, ok) // found in cache
+			asMap, ok := value.(map[string]interface{})
+			require.True(t, ok)
+			require.Empty(t, asMap)
+		})
+	})
+
+	t.Run("with non-nil root, empty cache", func(t *testing.T) {
+		cache := defaultResolutionCache()
+		require.Equal(t, pseudoRoot, baseForRoot(map[string]interface{}{"key": "arbitrary"}, cache))
+
+		t.Run("non-empty root is cached", func(t *testing.T) {
+			value, ok := cache.Get(pseudoRoot)
+			require.True(t, ok) // found in cache
+			asMap, ok := value.(map[string]interface{})
+			require.True(t, ok)
+			require.Contains(t, asMap, "key")
+			require.Equal(t, "arbitrary", asMap["key"])
+		})
+
+		t.Run("with nil root, non-empty cache", func(t *testing.T) {
+			require.Equal(t, pseudoRoot, baseForRoot(nil, cache))
+
+			t.Run("non-empty root is kept", func(t *testing.T) {
+				value, ok := cache.Get(pseudoRoot)
+				require.True(t, ok) // found in cache
+				asMap, ok := value.(map[string]interface{})
+				require.True(t, ok)
+				require.Contains(t, asMap, "key")
+				require.Equal(t, "arbitrary", asMap["key"])
+			})
+		})
+	})
+}
+
 // PetStore20 json doc for swagger 2.0 pet store
 const PetStore20 = `{
   "swagger": "2.0",

fixtures/bugs/145/Program Files (x86)/AppName/ref.json

@@ -0,0 +1,17 @@
+{
+"definitions": {
+    "todo-partial": {
+      "title": "Todo Partial",
+      "type": "object",
+      "properties": {
+        "name": {
+          "type": "string"
+        },
+        "completed": {
+          "type": ["boolean", "null"]
+        }
+      },
+      "required": ["name", "completed"]
+    }
+  }
+}

fixtures/bugs/145/Program Files (x86)/AppName/todos.common.json

@@ -0,0 +1,103 @@
+{
+  "swagger": "2.0",
+  "info": {
+    "version": "1.0",
+    "title": "To-do Demo",
+    "description":
+      "### Notes:\n\nThis OAS2 (Swagger 2) specification defines common models and responses, that other specifications may reference.\n\nFor example, check out the user poperty in the main.oas2 todo-partial model - it references the user model in this specification!\n\nLikewise, the main.oas2 operations reference the shared error responses in this common specification.",
+    "contact": {
+      "name": "Stoplight",
+      "url": "https://stoplight.io"
+    },
+    "license": {
+      "name": "MIT"
+    }
+  },
+  "host": "example.com",
+  "securityDefinitions": {},
+  "paths": {},
+  "responses": {
+    "401": {
+      "description": "",
+      "schema": {
+        "$ref": "#/definitions/error-response"
+      },
+      "examples": {
+        "application/json": {
+          "status": "401",
+          "error": "Not Authorized"
+        }
+      }
+    },
+    "403": {
+      "description": "",
+      "schema": {
+        "$ref": "#/definitions/error-response"
+      },
+      "examples": {
+        "application/json": {
+          "status": "403",
+          "error": "Forbbiden"
+        }
+      }
+    },
+    "404": {
+      "description": "",
+      "schema": {
+        "$ref": "#/definitions/error-response"
+      },
+      "examples": {
+        "application/json": {
+          "status": "404",
+          "error": "Not Found"
+        }
+      }
+    },
+    "500": {
+      "description": "",
+      "schema": {
+        "$ref": "#/definitions/error-response"
+      },
+      "examples": {
+        "application/json": {
+          "status": "500",
+          "error": "Server Error"
+        }
+      }
+    }
+  },
+  "definitions": {
+    "user": {
+      "title": "User",
+      "type": "object",
+      "properties": {
+        "name": {
+          "type": "string",
+          "description": "The user's full name."
+        },
+        "age": {
+          "type": "number",
+          "minimum": 0,
+          "maximum": 150
+        },
+        "error": {
+          "$ref": "#/definitions/error-response"
+        }
+      },
+      "required": ["name", "age"]
+    },
+    "error-response": {
+      "type": "object",
+      "title": "Error Response",
+      "properties": {
+        "status": {
+          "type": "string"
+        },
+        "error": {
+          "type": "string"
+        }
+      },
+      "required": ["status", "error"]
+    }
+  }
+}