feat(repository-json-schema): add title to filter

[derdeka]

Suggestion

Add title property to filter schema definition in order to enable better code generation by third party tools such as https://github.com/OpenAPITools/openapi-generator .

Without a title property the generator simply name the filter as Filter1, Filter2, etc. By adding the title property, the tooling can produce a filter interface named after title.

Use Cases

Parameters of controller methods are decorated with @param.query.object where getFilterSchemaFor(User) generates a JsonSchema for the filter.

@param.query.object('filter', getFilterSchemaFor(User)) filter?: Filter<User>,

When using openapi-generator, this filter gets just named Filter1. With a large number of filters in a project and randomized order of generation, the git history of the generated files gets trashed.

Discussion points

One filter getFilterSchemaFor(ModelX) can be used multiple times in a project. Currently the filters are created inline. Simply adding a title like ModelXFilter creates name collisions when generating the client sdk. Is it helpful to move Filters to /components/schemas ?

Acceptance criteria

TBD - will be filled by the team.

[dougal83]

+1 for refactoring the filter object. Thank you for looking further into this!

[bajtos]

@derdeka thank you for a great suggestion! I agree we should add title to the schema generated for model filters.

One filter getFilterSchemaFor(ModelX) can be used multiple times in a project. Currently the filters are created inline. Simply adding a title like ModelXFilter creates name collisions when generating the client sdk. Is it helpful to move Filters to /components/schemas ?

I see two options (but there may be more):

Short-term, we can use the approach we have in place for model schemas - instead of emitting an "inline" schema, we generate a schema reference and include the target schema in definitions property. Take a look at getModelSchemaRef works under the hood, we can build getFilterSchemaRefFor on a similar way. I find this approach as not very pretty, but it makes the implementation of the requested feature quick & easy.

For longer term, I would like our REST layer to be smarter and able to automatically extract schemas used in multiple places into /components/schemas and replace them with $ref at usage points.

This logic can work as follows:

1. When building OpenAPI schema, we need to scan the entire OpenAPI spec tree and look for all known places where a schema object can be provided. Those places include endpoint parameters, request bodies, response schemas, and so one. I think it's ok to start with few commonly-used places first and add more of them later, as we learn more.
2. For each schema object encountered, if the object has a title, then add it to a look-up table (title -> schema) and increase the reference counter.
3. After we scanned the spec tree, iterate over all entries in the look-up table, pick entries with two or more references and move them to /components/schemas.
4. Scan the spec tree again, replace all schemas that were moved to /components/schemas in step 3 with $ref. (I think this could be optimized to avoid the second spec tree scan, but that could be done later.)

[bajtos]

@derdeka @dougal83 would you like to contribute this feature yourself?

[dougal83]

Just dumping research into possible patterns of references to consider. I'll look further depending on time available.

Type specific:

{
  "TodoFields": {
    "title": "TodoFields",
    "type": "object",
    "properties": {
      "id": {
        "type": "boolean"
      }
     // ...
    }
  },
  "TodoFilter": {
    "title": "TodoFilter",
    "type": "object",
    "properties": {
      "where": {
        "type": "object"
      },
      "fields": {
        "$ref": "#/components/schemas/TodoFields"
      },
      "offset": {
        "minimum": 0,
        "type": "integer",
        "format": "int32"
      },
      "limit": {
        "minimum": 1,
        "type": "integer",
        "format": "int32",
        "example": 100
      },
      "skip": {
        "minimum": 0,
        "type": "integer",
        "format": "int32"
      },
      "order": {
        "type": "array",
        "items": {
          "type": "string"
        },
        "description": ""
      },
      "include": {
        "type": "array",
        "items": {
          "$ref": "#/components/schemas/Include"
        },
        "description": ""
      }
    }
  }

Generic:

  "Include": {
    "title": "Include",
    "type": "object",
    "properties": {
      "relation": {
        "type": "string"
      },
      "scope": {
        "$ref": "#/components/schemas/Scope"
      }
    }
  },
  "Scope": {
    "title": "Scope",
    "type": "object",
    "properties": {
      "where": {
        "type": "object"
      },
      "fields": {
        "type": "object"
      },
      "offset": {
        "minimum": 0,
        "type": "integer",
        "format": "int32"
      },
      "limit": {
        "minimum": 1,
        "type": "integer",
        "format": "int32",
        "example": 100
      },
      "skip": {
        "minimum": 0,
        "type": "integer",
        "format": "int32"
      },
      "order": {
        "type": "array",
        "items": {
          "type": "string"
        },
        "description": ""
      }
    }
  }
}

[achrinza]

A separate issue has been created to work on refactoring common filter schemas: #4328

[dougal83]

@bajtos I'll give the REST layer improvement a crack. Will wait for review on #4355 to see if all/part of the pull is required in your estimation.

@jannyHou Hey, would #4258 be a good avenue to implement improvements to rest layer as per @bajtos comments on improving the rest layer? Suggestions welcome!

For longer term, I would like our REST layer to be smarter and able to automatically extract schemas used in multiple places into /components/schemas and replace them with $ref at usage points.

[jannyHou]

@dougal83 I am glad that you are aware of the oas enhancer service and agree that this is a good chance to try it 👍

After story #4258, we are going to add an OASEnhancerService in the rest server, as how it has other properties like _requestHandler.

I think function _serveOpenApiSpec is the place where you can run await this.getOASEnhancerService.applyEnhancerByName('consolidate-schema-objects'); to organize duplicate schema objects into component.schemas and replace the inline one with the reference.

And to create the enhancer, you can follow the example in adding a new OAS enhancer, it should be pretty simple, the enhancer has a function called modifySpec(), which takes in the current spec, modifies it(your gist code), and returns a new one.

And since restserver extends context, theoretically you can bind the extensionpoint(oasEnhancerService) and extensions(enhancers) like how the example binds them to the app, but I am not sure if you can add binding in the restserver's constructor. Let me know if you have any questions.