Skip to content

Commit

Permalink
Introduce a type unicity verifier and fix endpoint unicity verifier (#…
Browse files Browse the repository at this point in the history
…216)

We didn't have a way to ensure that the same type name was used twice.

It turns out, our endpoint name verifier was also incorrect because it compared Locatables and not strings.

Adding unit tests to ensure we never break this again :)
  • Loading branch information
fwouts authored May 8, 2019
1 parent ac4d321 commit 478c045
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 11 deletions.
71 changes: 71 additions & 0 deletions lib/src/verifiers/unicity/endpoints.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { HttpMethod } from "../../models/http";
import {
ApiNode,
ContractNode,
EndpointNode,
RequestNode
} from "../../models/nodes";
import { fakeLocatable } from "../../test/fake-locatable";
import { verifyUniqueEndpointNames } from "./endpoints";

describe("unique endpoint names verifier", () => {
test("valid for correct usage", () => {
const contractNode: ContractNode = {
api: fakeLocatable({} as ApiNode),
endpoints: [
fakeLocatable<EndpointNode>({
name: fakeLocatable("EndpointOne"),
tags: fakeLocatable([]),
method: fakeLocatable<HttpMethod>("POST"),
path: fakeLocatable("/a"),
request: fakeLocatable({} as RequestNode),
responses: [],
tests: []
}),
fakeLocatable<EndpointNode>({
name: fakeLocatable("EndpointTwo"),
tags: fakeLocatable([]),
method: fakeLocatable<HttpMethod>("POST"),
path: fakeLocatable("/b"),
request: fakeLocatable({} as RequestNode),
responses: [],
tests: []
})
],
types: []
};
expect(verifyUniqueEndpointNames(contractNode)).toHaveLength(0);
});

test("invalid for duplicate names", () => {
const contractNode: ContractNode = {
api: fakeLocatable({} as ApiNode),
endpoints: [
fakeLocatable<EndpointNode>({
name: fakeLocatable("EndpointOne"),
tags: fakeLocatable([]),
method: fakeLocatable<HttpMethod>("POST"),
path: fakeLocatable("/a"),
request: fakeLocatable({} as RequestNode),
responses: [],
tests: []
}),
fakeLocatable<EndpointNode>({
name: fakeLocatable("EndpointOne"),
tags: fakeLocatable([]),
method: fakeLocatable<HttpMethod>("POST"),
path: fakeLocatable("/b"),
request: fakeLocatable({} as RequestNode),
responses: [],
tests: []
})
],
types: []
};
expect(verifyUniqueEndpointNames(contractNode)).toMatchObject([
{
message: "endpoints must have unique names"
}
]);
});
});
21 changes: 21 additions & 0 deletions lib/src/verifiers/unicity/endpoints.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { uniq } from "lodash";
import { ContractNode } from "../../models/nodes";
import { VerificationError } from "../verification-error";

export function verifyUniqueEndpointNames(
contract: ContractNode
): VerificationError[] {
const errors: VerificationError[] = [];
const endpointNames = contract.endpoints.map(
endpoint => endpoint.value.name.value
);
if (uniq(endpointNames).length !== endpointNames.length) {
errors.push({
message: "endpoints must have unique names",
// TODO: use a duplicated endpoint location
location: contract.api.location,
line: contract.api.line
});
}
return errors;
}
46 changes: 46 additions & 0 deletions lib/src/verifiers/unicity/types.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { ApiNode, ContractNode } from "../../models/nodes";
import { STRING } from "../../models/types";
import { fakeLocatable } from "../../test/fake-locatable";
import { verifyUniqueTypeNames } from "./types";

describe("unique type names verifier", () => {
test("valid for correct usage", () => {
const contractNode: ContractNode = {
api: fakeLocatable({} as ApiNode),
endpoints: [],
types: [
{
name: "TypeOne",
type: STRING
},
{
name: "TypeTwo",
type: STRING
}
]
};
expect(verifyUniqueTypeNames(contractNode)).toHaveLength(0);
});

test("invalid for duplicate names", () => {
const contractNode: ContractNode = {
api: fakeLocatable({} as ApiNode),
endpoints: [],
types: [
{
name: "TypeOne",
type: STRING
},
{
name: "TypeOne",
type: STRING
}
]
};
expect(verifyUniqueTypeNames(contractNode)).toMatchObject([
{
message: "types must have unique names"
}
]);
});
});
19 changes: 19 additions & 0 deletions lib/src/verifiers/unicity/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { uniq } from "lodash";
import { ContractNode } from "../../models/nodes";
import { VerificationError } from "../verification-error";

export function verifyUniqueTypeNames(
contract: ContractNode
): VerificationError[] {
const errors: VerificationError[] = [];
const typeNames = contract.types.map(type => type.name);
if (uniq(typeNames).length !== typeNames.length) {
errors.push({
message: "types must have unique names",
// TODO: use a duplicated type location
location: contract.api.location,
line: contract.api.line
});
}
return errors;
}
15 changes: 4 additions & 11 deletions lib/src/verifiers/verifier.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { uniq } from "lodash";
import { ContractNode } from "../models/nodes";
import { verifyApiNode } from "./nodes/api-verifier";
import { verifyEndpointNode } from "./nodes/endpoint-verifier";
import { verifyUniqueEndpointNames } from "./unicity/endpoints";
import { verifyUniqueTypeNames } from "./unicity/types";
import { VerificationError } from "./verification-error";

export function verify(contract: ContractNode): VerificationError[] {
Expand All @@ -13,16 +14,8 @@ export function verify(contract: ContractNode): VerificationError[] {
errors.push(...verifyEndpointNode(endpoint.value, contract.types))
);

// ensure endpoints have unique names
const endpointNames = contract.endpoints.map(endpoint => endpoint.value.name);
if (uniq(endpointNames).length !== endpointNames.length) {
errors.push({
message: "endpoints must have unique names",
// TODO: use a duplicated endpoint location
location: contract.api.location,
line: contract.api.line
});
}
errors.push(...verifyUniqueEndpointNames(contract));
errors.push(...verifyUniqueTypeNames(contract));

return errors;
}

0 comments on commit 478c045

Please sign in to comment.