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

feat: support "host" resources #62

Merged
merged 6 commits into from
Apr 6, 2020
Merged

Conversation

marcofriso
Copy link
Contributor

@marcofriso marcofriso commented Mar 18, 2020

Update the specification at to include "Server Element" structure described by OAS 3 format (https://swagger.io/docs/specification/api-host-and-base-path/).

As reported on Swagger website:

"In OpenAPI 3.0, you use the servers array to specify one or more base URLs for your API. servers replaces the host, basePath and schemes keywords used in OpenAPI 2.0. Each server has an url and an optional Markdown-formatted description."

In OAS 3.0 a simple server looks like this:

servers:
  - url: '{protocol}://api.example.com'
    description: 'Description of the server'
    variables:
      protocol:
        enum:
          - http
          - https
        default: https

In Api Elements a server should contain a href and optionally hrefVariables.
A simple OAS3 server would map to a server element like this:

element: server
meta:
  description: Production
attributes:
  href: https://example.com/

For more information about the design please check the following link → #53 (comment)

UPDATE
→ after talk to @tjanc I am going to remove server object and introduce hosts classification under "Resource" and "Category"

@marcofriso marcofriso force-pushed the marco/server-object-definition branch from 6f683ac to 184a297 Compare March 18, 2020 12:41
@marcofriso marcofriso requested a review from a team March 18, 2020 12:41
@marcofriso
Copy link
Contributor Author

I am not completely sure about this one...

docs/element-definitions.md Outdated Show resolved Hide resolved
docs/element-definitions.md Outdated Show resolved Hide resolved
docs/element-definitions.md Outdated Show resolved Hide resolved
docs/element-definitions.md Outdated Show resolved Hide resolved
docs/element-definitions.md Outdated Show resolved Hide resolved
@marcofriso marcofriso requested review from tjanc and removed request for tjanc March 24, 2020 11:24
@marcofriso
Copy link
Contributor Author

In the last commit server object has been replaced by classification hosts under resource and category elements.

@marcofriso marcofriso requested a review from a team March 24, 2020 11:54
@marcofriso marcofriso changed the title docs: add server object to element-definitions docs: add server support to element-definitions Mar 24, 2020
@@ -1139,6 +1139,10 @@ The Resource representation with its available transitions and its data.

The `content` MUST NOT include more than one `Data Structure`.

#### Classifications

- `"hosts"` - Resource is a set of servers.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be
"host" - Resource is a host description

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, explain what a "host" resource is in the description of the resource element (L1123).
Something like "A resource element classified as host SHALL be interpreted as...".

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 am not sure if this has been implemented in the end as in Resource we have hosts

docs/element-definitions.md Outdated Show resolved Hide resolved
Copy link
Member

@kylef kylef left a comment

Choose a reason for hiding this comment

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

I've made a couple of suggestions, one is regarding adding the hosts attribute to both Resource and Transition. This is to support the OpenAPI feature of overriding the host at the resource or transition element.

The other is to include further semantics and explanation for the reader for the host resource classification.

In terms of API Elements JS, we likely want to add the following:

  • hosts property to Category which looks for hosts categories, something like item.classes.contains(‘hosts’).
  • hosts property on Resource which looks in this.attributes for the hosts category.
  • hosts property on Transition which looks in this.attributes for the hosts category.

docs/element-definitions.md Outdated Show resolved Hide resolved
docs/element-definitions.md Show resolved Hide resolved
- for Resource and Transition
- refactoring
@marcofriso marcofriso force-pushed the marco/server-object-definition branch from e6be9ae to 66dcf39 Compare March 25, 2020 11:15
@marcofriso marcofriso requested review from a team, tjanc and kylef and removed request for tjanc and kylef March 25, 2020 12:00
@kylef kylef requested a review from tjanc March 26, 2020 11:40
Copy link
Member

@kylef kylef left a comment

Choose a reason for hiding this comment

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

This looks good to me, I'll let @tjanc have another look before we merge otherwise looks great 👍.

Copy link
Contributor

@tjanc tjanc left a comment

Choose a reason for hiding this comment

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

Some wording/fixes. @kylef feedback appreciated, my the english is not the perfectest.
Also, @marcofriso please add an entry to the overview here:
https://github.com/apiaryio/api-elements/blob/master/docs/overview.md#relationship-of-elements
Something like

Category (Group of Resource Elements representing hosts)

after

Category (Group of Resource Elements)
Category (Group of Authentication and Authorization Scheme Definitions)

@@ -1126,11 +1126,17 @@ The Resource representation with its available transitions and its data.

- `element` - `"resource"`
- `attributes`
- `hosts` ([Array][][[Resource][]]).

Resources in the hosts attribute SHOULD implicitly be treated as they have the `host` resource classification.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps

Optional list of host resources. Every entry SHALL be interpreted as if classified as `host`.
_See `host` classification in [Resource][] for further semantics._

Overrides any otherwise relevant `hosts` definitions.

@kylef what do you think (wording)?

Copy link
Contributor Author

@marcofriso marcofriso Mar 31, 2020

Choose a reason for hiding this comment

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

what's the difference between SHALL and SHOULD in these circumstances?

Copy link
Contributor

Choose a reason for hiding this comment

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

SHALL describes a requirement. SHOULD describes a recommendation. See RFC 2119.
I think we should require any interpreter of APIE to see these entries as hosts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed


Definition of any input message-body attribute for this transition.

- `hosts` ([Array][][[Transition][]]).
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be

`hosts` ([Array][][[Resource][]])

Also, wording, similar to above comment:

Optional list of host resources. Every entry SHALL be interpreted as if classified as `host`.
_See `host` classification in [Resource][] for further semantics._

All [Resource][]s nested under the [Transition][]'s `content` SHALL interpret this `hosts` definition as their own, unless it is overridden by another `hosts` definition on the path to the [Resource][] element.

@kylef feedback appreciated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

Copy link
Contributor Author

@marcofriso marcofriso Apr 3, 2020

Choose a reason for hiding this comment

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

moved 'See host classification in [Resource][] for further semantics.' at the end of the paragraph.

- `"host"` - A host transition represents the "root" of the API transition.
The transition href MAY be append to the host href to create a absolute URI.
A transition that has a `host` classification MUST be a root component of a URI.

Copy link
Contributor

Choose a reason for hiding this comment

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

A transition cannot be a host no? Just a resource. I think we can remove this paragraph.

Copy link
Contributor Author

@marcofriso marcofriso Mar 31, 2020

Choose a reason for hiding this comment

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

Being application state transitions ("Transitions") resource operations should we include here the Category of Hosts at all (#53 (comment))?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

- `"dataStructures"` - Category is a set of data structures.
- `"hosts"` - Category of [Resource][] or [Transition][] which implicitly contain the respective `host` classification.
Copy link
Contributor

Choose a reason for hiding this comment

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

Wording:

Category of [Resource][]s interpreted as a list of host resources of an API. Every entry SHALL be interpreted as if classified as `host`.

_See `host` classification in [Resource][] for further semantics._

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

@marcofriso
Copy link
Contributor Author

I see also some of the internal links have to be changed as they don't work

@marcofriso marcofriso requested a review from tjanc April 6, 2020 11:44
@kylef kylef changed the title docs: add server support to element-definitions feat: support "host" resources Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants