Skip to content
This repository has been archived by the owner on Aug 4, 2023. It is now read-only.

Unable to restrict model properties when using allOf #450

Open
pospi opened this issue Nov 10, 2016 · 8 comments · May be fixed by #453
Open

Unable to restrict model properties when using allOf #450

pospi opened this issue Nov 10, 2016 · 8 comments · May be fixed by #453

Comments

@pospi
Copy link

pospi commented Nov 10, 2016

I am finding it impossible to limit the properties which can be sent in an object when using schema composition. By way of example, let's say we have an object with id, name and value properties. The id field is only set for records once they are created.

I would like to be able to use the same field definitions in all my schemas for consistency as I would have done with standard JSON Schema. To do this I usually create multiple partials and then reference them in my model schemas, like this:

definitions:

  with_id:
    type: object
    properties:
      id:
        type: string
        format: uuid

  record_data:
    type: object
    properties:
      name:
        type: string
      value:
        type: number

  new_record:
    allOf:
    - "$ref": "#/definitions/record_data"
    properties:
      name: {}
      value: {}
    required:
    - name
    - value
    additionalProperties: false

  existing_record:
    allOf:
    - "$ref": "#/definitions/with_id"
    - "$ref": "#/definitions/record_data"
    properties:
      id: {}
      name: {}
      value: {}
    required:
    - id
    - name
    - value
    additionalProperties: false

Note that I need to redeclare the child record properties as properties of the toplevel record in order to workaround JSON Schema's limitations regarding additionalProperties. This works for JSON Schema, but with Swagger's validator I get a CHILD_DEFINITION_REDECLARES_PROPERTY error.

Is there a good reason for including this error? Can it be removed, or perhaps only enabled when validating the higher-level Swagger schema & ignored when validating JSON Schema model definitions?

@whitlockjc
Copy link
Member

This might be a relic of the Swagger 1.2 support where this was specifically forbidden. Let me ping @webron to see if he can help and I'll also use that to check in with him to see if Swagger 2.0 forbids children models from redeclaring the properties of their ancestors.

@webron
Copy link

webron commented Nov 10, 2016

I don't believe there's a technical problem redefining properties, but the snippet is showing an invalid JSON Schema, as far as I can tell.

    properties:
      name:
      value:

does not translate well.

@pospi
Copy link
Author

pospi commented Nov 11, 2016

Ah, true. YAML may not let you express an empty object definition. It was a contrived example, I'm actually adding mine programatically like this so I don't have to manually replicate properties:

import { curry } from 'ramda';
import SwaggerParser from 'swagger-parser';

function mapObject(obj: Object, cb: (v: any, k: string, obj: Object) => any) {
  Object.keys(obj).forEach(k => {
    obj[k] = cb(obj[k], k, obj);
  });
}

// @see http://stackoverflow.com/questions/22689900/ddg#23001194
const createToplevelProperties = curry(($refs, def) => {
  if (def.additionalProperties !== false) {
    return def;
  }

  const extras = (def.allOf || []).concat(def.anyOf || []).concat(def.oneOf || []);
  if (!extras.length) {
    return def;
  }
  if (!def.properties) {
    def.properties = {};
  }

  extras.forEach(schema => {
    if (schema.properties) {
      Object.keys(schema.properties).forEach(prop => (def.properties[prop] = {}));
    }
    if (schema.$ref) {
      const reference = $refs.get($refs.paths()[0] + schema.$ref);
      if (reference.properties) {
        Object.keys(reference.properties).forEach(prop => (def.properties[prop] = {}));
      }
    }
  });

  return def;
});

const loadSchema = (filePath: string) => {
  return Promise.all([
    SwaggerParser.bundle(filePath),
    SwaggerParser.resolve(filePath),
  ]).then(([schema, $refs]) => {
      mapObject(schema.definitions, createToplevelProperties($refs));
      return schema;
    });
};

export default loadSchema;

@webron
Copy link

webron commented Nov 11, 2016

@pospi you can specify an empty object in YAML this way:

    properties:
      name: {}
      value: {}

@pospi
Copy link
Author

pospi commented Nov 22, 2016

@webron updated initial example to specify empty objects properly for acceptance testing (: Have also pushed up removal of the check in swagger-tools to continue the conversation..

@whitlockjc
Copy link
Member

I remember where this feature came from. Back in Swagger 1.2.x, this was in the documentation:

A sub-model definition MUST NOT override the properties of any of its ancestors. All
sub-models MUST be defined in the same API Declaration.

So this is likely a 1.2.x feature that ended up in 2.0.x by mistake. I can remove this from 2.0.x once @webron has a chance to comment with this new information.

@webron
Copy link

webron commented Nov 22, 2016

@whitlockjc I believe you're right. The restriction existed in 1.2 as we didn't really use JSON Schema back then, and that restriction is not relevant in 2.0.

@whitlockjc
Copy link
Member

Thanks for clarifying.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants