Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support constraints! #76

Merged
merged 4 commits into from
May 30, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
54 changes: 54 additions & 0 deletions docs/schema.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Schema support

The `Schema` object in [Open API v3 spec][oaschema] describes several properties
that are shared from JSON Schema, deviations from JSON Schema, or in addition
to JSON Schema. The document descibes this project's support for these
properties.

## Properties

The following properties are supported, and implemented according to the
[JSON Schema Validation spec][jsschema]:

- [x] multipleOf
- [x] maximum
- [x] exclusiveMaximum
- [x] minimum
- [x] exclusiveMinimum
- [x] maxLength
- [x] minLength
- [x] pattern
- [x] maxItems
- [x] minItems
- [x] uniqueItems
- [x] maxProperties
- [x] minProperties
- [x] format
- [x] required
- [x] enum

## Adjusted JSON Schema Properties

The OpenAPI specification also supports several additional properties from JSON
Schema, with some adjustments. This project attempts to honor these adjustments,
with any exceptions outlined below:

- [x] type - Value may be an array, multiple types are supported.
- [x] allOf
- [ ] oneOf
- [ ] anyOf
- [ ] not
- [x] items
- [x] properties
- [ ] additionalProperties
- [x] description
- [x] format
- [x] default

## Fixed Fields

At this time, the project supports no [fixed fields][ff].

[ff]: https://github.com/OAI/OpenAPI-Specification/blob/OpenAPI.next/versions/3.0.md#fixed-fields-20
[jsschema]: http://json-schema.org/latest/json-schema-validation.html#rfc.section.6
[oaschema]: https://github.com/OAI/OpenAPI-Specification/blob/OpenAPI.next/versions/3.0.md#schemaObject
53 changes: 53 additions & 0 deletions src/components/ArrayProperty/ArrayProperty.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import React, { PureComponent } from 'react';
import PropTypes from 'prop-types';

export default class ArrayProperty extends PureComponent {
render() {
const { constraints } = this.props;

if (!constraints) {
return null;
}

const { minItems, maxItems, uniqueItems } = constraints;
const validations = [];

if (uniqueItems) {
validations.push('items must be unique');
}

if (maxItems !== undefined && minItems !== undefined) {
// Be succint if the minItems is the same maxItems
// ie. value can only be of `x` length.
if (maxItems === minItems) {
validations.push(`${minItems} items`);
} else {
validations.push(`${minItems}-${maxItems} items`);
}
} else if (minItems !== undefined) {
validations.push(`at least ${minItems} items`);
} else if (maxItems !== undefined) {
validations.push(`at most ${maxItems} items`);
}

if (!validations.length) {
return null;
}

return (
<span>
{validations.map(constraint =>
<span key={constraint} className="array-constraints">{constraint}</span>
)}
</span>
);
}
}

ArrayProperty.propTypes = {
constraints: PropTypes.shape({
maxItems: PropTypes.number,
minItems: PropTypes.number,
uniqueItems: PropTypes.bool
})
};
Copy link
Contributor

Choose a reason for hiding this comment

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

This class's responsibility is to render the validation constraints for arrays, and thus should be named accordingly.

