Skip to content

Commit

Permalink
feat(DPE-35): Created Collections rules (#86)
Browse files Browse the repository at this point in the history
Impact testing results are about 8 more errors in total. The execution
time for the impact testing is within a second or so compared to the
current ruleset in the main branch.

Performance testing results are okay. These changes will create about
700 more errors or warnings within this Open API document suite we use.
About 564 of those 700 errors/warnings are mainly concentrated in a few
documents. The two rules `sps-missing-pagination-query-parameters` and
`sps-no-collection-paging-capability` are the ones that are causing the
most errors since not all GET endpoints are not designed with
collections capabilities. Execution time is still within a second or so
of the current ruleset on the main branch.

---------

Signed-off-by: Ethan Honzik <[email protected]>
Co-authored-by: Ethan Honzik <[email protected]>
Co-authored-by: Ethan Honzik <[email protected]>
Co-authored-by: Mark DeBeer <[email protected]>
  • Loading branch information
4 people authored Aug 12, 2024
1 parent 08c542b commit 1ad59f3
Show file tree
Hide file tree
Showing 13 changed files with 1,236 additions and 11 deletions.
1 change: 1 addition & 0 deletions rulesets/src/.spectral.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ extends:
- serialization.ruleset.yml
- url-structure.ruleset.yml
- webhooks.ruleset.yml
- collections.ruleset.yml

rules:
# not keeping this, just off for testing in this repo
Expand Down
168 changes: 168 additions & 0 deletions rulesets/src/collections.ruleset.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
rules:
##### General #####
sps-no-collection-paging-capability:
description: "Response bodies from collection endpoints SHOULD offer paging capability."
severity: warn
given: $.paths[?([email protected](/.*\/\{[^}]+\}$/))].get.responses['200'].content.application/json.schema.properties
then:
- field: "paging"
function: truthy
- field: "paging.type"
function: pattern
functionOptions:
match: "object"

##### Root Element #####
sps-collection-missing-results-array:
description: "Response bodies must have a root element called results and is an array of objects."
severity: error
given: $.paths[?([email protected](/.*\/\{[^}]+\}$/))].get.responses['200'].content.application/json.schema.properties.results
then:
- field: type
function: pattern
functionOptions:
match: "array"
- field: items.type
function: pattern
functionOptions:
match: "object"

##### Pagination #####
sps-missing-pagination-query-parameters:
description: "Collection GET endpoints SHOULD support pagination using query parameters. Offset or cursor based pagination is required."
severity: warn
given: $.paths[?([email protected](/.*\/\{[^}]+\}\/*.*/))].get
then:
- field: parameters
function: schema
functionOptions:
schema:
type: array
items:
type: object
contains:
type: object
properties:
name:
const: limit
in:
const: query
allOf:
- anyOf:
- contains:
type: object
properties:
name:
const: offset
in:
const: query
- contains:
type: object
properties:
name:
const: cursor
in:
const: query

sps-post-request-body-missing-paging-object:
description: "POST collection endpoints MUST have a request body schema that includes paging parameters."
severity: error
given: $.paths[?([email protected](/.*\/\{[^}]+\}$/))].post.requestBody.content.application/json.schema.properties.paging
then:
field: "type"
function: pattern
functionOptions:
match: "object"

##### FILTERING #####
sps-disallow-resource-identifier-filtering:
description: "Resource identifier filtering is not allowed as a query parameter. Use the resource identifier in the URL path."
severity: warn
given: $.paths..get.parameters.[?(@.in=='query' && @.name=='id')]
then:
field: "name"
function: pattern
functionOptions:
notMatch: "^id$"

sps-filtering-only-get-requests:
description: "Only GET-based endpoints SHOULD have have the query parameter 'filter'."
severity: error
given: $.paths.*[?(@property!='get')].parameters.[?(@.in=='query' && @.name=='filter')].name
then:
function: falsy

# Commented out because
# https://github.com/SPSCommerce/sps-api-standards/pull/86#discussion_r1680361945
# sps-unreasonable-query-parameters-limit:
# description: "Filtering query parameters SHOULD have a reasonable limit, no more than 12."
# severity: warn
# given: $.paths..get
# then:
# function: schema
# functionOptions:
# schema:
# type: object
# properties:
# parameters:
# type: array
# minContains: 0
# maxContains: 12
# contains:
# type: object
# properties:
# in:
# const: query

sps-hybird-filtering-exists-with-root-filter:
description: "Hybrid filtering MAY be offered on multiple attributes, but MUST never exist if a root \"filter\" query parameter is present."
severity: error
given: $.paths..get.parameters^
then:
function: schema
functionOptions:
schema:
type: object
properties:
parameters:
type: array
items:
type: object
properties:
name:
type: string
in:
type: string
allOf:
- if:
properties:
parameters:
type: array
contains:
type: object
properties:
name:
const: filter
then:
not:
properties:
parameters:
type: array
contains:
type: object
properties:
name:
type: string
pattern: \w+Filter

