fix(oas): user data + server variable quirks (#848)

* fix(oas): fallback to user object for default vars

* fix: supplied vars should take precedence over user data in `replaceUrl`

* test: more coverage for the fix in 6e89c47

packages/oas/src/index.ts

@@ -330,7 +330,7 @@ export default class Oas {
 
   url(selected = 0, variables?: Variables) {
     const url = normalizedUrl(this.api, selected);
-    return this.replaceUrl(url, variables || this.variables(selected)).trim();
+    return this.replaceUrl(url, variables || this.defaultVariables(selected)).trim();
   }
 
   variables(selected = 0) {
@@ -439,12 +439,13 @@ export default class Oas {
    *
    * There are a couple ways that this will utilize variable data:
    *
-   *  - If data is stored in `this.user` and it matches up with the variable name in the URL user
-   *    data will always take priority. See `getUserVariable` for some more information on how this
-   *    data is pulled from `this.user`.
-   *  - Supplying a `variables` object. This incoming `variables` object can be two formats:
+   *  - Supplying a `variables` object. If this is supplied, this data will always take priority.
+   *    This incoming `variables` object can be two formats:
    *    `{ variableName: { default: 'value' } }` and `{ variableName: 'value' }`. If the former is
-   *    present, that will take prescendence over the latter.
+   *    present, that will take precedence over the latter.
+   *  - If the supplied `variables` object is empty or does not match the current template name,
+   *    we fallback to the data stored in `this.user` and attempt to match against that.
+   *    See `getUserVariable` for some more information on how this data is pulled from `this.user`.
    *
    * If no variables supplied match up with the template name, the template name will instead be
    * used as the variable data.
@@ -457,11 +458,6 @@ export default class Oas {
     // lookups, so if we have one here on, slice it off.
     return stripTrailingSlash(
       url.replace(SERVER_VARIABLE_REGEX, (original: string, key: string) => {
-        const userVariable = getUserVariable(this.user, key);
-        if (userVariable) {
-          return userVariable as string;
-        }
-
         if (key in variables) {
           const data = variables[key];
           if (typeof data === 'object') {
@@ -473,6 +469,11 @@ export default class Oas {
           }
         }
 
+        const userVariable = getUserVariable(this.user, key);
+        if (userVariable) {
+          return userVariable as string;
+        }
+
         return original;
       }),
     );

packages/oas/src/lib/get-user-variable.ts

@@ -19,5 +19,5 @@ export default function getUserVariable(user: RMOAS.User, property: string, sele
     }
   }
 
-  return key[property] || null;
+  return key[property] || user[property] || null;
 }

packages/oas/test/index.test.ts

@@ -168,6 +168,46 @@ describe('#url([selected])', () => {
       ).toBe('https://domh.example.com');
     });
 
+    it('should use user variables over defaults even if user payload has keys array', () => {
+      expect(
+        new Oas(
+          {
+            openapi: '3.0.0',
+            info: {
+              title: 'testing',
+              version: '1.0.0',
+            },
+            servers: [{ url: 'https://{username}.example.com', variables: { username: { default: 'demo' } } }],
+            paths: {},
+          },
+          {
+            username: 'domh',
+            keys: [
+              { apiKey: '123456', name: 'app-1' },
+              { apiKey: '7890', name: 'app-2' },
+            ],
+          },
+        ).url(),
+      ).toBe('https://domh.example.com');
+    });
+
+    it('should use supplied variables over user variables', () => {
+      expect(
+        new Oas(
+          {
+            openapi: '3.0.0',
+            info: {
+              title: 'testing',
+              version: '1.0.0',
+            },
+            servers: [{ url: 'https://{username}.example.com', variables: { username: { default: 'demo' } } }],
+            paths: {},
+          },
+          { username: 'domh' },
+        ).url(0, { username: 'owlbert' }),
+      ).toBe('https://owlbert.example.com');
+    });
+
     it('should fetch user variables from keys array', () => {
       expect(
         new Oas(

packages/oas/test/lib/get-user-variable.test.ts

@@ -26,6 +26,18 @@ test('should return first item from keys array if no app selected', () => {
   expect(getUserVariable(keysUser, 'apiKey')).toBe('123456');
 });
 
+test('should grab item from keys array in combined object', () => {
+  expect(getUserVariable({ ...topLevelUser, ...keysUser }, 'apiKey')).toBe('123456');
+});
+
+test('should grab item from keys array with specific app name in combined object', () => {
+  expect(getUserVariable({ ...topLevelUser, ...keysUser }, 'apiKey', 'app-2')).toBe('7890');
+});
+
+test('should fall back to top-level item if not present in keys array', () => {
+  expect(getUserVariable({ ...topLevelUser, ...keysUser }, 'user')).toBe('user');
+});
+
 test('should return selected app from keys array if app provided', () => {
   expect(getUserVariable(keysUser, 'apiKey', 'app-2')).toBe('7890');
 });