Need one space in UI (Carriers > Returns a list of integrations available for registration)

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want all the *Property classes to be renamed to *Constraints (I don't feel strongly either way)?

As for the space, I'd add some CSS.

Copy link
Contributor

@quangkhoa quangkhoa May 29, 2017

Choose a reason for hiding this comment

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

Yes, they should either have Constraint in the name, or are all under a Constraints directory. That makes it easy to follow.

12 changes: 7 additions & 5 deletions src/components/BodySchema/BodySchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,20 @@ export default class BodySchema extends Component {
if (properties.length === iterator) {
isLast = true;
}
if (property.type === 'array' && expandedProp.indexOf(property.name) !== -1 && property.properties !== undefined) {

if (property.type.includes('array') && expandedProp.includes(property.name) && property.properties !== undefined) {
return createFragment({
property: this.renderPropertyRow(property, isLast, true),
subset: this.renderSubsetProperties(property, true)
});
} else if (property.type === 'array' && property.properties !== undefined) {
} else if (property.type.includes('array') && property.properties !== undefined) {
return this.renderPropertyRow(property, isLast, false);
} else if (property.type === 'object' && expandedProp.indexOf(property.name) !== -1 && property.properties !== undefined) {
} else if (property.type.includes('object') && expandedProp.includes(property.name) && property.properties !== undefined) {
return createFragment({
property: this.renderPropertyRow(property, isLast, true),
subset: this.renderSubsetProperties(property)
});
} else if (property.type === 'object' && property.properties !== undefined) {
} else if (property.type.includes('object') && property.properties !== undefined) {
return this.renderPropertyRow(property, isLast, false);
} else {
return this.renderPropertyRow(property, isLast);
Expand All @@ -71,6 +72,7 @@ export default class BodySchema extends Component {
description={property.description}
enumValues={property.enum}
defaultValue={property.defaultValue}
constraints={property.constraints}
onClick={this.onClick.bind(this, property.name)}
isRequired={property.required}
isOpen={isOpen}
Expand Down Expand Up @@ -110,7 +112,7 @@ export default class BodySchema extends Component {

onClick(propertyName) {
const { expandedProp } = this.state;
if (expandedProp.indexOf(propertyName) !== -1) {
if (expandedProp.includes(propertyName)) {
const newExpanded = expandedProp.filter((prop) => prop !== propertyName);
this.setState({ expandedProp: newExpanded });
} else {
Expand Down
9 changes: 1 addition & 8 deletions src/components/Method/Method.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,7 @@ export default class Method extends Component {
return (
<div className="method-responses">
<h4>Responses</h4>
{responses.map((response) => {
return (
<Response
key={response.code}
response={response}
/>
);
})}
{responses.map((r) => <Response key={r.code} response={r} />)}
</div>
);
}
Expand Down
58 changes: 58 additions & 0 deletions src/components/NumericProperty/NumericProperty.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import React, { PureComponent } from 'react';
import PropTypes from 'prop-types';

export default class NumericProperty extends PureComponent {
render() {
const { constraints } = this.props;

if (!constraints) {
return null;
}

const { exclusiveMinimum, exclusiveMaximum, maximum, minimum, multipleOf } = constraints;
const validations = [];

if (multipleOf) {
validations.push(`multiple of ${multipleOf}`);
}

// We're following JSON-Schema Draft 6, which states that exclusive* are
// integers, not boolean values. Also using `undefined` to prevent edge
// cases where the value is 0 or 1.
if (maximum !== undefined && minimum !== undefined) {
validations.push(`${minimum}…${maximum}`);
} else if (exclusiveMaximum !== undefined && exclusiveMinimum !== undefined) {
validations.push(`${exclusiveMinimum}…${exclusiveMaximum}`);
} else if (minimum !== undefined) {
validations.push(`≥ ${minimum}`);
} else if (maximum !== undefined) {
validations.push(`≤ ${maximum}`);
} else if (exclusiveMinimum !== undefined) {
validations.push(`> ${exclusiveMinimum}`);
} else if (exclusiveMaximum !== undefined) {
validations.push(`< ${exclusiveMaximum}`);
}

if (!validations.length) {
return null;
}

return (
<span>
{validations.map(constraint =>
<span key={constraint} className="numeric-constraints">{constraint}</span>
)}
</span>
);
}
}

NumericProperty.propTypes = {
constraints: PropTypes.shape({
exclusiveMinimum: PropTypes.number,
exclusiveMaximum: PropTypes.number,
maximum: PropTypes.number,
minimum: PropTypes.number,
multipleOf: PropTypes.number
})
};
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the construction of the constraints in those new constraints components could be extracted out, which would make the logic a bit easier to test (and those components could potentially go away??? We can then have a shared ConstraintComponent that requires {validations, validationType}. What do you reckon @brendo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't agree sorry. I considered having one component be responsible for constraints, but ultimately it would still have switch logic in it to only apply certain constraints to certain components.

Having small (tiny even!) components that just focus on all things Numeric or String made for really easy testing and if we want to add more constraints, we'd only have to touch a small part of the codebase!

I'm happy to dumb down the Property constraints proptype (from shape to object) and just leave the smaller components deal with the actual specifics if you think that'd reduce duplication?

Copy link
Contributor

@quangkhoa quangkhoa May 29, 2017

Choose a reason for hiding this comment

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

I totally agreed with you about the approach of having multiple small components.

My point was that all those constraints UI components share the same pattern:

  1. Construct the validations array
  2. Render the validations array, e.g.
<span>
  {validations.map(constraint =>
    <span key={constraint} className={constraintName}>{constraint}</span>
  )}
</span>

So they only differ in the way validations are constructed. This part, however, has nothing to do with UI.

If we move the validation logic out (as we try to keep the UI components really lean), to seperate functions, then there are no difference between those UI constraint components. React components offer PropTypes.shape, which is a nice way to specify the signature (input) of the UI component; however we could achieve something like that with function signature and typescript (down the track).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah got you, I have an idea, let me come back to you...

48 changes: 48 additions & 0 deletions src/components/ObjectProperty/ObjectProperty.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import React, { PureComponent } from 'react';
import PropTypes from 'prop-types';

export default class ObjectProperty extends PureComponent {
render() {
const { constraints } = this.props;

if (!constraints) {
return null;
}

const { minProperties, maxProperties } = constraints;
const validations = [];

if (maxProperties !== undefined && minProperties !== undefined) {
// Be succint if the minProperties is the same maxProperties
// ie. value can only be of `x` length.
if (maxProperties === minProperties) {
validations.push(`${minProperties} properties`);
} else {
validations.push(`${minProperties}-${maxProperties} properties`);
}
} else if (minProperties !== undefined) {
validations.push(`at least ${minProperties} properties`);
} else if (maxProperties !== undefined) {
validations.push(`at most ${maxProperties} properties`);
}

if (!validations.length) {
return null;
}

return (
<span>
{validations.map(constraint =>
<span key={constraint} className="object-constraints">{constraint}</span>
)}
</span>
);
}
}

ObjectProperty.propTypes = {
constraints: PropTypes.shape({
maxProperties: PropTypes.number,
minProperties: PropTypes.number
})
};
38 changes: 35 additions & 3 deletions src/components/Property/Property.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,20 @@ import React, { Component } from 'react';
import classNames from 'classnames';
import PropTypes from 'prop-types';

import ArrayProperty from '../ArrayProperty/ArrayProperty';
import Description from '../Description/Description';
import Indicator from '../Indicator/Indicator';
import NumericProperty from '../NumericProperty/NumericProperty';
import ObjectProperty from '../ObjectProperty/ObjectProperty';
import StringProperty from '../StringProperty/StringProperty';

import './Property.scss';

export default class Property extends Component {
render() {
const { name, type, description, isRequired, enumValues, defaultValue, onClick, isOpen, isLast } = this.props;
const {
name, type, description, constraints, isRequired, enumValues, defaultValue, onClick, isOpen, isLast
} = this.props;

let subtype;
if (type.includes('array')) {
Expand All @@ -20,6 +26,7 @@ export default class Property extends Component {
if (isOpen !== undefined) {
isClickable = true;
}

let status;
if (isOpen) {
status = 'open';
Expand All @@ -41,10 +48,18 @@ export default class Property extends Component {
}
</td>
<td className="property-info">
<span>{type.join(', ')}</span>{subtype && <span> of {subtype}</span>}
<span className="property-type">
{!subtype ? type.join(', ') : <span className="property-subtype">{subtype}[]</span>}
{!subtype && constraints && constraints.format &&
<span className="property-format">&lt;{constraints.format}&gt;</span>}
</span>
{isRequired && <span className="property-required">Required</span>}
{['number', 'integer'].some(t => type.includes(t)) && <NumericProperty constraints={constraints} />}
{type.includes('string') && <StringProperty constraints={constraints} />}
{type.includes('array') && <ArrayProperty constraints={constraints} />}
{type.includes('object') && <ObjectProperty constraints={constraints} />}
{enumValues && this.renderEnumValues(enumValues)}
{defaultValue && this.renderDefaultValue(defaultValue)}
{defaultValue !== undefined && this.renderDefaultValue(defaultValue)}
{description && <Description description={description}/>}
</td>
</tr>
Expand Down Expand Up @@ -93,7 +108,24 @@ Property.propTypes = {
name: PropTypes.string.isRequired,
type: PropTypes.arrayOf(PropTypes.string).isRequired,
subtype: PropTypes.string,
title: PropTypes.string,
description: PropTypes.string,
constraints: PropTypes.shape({
format: PropTypes.string,
exclusiveMinimum: PropTypes.number,
exclusiveMaximum: PropTypes.number,
maximum: PropTypes.number,
maxItems: PropTypes.number,
maxLength: PropTypes.number,
maxProperties: PropTypes.number,
minimum: PropTypes.number,
minItems: PropTypes.number,
minLength: PropTypes.number,
minProperties: PropTypes.number,
multipleOf: PropTypes.number,
pattern: PropTypes.string,
uniqueItems: PropTypes.bool
}),
enumValues: PropTypes.array,
defaultValue: PropTypes.any,
isRequired: PropTypes.bool,
Expand Down
Loading