##### SORTING #####
sps-sorting-parameters-only-get-requests:
description: "Non-GET endpoints MUST NOT have sorting query parameters. Parameter names such as sort, sorting, orderBy, etc."
severity: error
given: $.paths.*[?(@property!='get')].parameters.[?(@.in=='query')]
then:
field: "name"
function: pattern
functionOptions:
notMatch: "^sort|sorting|sortBy|order|ordering|orderBy$"

Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
const { SpectralTestHarness } = require("../harness/spectral-test-harness.js");

describe("sps-collection-missing-results-array", () => {
let spectral = null;
const ruleName = "sps-collection-missing-results-array";
const ruleset = "src/collections.ruleset.yml";

beforeEach(async () => {
spectral = new SpectralTestHarness(ruleset);
});

test("valid - collection response with results array of objects", async () => {
const spec = `
openapi: 3.0.0
info:
title: Sample API
version: 1.0.0
paths:
/users:
get:
summary: Get a list of users
responses:
'200':
description: A list of users
content:
application/json:
schema:
properties:
results:
type: array
items:
type: object
paging:
type: object
`;

await spectral.validateSuccess(spec, ruleName);
});

test("invalid - collection response - results is not an array", async () => {
const spec = `
openapi: 3.0.0
info:
title: Sample API
version: 1.0.0
paths:
/users:
get:
summary: Get a list of users
responses:
'200':
description: A list of users
content:
application/json:
schema:
properties:
results:
type: object
paging:
type: object
`;

await spectral.validateFailure(spec, ruleName, "Error", 1);
});

test("invalid - collection response - results is not an array of objects", async () => {
const spec = `
openapi: 3.0.0
info:
title: Sample API
version: 1.0.0
paths:
/users:
get:
summary: Get a list of users
responses:
'200':
description: A list of users
content:
application/json:
schema:
properties:
results:
type: array
items:
type: string
paging:
type: object
`;

await spectral.validateFailure(spec, ruleName, "Error", 1);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
const { SpectralTestHarness } = require("../harness/spectral-test-harness.js");

describe("sps-disallow-resource-identifier-filtering", () => {
let spectral = null;
const ruleName = "sps-disallow-resource-identifier-filtering";
const ruleset = "src/collections.ruleset.yml";

beforeEach(async () => {
spectral = new SpectralTestHarness(ruleset);
});

test("valid - resource identifier is within the path of the endpoint", async () => {
const spec = `
openapi: 3.0.0
info:
title: Sample API
version: 1.0.0
paths:
/users/{id}:
get:
summary: Get a list of users
parameters:
- name: id
in: path
required: true
responses:
'200':
description: A list of users
`;

await spectral.validateSuccess(spec, ruleName);
});

test("invalid - resource identifier is defined as a query parameter", async () => {
const spec = `
openapi: 3.0.0
info:
title: Sample API
version: 1.0.0
paths:
/users:
get:
summary: Get a list of users
parameters:
- name: id
in: query
required: false
`;

await spectral.validateFailure(spec, ruleName, "Warning", 1);
});
});
65 changes: 65 additions & 0 deletions rulesets/test/collections/sps-filtering-only-get-requests.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
const { SpectralTestHarness } = require("../harness/spectral-test-harness.js");

describe("sps-filtering-only-get-requests", () => {
let spectral = null;
const ruleName = "sps-filtering-only-get-requests";
const ruleset = "src/collections.ruleset.yml";

beforeEach(async () => {
spectral = new SpectralTestHarness(ruleset);
});

test("valid - GET endpoint has a filter query parameter", async () => {
const spec = `
openapi: 3.0.0
info:
title: Sample API
version: 1.0.0
paths:
/users:
get:
summary: Get a list of users
parameters:
- name: filter
in: query
required: false
responses:
'200':
description: A list of users
/employees:
get:
summary: Get a list of users
parameters:
- name: filter
in: query
required: false
responses:
'200':
description: A list of users
`;

await spectral.validateSuccess(spec, ruleName);
});

test("invalid - non-GET endpoints has filter query parameter", async () => {
const spec = `
openapi: 3.0.0
info:
title: Sample API
version: 1.0.0
paths:
/users:
post:
summary: Create a user
parameters:
- name: filter
in: query
required: false
responses:
'200':
description: Create a user
`;

await spectral.validateFailure(spec, ruleName, "Error", 1);
});
});
Loading

0 comments on commit 1ad59f3

Please sign in to comment.