Skip to content
This repository was archived by the owner on Nov 8, 2024. It is now read-only.

Support array of type in Schema Object #600

Merged
merged 1 commit into from
Feb 23, 2021
Merged

Support array of type in Schema Object #600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 68 additions & 6 deletions packages/openapi3-parser/lib/parser/oas/parseSchemaObject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const {
} = require('../annotations');
const pipeParseResult = require('../../pipeParseResult');
const {
isString, hasKey, getValue,
isArray, isString, hasKey, getValue,
} = require('../../predicates');
const parseObject = require('../parseObject');
const parseArray = require('../parseArray');
Expand Down Expand Up @@ -128,10 +128,14 @@ const typeToElementNameMap = {
* Normalises result into an ArrayElement of StringElement
*/
function parseType(context) {
const { namespace } = context;

let types;
let isValidType;

if (context.isOpenAPIVersionMoreThanOrEqual(3, 1)) {
const isOpenAPI31OrHigher = R.always(context.isOpenAPIVersionMoreThanOrEqual(3, 1));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a suggestion for another PR

A context.isOpenAPI31OrHigher could be added as that is something we are using the isOpenAPIVersionMoreThanOrEqual almost always for IIRC. Then it could be destructured nicely in

const { namespace, isOpenAPI31OrHigher } = context;


if (isOpenAPI31OrHigher()) {
types = openapi31Types;
isValidType = isValidOpenAPI31Type;
} else {
Expand All @@ -141,13 +145,71 @@ function parseType(context) {

const ensureValidType = R.unless(
element => isValidType(element.toValue()),
createWarning(context.namespace, `'${name}' 'type' must be either ${types.join(', ')}`)
createWarning(namespace, `'${name}' 'type' must be either ${types.join(', ')}`)
);

return pipeParseResult(context.namespace,
R.unless(isString, value => createWarning(context.namespace, `'${name}' 'type' is not a string`, value)),
const parseStringType = pipeParseResult(namespace,
ensureValidType,
type => new context.namespace.elements.Array([type]));
type => new namespace.elements.Array([type]));

const ensureValidTypeInArray = R.unless(
element => isValidType(element.toValue()),
createWarning(namespace, `'${name}' 'type' array must only contain values: ${types.join(', ')}`)
);

const parseArrayTypeItem = pipeParseResult(namespace,
R.unless(isString, createWarning(namespace, `'${name}' 'type' array value is not a string`)),
ensureValidTypeInArray);

const isEmpty = arrayElement => arrayElement.isEmpty;

// ArrayElement -> ParseResult<ArrayElement, Annotation>
const ensureTypesAreUnique = (types) => {
Comment on lines +166 to +167
Copy link
Member Author

@kylef kylef Feb 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this function is a bit complicated due to its state, I am pretty sure I can make this function a bit simpler tomorrow with fresh eyes.

This is a pattern we've done elsewhere too, but with map (because we can return the value element or an annotation element into a parse result, but that's a bit tricky because we still need the "values" to be wrapped in an array element instead of being "loose" in a parse result.

const ensureTypesAreUnique = (types) => {
  const inspectedTypes = [];
  const isDuplicate = type => inspectedTypes.includes(type.toValue());
  // allow, like R.T but also with side-effects
  const allow = (type) => { inspectedTypes.push(type.toValue()); return type };
  const validateType = R.ifElse(isDuplicate, createWarning(...), allow);
  return new namespace.elements.ParseResult(R.map(validateType, types));
  /* ^ but above isn't good, because:
    ParseResult(["string", "number", Annotation()]) instead of:
    ParseResult([["string", "number"], Annotation()])
  */
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using R.uniqBy?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

const types = ['string', 'boolean', 'boolean', 'annotation'].map(t => ({ toValue: () => t }));

const toValue = R.invoker(0, 'toValue');
R.pipe(
  R.uniqBy(toValue),
  R.map(toValue)
)(types)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously the above doesn't help collecting the Annotations separately. Perhaps you could get that single list along with annotations and then filter them out in the last step:

const validated = R.map(validateType, types);
const annotation = R.find(is(Annotation), validated);
const validatedTypes = R.filter(complement(equals(annotation)), validated);
return new namespace.elements.ParseResult(validatedTypes, annotation);

const inspectedTypes = [];
const warnings = [];
const permittedTypes = R.filter((type) => {
if (inspectedTypes.includes(type.toValue())) {
warnings.push(createWarning(namespace, `'${name}' 'type' array must contain unique items, ${type.toValue()} is already present`, type));
return false;
}

inspectedTypes.push(type.toValue());
return true;
}, types);

const parseResult = new namespace.elements.ParseResult();
parseResult.push(permittedTypes.elements);

if (warnings.length > 0) {
parseResult.push(...warnings);
}
return parseResult;
};
Comment on lines +168 to +187
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the attempts I suggested above I think this way the implementation shows what you meant by the comment clearly so that I'd leave as is...


const parseArrayType = pipeParseResult(namespace,
R.when(isEmpty, createWarning(namespace, `'${name}' 'type' array must contain at least one type`)),
parseArray(context, `${name}' 'type`, parseArrayTypeItem),
ensureTypesAreUnique,

// FIXME support >1 type
R.when(e => e.length > 1, createWarning(namespace, `'${name}' 'type' more than one type is current unsupported`)));
Comment on lines +194 to +195
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two lines will be removed to allow multiple types with the type parser in the future.


return R.cond([
[isString, parseStringType],
[
R.both(isArray, isOpenAPI31OrHigher),
parseArrayType,
],

[
isOpenAPI31OrHigher,
value => createWarning(namespace, `'${name}' 'type' is not a string or an array`, value),
],
[
R.T,
value => createWarning(namespace, `'${name}' 'type' is not a string`, value),
],
]);
}

// Returns whether the given element value matches the provided schema type
Expand Down
Loading