Loopback-next: 'title'-option of JsonSchemaOptions<T> causes wrong related class

[iLem0n]

Steps to reproduce

1. download minimum error example project: https://github.com/iLem0n/loopback-relation-error
2. npm install && npm run start
3. browse to http://localhost:3000/openapi.json or check the generated openapi-spec
4. Check 'components -> schema -> ParentWithChildren -> Properties -> children -> items -> $ref'

Current Behavior

the generated openapi3 spec has the parent class itself as the related class.
see: 'components -> schema -> ParentWithChildren -> Properties -> children -> items -> $ref'

"components": {
    "schemas": {
      "ParentWithChildren": {
        "title": "ParentWithChildren",
        "description": "(Schema options: { title: 'ParentWithChildren', includeRelations: true })",
        "properties": {
          "id": {
            "type": "string"
          },
          "name": {
            "type": "string"
          },
          "children": {
            "type": "array",
            "items": {
              "$ref": "#/components/schemas/ParentWithChildren" // <--- expect "#/components/schemas/ParentWithChildren"
            }
          }
        },
        "required": [
          "name"
        ],
        "additionalProperties": false
      }
    }
  }

Expected Behavior

the related class should be the class 'Children'

"components": {
    "schemas": {
      "ParentWithChildren": {
        "title": "ParentWithChildren",
        "description": "(Schema options: { title: 'ParentWithChildren', includeRelations: true })",
        "properties": {
          "id": {
            "type": "string"
          },
          "name": {
            "type": "string"
          },
          "children": {
            "type": "array",
            "items": {
              "$ref": "#/components/schemas/ParentWithChildren" // <--- expect "#/components/schemas/ParentWithChildren"
            }
          }
        },
        "required": [
          "name"
        ],
        "additionalProperties": false
      }
    }
  }

Link to reproduction sandbox

https://github.com/iLem0n/loopback-relation-error

Additional information

node -e 'console.log(process.platform, process.arch, process.versions.node)' --->

darwin x64 12.10.0

npm ls --prod --depth 0 | grep loopback --->

├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]

[iLem0n]

Hello @dougal83: Thanks for the quick reply. This would only lead to a 'Children' class with the structure (attributes) of a 'Parent'-Class. The $ref would be the right one, but the class naming would be wrong and misleading.
I would expect a class "ParentWithChildren" which has the parent structure (attributes of class 'Parent') and the resolved class of the items in the children's array should be 'Children'.

something like:

"ParentWithChildren": {
        "title": "ParentWithChildren",
        "description": "(Schema options: { title: 'ParentWithChildren', includeRelations: true })",
        "properties": {
          "id": {
            "type": "string"
          },
          "name": {
            "type": "string"
          },
          "children": {
            "type": "array",
            "items": {
              "$ref": "#/components/schemas/Children"
            }
          }
        },
        "required": [
          "name"
        ],
        "additionalProperties": false
      }

Or is there a missunderstanding from my side ?

[iLem0n]

Before I didn't used the 'title'-option but this leads into multiple classes with ugly naming:

without the title option the related class is generated the right way:

[dougal83]

@iLem0n Sorry, I deleted my post earlier as I didn't have time to follow up with anything coherent. The last image with ChildWithRelations is what I would expect to see. I think that the use of title was overriding the naming of the child.

Are you all good now?

[iLem0n]

@dougal83: No problem. This would be the right behaviour, yes. but this only works without the 'title'-option. The options brakes this behavior. In this screenshot you see two times the same configuration schema, the only difference is the title attribute. the second one also changes the items $ref

[iLem0n]

I've build a workaround by adding the schemas manually in the @api() decorator of the class.

```import { getModelSchemaRef } from "@loopback/rest";
import { JsonSchemaOptions, } from '@loopback/repository-json-schema';

export const defaultSchemaOptions = {
exclude: ['ownerId']
};

export function getDefaultModelSchemaRef(
modelCtor: Function & {prototype: T},
additionalOptions?: JsonSchemaOptions | null,
propertyOverrides?: {} | null
) {
const options = Object.assign({}, defaultSchemaOptions, additionalOptions);
const schemaRef = getModelSchemaRef(modelCtor, options);
const firstKey = Object.keys(schemaRef.definitions)[0];

const def = schemaRef.definitions[firstKey];

return {
...def,
properties: {
...def.properties,
...propertyOverrides
}
};
}

then, on the controller: 

@api({
basePath: '/plans',
paths: {},
components: {
schemas: {
"Plan": getDefaultModelSchemaRef(Plan, {
title: 'Plan'
}),
"PlanNew": getDefaultModelSchemaRef(Plan, {
title: 'PlanNew',
exclude: ['id']
}),
"PlanPartial": getDefaultModelSchemaRef(Plan, {
title: 'PlanPartial',
partial: true
}),
"PlanWithItems": getDefaultModelSchemaRef(Plan, {
title: 'PlanWithItems'
}, {
items: {
type: "array",
items: getModelSchemaRef(PlanItem)
}
})
}
},
})
export class PlanController { ... }
```

this actually works for me, but it is not the prettiest way to work with it.
This is just a workaround to mask the wrong behavior of the title-option.

UPDATE: just recognized that I can use this workaround directly in the endpoint spec. You don't have to put it in the @api() at the top.

[dougal83]

This is just a workaround to mask the wrong behaviour of the title-option.

Yeah, the behaviour of the title option is a bug. Thank you for highlighting it.

So to clarify, when applying the title option to parent schema, the included relations also have the parent's title applied, which is a bug.

[dougal83]

Hey @agnes512 Could you please take a look?

[iLem0n]

@dougal83: Yes that totally right. Thanks for your support.
@agnes512: Thank you too ;)

[dougal83]

Another example of broken relation : Need to remove title property to get expected behaviour

[jannyHou]

This line causes the problem:

modelToJsonSchema takes in the options(which has the title property) to build the response schema.
Since the Parent model has a relation property children, it builds child model's schema starting from line 499, which calls modelToJsonSchema again and passes the same options to it.

I am looking at the JsonSchemaOptions interface, I still think the title is a valid option field, while it shouldn't be passed to the recursive calls inside modelToJsonSchema by any means:
Building the schema for relation properties OR properties with custom type(need to double check the behavior for this use case).

Submitting a PR to fix it.

[jannyHou]

Created a PR in https://github.com/strongloop/loopback-next/pull/5108 for it.

[jannyHou]

should be fixed by https://github.com/strongloop/loopback-next/pull/5108, feel free to reopen it if you still have the problem.

[Svettis2k]

How about when the property is not a relation, but rather an embedded object (which is often the case when using MongoDB)?

Is there any harm in adding a "delete propOptions.title;" after line 475? This fixed the same problem as stated by the OP in our case (where the model is simply stored as part of the parent model).

Edit: After inspecting the schema I see that all nested objects are created with its own generated title (e.g. "XExcluding_id_"). I have not really studied the underlying code, but apparently there is something that is done with the title which does not work well with nested object properties which are not relations.

[jannyHou]

@Svettis2k Good catch. I think title in options should be deleted right after generating the title for the current schema: https://github.com/strongloop/loopback-next/blob/master/packages/repository-json-schema/src/build-schema.ts#L416.

Let me submit a PR for further discussion.

[jannyHou]

cc @Svettis2k PR created in https://github.com/strongloop/loopback-next/pull/5231 🙂 hope it fixes your problem.

[Svettis2k]

@jannyHou I don't think we're quite there yet. I see the reason for the EDIT in my previous post is due to exclude being passed on to the generation of nested properties as well. All the nested objects will then get a title on the form "XExcluding_id_" (given that the propOptions.exclude = ['id']). I would assume that the exclude option should only be used for the parent model?

It might be worth taking a look at which of the JSON schema options should be kept solely at the parent level (or perhaps which are required to be passed down)?