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: Allow non conflicting path methods to merge #78

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
30,459 changes: 30,459 additions & 0 deletions package-lock.json

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
"@types/js-yaml": "^3.12.5",
"@types/lodash": "^4.14.150",
"@types/node": "^13.13.5",
"@typescript-eslint/eslint-plugin": "^2.31.0",
"@typescript-eslint/parser": "^2.31.0",
"@typescript-eslint/eslint-plugin": "^5.30.7",
"@typescript-eslint/parser": "^5.30.7",
"ajv": "^6.12.2",
"atlassian-openapi": "^1.0.8",
"babel-jest": "^26.0.1",
Expand Down
6 changes: 6 additions & 0 deletions packages/openapi-merge/.prettierrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
printWidth: 160,
trailingComma: 'none',
singleQuote: true,
endOfLine: 'lf'
};
199 changes: 162 additions & 37 deletions packages/openapi-merge/src/__tests__/paths.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { merge } from "..";
import { toOAS } from "./oas-generation";
import { expectMergeResult, toMergeInputs, expectErrorType } from "./test-utils";
import { SingleMergeInput } from "../data";
import { merge } from '..';
import { toOAS } from './oas-generation';
import { expectMergeResult, toMergeInputs, expectErrorType } from './test-utils';
import { SingleMergeInput, SingleMergeInputV2 } from '../data';

