Skip to content

New response type #391

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Apr 29, 2021
Merged

New response type #391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1384e73
WIP: refactoring Response classes
delvedor Apr 28, 2021
56d07e4
WIP: refactoring Response classes and removed old behaviors
delvedor Apr 28, 2021
fe5f42a
Updated compiler to adopt the new Response type
delvedor Apr 28, 2021
20660c0
Updated ts generator to update the new Response type
delvedor Apr 28, 2021
34ec326
Updated output
delvedor Apr 28, 2021
df12be8
Updated docs
delvedor Apr 28, 2021
02dc6b0
Merge branch 'master' into new-response-type
delvedor Apr 28, 2021
6d5134c
Updated output
delvedor Apr 28, 2021
73011bd
Lint
delvedor Apr 28, 2021
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
106 changes: 60 additions & 46 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,62 +84,76 @@ endpoint of Elasticsearch and the respective type mapping. For example:

```jsonc
{
"types": [{
"kind": "request",
"name": {
"namespace": "document.single.index",
"name": "IndexRequest"
},
"description": "The document",
"annotations": {
"type_stability": "stable"
},
"generics": [
"TDocument"
"types": [ {
"attachedBehaviors": [
"CommonQueryParameters"
],
"inherits": [
{
"body": {
"kind": "value",
"value": {
"kind": "instance_of",
"type": {
"namespace": "common_abstractions.request",
"name": "RequestBase"
"name": "TDocument",
"namespace": "_global.index"
}
}
],
"path": [...],
"query": [...],
"body": {...}
}, {
"kind": "interface",
"name": {
"namespace": "document.single.index",
"name": "IndexResponse"
},
"inherits": [
"generics": [
{
"type": {
"namespace": "document.single",
"name": "WriteResponseBase"
}
"name": "TDocument",
"namespace": "_global.index"
}
],
"properties": []
}],
"endpoints": [{
"name": "index",
"description": "Creates or updates a document in an index.",
"docUrl": "https://www.elastic.co/guide/en/elasticsearch/reference/master/docs-index_.html",
"stability": "stable",
"request": {
"namespace": "document.single.index",
"name": "IndexRequest"
"inherits": {
"type": {
"name": "RequestBase",
"namespace": "_types"
}
},
"kind": "request",
"name": {
"name": "Request",
"namespace": "_global.index"
},
"requestBodyRequired": true,
"response": {
"namespace": "document.single.index",
"name": "IndexResponse"
"path": [...],
"query": [...]
}, {
"inherits": {
"type": {
"name": "WriteResponseBase",
"namespace": "_types"
}
},
"urls": [...]
}]
"kind": "response",
"name": {
"name": "Response",
"namespace": "_global.index"
}
}],
"endpoints": [{
"accept": [
"application/json"
],
"contentType": [
"application/json"
],
"description": "Creates or updates a document in an index.",
"docUrl": "https://www.elastic.co/guide/en/elasticsearch/reference/master/docs-index_.html",
"name": "index",
"request": {
"name": "Request",
"namespace": "_global.index"
},
"requestBodyRequired": true,
"response": {
"name": "Response",
"namespace": "_global.index"
},
"since": "0.0.0",
"stability": "stable",
"urls": [...],
"visibility": "public"
}]
}
```

