Swagger-ui: Using 'properties' property name in Model results in it not showing up correctly on the UI models

[saarivirtajCGI]

| Q | A
| ------------------------------- | -------
| Bug or feature request? | Bug
| Which Swagger/OpenAPI version? | 2.0
| Which Swagger-UI version? | 3.10.0
| How did you install Swagger-UI? | Downloaded latest source package, copied dist/ files
| Which browser & version? | Chromium
| Which operating system? | Windows 10

Demonstration API definition

When you have a property that is named "properties" the UI doesn't seem to respond well to it.

This can be verified by setting a property's name "properties". I've included sample swagger.json files for inspection:

swagger.json with 'properties'
swagger.json with 'properties' renamed to 'xxx'

When the 'properties' is renamed to 'xxx', everything then works as expected.

This visual diff shows the only difference in the spec:
https://i.imgur.com/cGvZOl4.png

Expected Behavior

Should display the models correctly, like this (see 'xxx' / ProductModel):
https://i.imgur.com/uYW1huI.png
https://i.imgur.com/HTViGLA.png

Current Behavior

Apparently because the property is called 'properties', the UI gets confused, and this is what we see:
https://i.imgur.com/IM3oxDw.png
https://i.imgur.com/sc06JKU.png

The property completely disappears from the model, and the type 'Props' is not resolved in the ProductModel, even though it's clearly defined.

Context

Having a property called 'properties' shouldn't break things in my opinion.

[heldersepu]

I just added one and I did not find any problem:
http://swashbuckletest.azurewebsites.net/swagger/ui/index#/Company/Company_Post

[saarivirtajCGI]

@heldersepu Try that UI with these two swagger jsons:

https://js-webapptest.azurewebsites.net/swag-working.txt
https://js-webapptest.azurewebsites.net/swag-notworking.txt

[heldersepu]

What I was trying to proof is that is not just using 'properties' property name in Model...
Like my sample shows it worked on that case, There might be more to this issue.

[webron]

Huh, that's kinda funny (okay, maybe not for you, @saarivirtajCGI). It looks like it's when there's a $ref for the value...

[hctv19]

Has there been any progress made on this bug, or is there a known workaround for it? This one's biting us as well.

[thePanz]

I guess the same issue has been reported here: https://github.com/swagger-api/swagger-ui/issues/3091

[heldersepu]

@hctv19 I think the workaround here is obvious: Don't name your property "properties"

[epet]

@shockey, thanks for picking this bug up. When do you think this might land on your inner radar circles as we just had a customer report on this issue.

[heldersepu]

@shockey I think the correction is simple, we just need to add it to the freelyNamedPaths:
https://github.com/swagger-api/swagger-js/blob/master/src/specmap/helpers.js

[epet]

Hey @shockey, if @heldersepu is correct, I'm happy to send that PR?

[heldersepu]

@epet Sure if you have time send a PR, you don't have to ask permission for that...

but make sure to include some unit tests, look at the history of that file I added something not too long ago and I included tests, you can use that as a starting point

[shockey]

@epet - yeah, by all means, open a PR 😄

[epet]

This is probably my lack of understanding nuances of the codebase but I put this test in my change and in the current master, it passes in both versions:

    it('should ignore $refs in freely named Swagger nested positions.', function () {
      return mapSpec({
        spec: {
          a: 1234,
          parameters: {
            $ref: '#/a'
          },
          responses: {
            $ref: '#/a'
          },
          definitions: {
            $ref: '#/a'
          },
          securityDefinitions: {
            $ref: '#/a'
          },
          properties: {
            properties: {
              $ref: '#/a'
            }
          },
          foo: {
            properties: {
              $ref: '#/a'
            }
          }
        },
        plugins: [refs],
      }).then((res) => {
        expect(res.spec).toEqual({
          a: 1234,
          parameters: {
            $ref: '#/a'
          },
          responses: {
            $ref: '#/a'
          },
          definitions: {
            $ref: '#/a'
          },
          securityDefinitions: {
            $ref: '#/a'
          },
          properties: {
            properties: {
              $ref: '#/a'
            }
          },
          foo: {
            properties: {
              $ref: '#/a'
            }
          }
        })
      })
    })

I'm not sure this is solving a problem. @heldersepu or @shockey, advice for better tests or validation for this scenario?

[heldersepu]

Bamm. Did the problem got fix?

@epet Maybe your unit-test does not actually test the problem, I remember that the first time around I was not able to reproduce, maybe your are on the same boat.

[heldersepu]

I just 2check the "notworking.txt" example and the issue still present:
http://petstore.swagger.io/?docExpansion=list&defaultModelsExpandDepth=2&url=https://js-webapptest.azurewebsites.net/swag-notworking.txt

[epet]

being a managed code developer, I'm ramping up with the javascript environments. I finally got my changes setup in a local branch with the swagger-ui. Adding 'properties' to the freelyNamedPaths does not solve the issue we are seeing. Open to other ideas or I will pass to let someone with more depth in this codebase fix it.

[heldersepu]

@epet I feel your pain!

Most of my experience is from managed code as well (.net w/ Visual Studio) one thing that I really miss is the ability to debug UnitTests, cases like this are perfect for a TDD approach, we create a test case reproducing the behavior and on VS we just set a few break-points & start debugging...

But so far I have not seen any tool that can help break the VS addiction.

[chris922]

Just had the same issue after updating from an older version (I think it was something like 3.0.x) to the newest 3.16.0. Because I had this issue in the Swagger editor as well I found #3376 and thought that the newest Swagger UI shouldn't have this issue.. unfortunately after the upgrade I noticed that this issue is back :cry:

Really looking forward for a fix as we are not able to rename the property from properties to something else and thus I have to downgrade again to the old version we had before.

[avb-qlik]

Same issue I'm facing with Swagger editor live demo here https://editor.swagger.io

[thePanz]

Any updates here?

[dalbrx]

The following change in https://github.com/swagger-api/swagger-js seems to fix the issue. However I am not sure if that doesn't arise new problems.

diff --git a/src/specmap/helpers.js b/src/specmap/helpers.js
index 6b3c78a..8ab4480 100644
--- a/src/specmap/helpers.js
+++ b/src/specmap/helpers.js
@@ -36,10 +36,11 @@ const freelyNamedAncestors = [

 export function isFreelyNamed(parentPath) {
   const parentKey = parentPath[parentPath.length - 1]
+  const parentParentKey = parentPath[parentPath.length - 2]
   const parentStr = parentPath.join('/')

-  return (
-    (freelyNamedKeyParents.indexOf(parentKey) > -1) ||
+    return (
+    ((freelyNamedKeyParents.indexOf(parentKey) > -1) && !(freelyNamedKeyParents.indexOf(parentParentKey) > -1)) ||
     (freelyNamedPaths.indexOf(parentStr) > -1) ||
     (freelyNamedAncestors.some(el => parentStr.indexOf(el) > -1))
   )