describe('OAS Path Merge', () => {
it('should merge paths where one paths is null', () => {
Expand Down Expand Up @@ -104,6 +104,49 @@ describe('OAS Path Merge', () => {
});
});

it('should allow duplicate operationIds when flagged to do so', () => {
const first = toOAS({
'/path/a': {
get: {
operationId: 'same',
responses: {}
}
}
});

const second = toOAS({
'/path/b': {
post: {
operationId: 'same',
responses: {}
}
}
});

const output = toOAS({
'/path/a': {
get: {
operationId: 'same',
responses: {}
}
},
'/path/b': {
post: {
operationId: 'same',
responses: {}
}
}
});

const mergeInputs: SingleMergeInputV2[] = toMergeInputs([first, second]);

mergeInputs[1]['uniqueOperations'] = false;

expectMergeResult(merge(mergeInputs), {
output
});
});

it('should prefix paths correctly', () => {
const first = toOAS({
'/path/a': {
Expand All @@ -121,7 +164,7 @@ describe('OAS Path Merge', () => {
}
});

expectMergeResult(merge([{ oas: first, pathModification: { prepend: '/service'}}]), {
expectMergeResult(merge([{ oas: first, pathModification: { prepend: '/service' } }]), {
output
});
});
Expand All @@ -143,7 +186,7 @@ describe('OAS Path Merge', () => {
}
});

expectMergeResult(merge([{ oas: first, pathModification: { stripStart: '/rest'}}]), {
expectMergeResult(merge([{ oas: first, pathModification: { stripStart: '/rest' } }]), {
output
});
});
Expand All @@ -165,17 +208,12 @@ describe('OAS Path Merge', () => {
}
});

expectMergeResult(merge([{ oas: first, pathModification: { stripStart: '/rest', prepend: '/service' }}]), {
expectMergeResult(merge([{ oas: first, pathModification: { stripStart: '/rest', prepend: '/service' } }]), {
output
});
});

/**
* TODO this is simpler logic to implement for now but, ideally, we would merge paths together if we could, if
* the HTTP methods do not overlap. I can see a use case for two different services providing different methods
* on the same path.
*/
it('should return an error if there are duplicate paths (simple case)', () => {
it('should return an error if there are duplicate paths and methods (simple case)', () => {
const first = toOAS({
'/path/a': {
get: {
Expand All @@ -186,7 +224,7 @@ describe('OAS Path Merge', () => {

const second = toOAS({
'/path/a': {
post: {
get: {
responses: {}
}
}
Expand All @@ -195,7 +233,7 @@ describe('OAS Path Merge', () => {
expectErrorType(merge(toMergeInputs([first, second])), 'duplicate-paths');
});

it('should return an error if modifying a path would result in a duplicate', () => {
it('should return an error if modifying a path would result in a duplicate method', () => {
const first = toOAS({
'/path/a': {
get: {
Expand All @@ -206,7 +244,7 @@ describe('OAS Path Merge', () => {

const second = toOAS({
'/service/rest/path/a': {
post: {
get: {
responses: {}
}
}
Expand All @@ -229,6 +267,86 @@ describe('OAS Path Merge', () => {
expectErrorType(merge([firstInput, secondInput]), 'duplicate-paths');
});

it('should allow duplicate paths with non-overlapping methods, resulting in a merged path', () => {
const first = toOAS({
'/path/a': {
get: {
responses: {}
}
}
});

const second = toOAS({
'/path/a': {
post: {
responses: {}
}
}
});

const output = toOAS({
'/path/a': {
get: {
responses: {}
},
post: {
responses: {}
}
}
});

expectMergeResult(merge(toMergeInputs([first, second])), {
output
});
});

it('should allow duplicate path with alternate methods if ther is no conflict', () => {
const first = toOAS({
'/path/a': {
get: {
responses: {}
}
}
});

const second = toOAS({
'/service/rest/path/a': {
post: {
responses: {}
}
}
});

const firstInput: SingleMergeInput = {
oas: first,
pathModification: {
prepend: '/rest'
}
};

const secondInput: SingleMergeInput = {
oas: second,
pathModification: {
stripStart: '/service'
}
};

const output = toOAS({
'/rest/path/a': {
get: {
responses: {}
},
post: {
responses: {}
}
}
});

expectMergeResult(merge([firstInput, secondInput]), {
output
});
});

describe('Tag Exclusion', () => {
it('should strip out Path Items with no operations', () => {
const first = toOAS({
Expand Down Expand Up @@ -359,7 +477,7 @@ describe('OAS Path Merge', () => {
}
});

expectMergeResult(merge([{ oas: first, operationSelection: { excludeTags: ['excluded'] }}, { oas: second }]), {
expectMergeResult(merge([{ oas: first, operationSelection: { excludeTags: ['excluded'] } }, { oas: second }]), {
output
});
});
Expand Down Expand Up @@ -428,7 +546,7 @@ describe('OAS Path Merge', () => {
}
});

expectMergeResult(merge([{ oas: first, operationSelection: { includeTags: ['included'] }}, { oas: second }]), {
expectMergeResult(merge([{ oas: first, operationSelection: { includeTags: ['included'] } }, { oas: second }]), {
output
});
});
Expand Down Expand Up @@ -491,7 +609,7 @@ describe('OAS Path Merge', () => {
}
});

expectMergeResult(merge([{ oas: first, operationSelection: { includeTags: ['included'], excludeTags: ['excluded'] }}, { oas: second }]), {
expectMergeResult(merge([{ oas: first, operationSelection: { includeTags: ['included'], excludeTags: ['excluded'] } }, { oas: second }]), {
output
});
});
Expand Down Expand Up @@ -521,16 +639,20 @@ describe('OAS Path Merge', () => {
}
});

first.tags = [{
name: 'included',
description: 'This tag is included'
}, {
name: 'excluded',
description: 'This tag is excluded'
}, {
name: 'unused',
description: 'This tag is not used'
}];
first.tags = [
{
name: 'included',
description: 'This tag is included'
},
{
name: 'excluded',
description: 'This tag is excluded'
},
{
name: 'unused',
description: 'This tag is not used'
}
];

const second = toOAS({
'/path/b': {
Expand All @@ -554,17 +676,20 @@ describe('OAS Path Merge', () => {
}
});

output.tags = [{
name: 'included',
description: 'This tag is included'
}, {
name: 'unused',
description: 'This tag is not used'
}];
output.tags = [
{
name: 'included',
description: 'This tag is included'
},
{
name: 'unused',
description: 'This tag is not used'
}
];

expectMergeResult(merge([{ oas: first, operationSelection: { excludeTags: ['excluded'] } }, { oas: second }]), {
output
});
});
});
});
});
Loading