Expand Down
100 changes: 64 additions & 36 deletions compiler/model/build-model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import {
getNameSpace,
hoistRequestAnnotations,
hoistTypeAnnotations,
isApi,
isKnownBehavior,
modelBehaviors,
modelEnumDeclaration,
Expand Down Expand Up @@ -160,61 +159,90 @@ export function compileSpecification (endpointMappings: Record<string, model.End
return model
}

function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | InterfaceDeclaration, mappings: Record<string, model.Endpoint>, allClasses: ClassDeclaration[]): model.Request | model.Interface {
function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | InterfaceDeclaration, mappings: Record<string, model.Endpoint>, allClasses: ClassDeclaration[]): model.Request | model.Response | model.Interface {
const name = declaration.getName()
assert(declaration, name != null, 'Anonymous definitions should not exists')

if (name.endsWith('Request')) {
if (name === 'Request') {
assert(
declaration,
Node.isInterfaceDeclaration(declaration),
`Request definitions must be declared as interfaces: ${name}`
)
}

if (name.endsWith('Response')) {
if (name === 'Response') {
assert(
declaration,
Node.isClassDeclaration(declaration),
`Response definitions must be declared as classes: ${name}`
)
}

// Request definitions needs to be handled
// Request and Response definitions needs to be handled
// differently from normal classes
if (Node.isInterfaceDeclaration(declaration) && isApi(declaration)) {
const response: model.TypeName = {
name: 'Response',
namespace: getNameSpace(declaration)
}
if (name === 'Request' || name === 'Response') {
let type: model.Request | model.Response
if (name === 'Request') {
type = {
kind: 'request',
name: { name, namespace: getNameSpace(declaration) },
path: new Array<model.Property>(),
query: new Array<model.Property>()
}

const type: model.Request = {
kind: 'request',
name: { name, namespace: getNameSpace(declaration) },
path: new Array<model.Property>(),
query: new Array<model.Property>()
}
const response: model.TypeName = {
name: 'Response',
namespace: getNameSpace(declaration)
}

hoistRequestAnnotations(type, declaration.getJsDocs(), mappings, response)
hoistRequestAnnotations(type, declaration.getJsDocs(), mappings, response)

for (const member of declaration.getMembers()) {
// we are visiting `path_parts, `query_parameters` or `body`
assert(
member,
Node.isPropertyDeclaration(member) || Node.isPropertySignature(member),
'Class and interfaces can only have property declarations or signatures'
)
const property = visitRequestProperty(member)
if (property.name === 'path_parts') {
type.path = property.properties
} else if (property.name === 'query_parameters') {
type.query = property.properties
} else {
// the body can either by a value (eg Array<string> or an object with properties)
if (property.valueOf != null) {
type.body = { kind: 'value', value: property.valueOf }
} else if (property.properties.length > 0) {
type.body = { kind: 'properties', properties: property.properties }
for (const member of declaration.getMembers()) {
// we are visiting `path_parts, `query_parameters` or `body`
assert(
member,
Node.isPropertyDeclaration(member) || Node.isPropertySignature(member),
'Class and interfaces can only have property declarations or signatures'
)
const property = visitRequestOrResponseProperty(member)
if (property.name === 'path_parts') {
type.path = property.properties
} else if (property.name === 'query_parameters') {
type.query = property.properties
} else {
// the body can either by a value (eg Array<string> or an object with properties)
if (property.valueOf != null) {
type.body = { kind: 'value', value: property.valueOf }
} else if (property.properties.length > 0) {
type.body = { kind: 'properties', properties: property.properties }
}
}
}
} else {
type = {
kind: 'response',
name: { name, namespace: getNameSpace(declaration) },
body: undefined
}

for (const member of declaration.getMembers()) {
// we are visiting `path_parts, `query_parameters` or `body`
assert(
member,
Node.isPropertyDeclaration(member) || Node.isPropertySignature(member),
'Class and interfaces can only have property declarations or signatures'
)
const property = visitRequestOrResponseProperty(member)
if (property.name === 'body') {
// the body can either by a value (eg Array<string> or an object with properties)
if (property.valueOf != null) {
type.body = { kind: 'value', value: property.valueOf }
} else if (property.properties.length > 0) {
type.body = { kind: 'properties', properties: property.properties }
}
} else {
assert(member, false, 'Response.body is the only Response property supported')
}
}
}
Expand Down Expand Up @@ -340,7 +368,7 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int
* differently as are described as nested objects, and the body could have two
* different types, `model.Property[]` (a normal object) or `model.ValueOf` (eg: an array or generic)
*/
function visitRequestProperty (member: PropertyDeclaration | PropertySignature): { name: string, properties: model.Property[], valueOf: model.ValueOf | null } {
function visitRequestOrResponseProperty (member: PropertyDeclaration | PropertySignature): { name: string, properties: model.Property[], valueOf: model.ValueOf | null } {
const properties: model.Property[] = []
let valueOf: model.ValueOf | null = null

Expand Down
24 changes: 22 additions & 2 deletions compiler/model/metamodel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class TypeName {
/**
* Type of a value. Used both for property types and nested type definitions.
*/
export type ValueOf = InstanceOf | ArrayOf | UnionOf | DictionaryOf | UserDefinedValue | LiteralValue
export type ValueOf = InstanceOf | ArrayOf | UnionOf | DictionaryOf | UserDefinedValue | LiteralValue | VoidValue

/**
* A single value
Expand Down Expand Up @@ -116,6 +116,13 @@ export class LiteralValue {
value: string | number | boolean
}

/**
* The absence of any type. This is commonly used in APIs that returns an empty body.
*/
export class VoidValue {
kind: 'void_value'
}

Comment on lines +119 to +125
Copy link
Member

@swallez swallez May 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Late review of this PR... IMHO we should remove this newly introduced VoidValue.

void is not a value, it's the absence of value (which is different from null and undefined that are values). As such a property cannot be of type void, but adding it to the ValueOf enumeration makes it possible.

We already have a simpler means to represent requests and responses with an empty body (or more exactly no body, as an empty body could be {}): the body property is optional: body?: ValueBody | PropertiesBody. If it's not specified, then the request or response has no body.

Or was this introduced so that body-less requests and responses are explicitly represented in the spec? In that case we should make the body property a required field and add a 3rd alternative: body: ValueBody | PropertiesBody | VoidBody

/**
* An interface or request interface property.
*/
Expand All @@ -142,7 +149,7 @@ export class Property {
// ------------------------------------------------------------------------------------------------
// Type definitions

export type TypeDefinition = Interface | Request | Enum | TypeAlias
export type TypeDefinition = Interface | Request | Response | Enum | TypeAlias

// ------------------------------------------------------------------------------------------------

Expand Down Expand Up @@ -240,6 +247,19 @@ export class Request extends BaseType {
attachedBehaviors?: string[]
}

/**
* A response type
*/
export class Response extends BaseType {
kind: 'response'
generics?: TypeName[]
inherits?: Inherits
implements?: Inherits[]
body?: ValueBody | PropertiesBody
behaviors?: Inherits[]
attachedBehaviors?: string[]
}

export class ValueBody {
kind: 'value'
value: ValueOf
Expand Down
20 changes: 18 additions & 2 deletions compiler/model/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ import { dirname } from 'path'
*/
export const knownBehaviors = [
'AdditionalProperties',
'ArrayResponseBase',
'EmptyResponseBase',
'CommonQueryParameters',
'CommonCatQueryParameters'
]
Expand Down Expand Up @@ -112,6 +110,17 @@ export function modelType (node: Node): model.ValueOf {
return type
}

case ts.SyntaxKind.VoidKeyword: {
const type: model.InstanceOf = {
kind: 'instance_of',
type: {
name: 'void',
namespace: 'internal'
}
}
return type
}

// TODO: this should not be used in the specification
// we should throw an error
case ts.SyntaxKind.AnyKeyword: {
Expand Down Expand Up @@ -238,6 +247,13 @@ export function modelType (node: Node): model.ValueOf {
return type
}

case 'Void': {
const type: model.VoidValue = {
kind: 'void_value'
}
return type
}

default: {
const generics = node.getTypeArguments().map(node => modelType(node))
const identifier = node.getTypeName()
Expand Down
Loading