-
Notifications
You must be signed in to change notification settings - Fork 14
CLOUDP-285964: Adds IPA rule 104 - Resource has get #301
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
a03356b
CLOUDP-285964: Adds IPA rule 104 - Resource has get
lovisaberggren d509d03
CLOUDP-285964: Clean up
lovisaberggren 4cb147f
CLOUDP-285964: Merge main
lovisaberggren 6e3b711
CLOUDP-285964: Move /ipa to /spectral folder
lovisaberggren cd65e45
CLOUDP-285964: Merge main
lovisaberggren 2884f15
CLOUDP-285964: Add links in js doc
lovisaberggren 65a79db
CLOUDP-285964: Add nested and nested singleton test case
lovisaberggren 99c6d47
CLOUDP-285964: Add more unit tests
lovisaberggren File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
88 changes: 88 additions & 0 deletions
88
tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
import { describe, expect, it } from '@jest/globals'; | ||
import { | ||
getResourcePaths, | ||
isSingletonResource, | ||
isStandardResource, | ||
} from '../../rulesets/functions/utils/resourceEvaluation'; | ||
|
||
const standardResourcePaths = ['/standard', '/standard/{id}']; | ||
|
||
const nestedStandardResourcePaths = ['/standard/{exampleId}/nested', '/standard/{exampleId}/nested/{exampleId}']; | ||
|
||
const standardResourceWithCustomPaths = ['/customStandard', '/customStandard/{id}', '/customStandard:method']; | ||
|
||
const singletonResourcePaths = ['/singleton']; | ||
|
||
const singletonResourceWithCustomPaths = ['/customSingleton', '/customSingleton:method']; | ||
|
||
const nestedSingletonResourcePaths = ['/standard/{exampleId}/nestedSingleton']; | ||
|
||
describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () => { | ||
describe('getResourcePaths', () => { | ||
it('returns the paths for a resource based on parent path', () => { | ||
const allPaths = standardResourcePaths.concat( | ||
nestedStandardResourcePaths, | ||
standardResourceWithCustomPaths, | ||
singletonResourcePaths, | ||
singletonResourceWithCustomPaths, | ||
nestedSingletonResourcePaths | ||
); | ||
expect(getResourcePaths('/standard', allPaths)).toEqual(standardResourcePaths); | ||
expect(getResourcePaths('/standard/{exampleId}/nested', allPaths)).toEqual(nestedStandardResourcePaths); | ||
expect(getResourcePaths('/customStandard', allPaths)).toEqual(standardResourceWithCustomPaths); | ||
expect(getResourcePaths('/singleton', allPaths)).toEqual(singletonResourcePaths); | ||
expect(getResourcePaths('/customSingleton', allPaths)).toEqual(singletonResourceWithCustomPaths); | ||
expect(getResourcePaths('/standard/{exampleId}/nestedSingleton', allPaths)).toEqual(nestedSingletonResourcePaths); | ||
}); | ||
}); | ||
describe('isStandardResource', () => { | ||
it('returns true for a standard resource', () => { | ||
expect(isStandardResource(standardResourcePaths)).toBe(true); | ||
}); | ||
|
||
it('returns true for a standard resource with custom methods', () => { | ||
expect(isStandardResource(standardResourceWithCustomPaths)).toBe(true); | ||
}); | ||
|
||
it('returns true for a nested standard resource', () => { | ||
expect(isStandardResource(nestedStandardResourcePaths)).toBe(true); | ||
}); | ||
|
||
it('returns false for a singleton resource', () => { | ||
expect(isStandardResource(singletonResourcePaths)).toBe(false); | ||
}); | ||
|
||
it('returns false for a singleton resource with custom methods', () => { | ||
expect(isStandardResource(singletonResourceWithCustomPaths)).toBe(false); | ||
}); | ||
|
||
it('returns false for a nested singleton resource', () => { | ||
expect(isStandardResource(nestedSingletonResourcePaths)).toBe(false); | ||
}); | ||
}); | ||
describe('isSingletonResource', () => { | ||
it('returns true for a singleton resource', () => { | ||
expect(isSingletonResource(singletonResourcePaths)).toBe(true); | ||
}); | ||
|
||
it('returns true for a singleton resource with custom methods', () => { | ||
expect(isSingletonResource(singletonResourceWithCustomPaths)).toBe(true); | ||
}); | ||
|
||
it('returns true for a nested singleton resource', () => { | ||
expect(isSingletonResource(nestedSingletonResourcePaths)).toBe(true); | ||
}); | ||
|
||
it('returns false for a standard resource', () => { | ||
expect(isSingletonResource(standardResourcePaths)).toBe(false); | ||
}); | ||
|
||
it('returns false for a standard resource with custom methods', () => { | ||
expect(isSingletonResource(standardResourceWithCustomPaths)).toBe(false); | ||
}); | ||
|
||
it('returns false for a nested standard resource', () => { | ||
expect(isSingletonResource(nestedStandardResourcePaths)).toBe(false); | ||
}); | ||
}); | ||
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,9 +8,9 @@ export function isCustomMethod(path) { | |
|
||
/** | ||
* Checks if a resource is a singleton resource ({@link https://docs.devprod.prod.corp.mongodb.com/ipa/113 IPA-113}) based on the paths for the | ||
* resource. The resource may have custom methods. | ||
* resource. The resource may have custom methods. Use {@link getResourcePaths} to get all paths of a resource. | ||
* | ||
* @param resourcePaths all paths for the resource as an array of strings | ||
* @param resourcePaths all paths for the resource to be evaluated as an array of strings | ||
* @returns {boolean} | ||
*/ | ||
export function isSingletonResource(resourcePaths) { | ||
|
@@ -23,9 +23,9 @@ export function isSingletonResource(resourcePaths) { | |
|
||
/** | ||
* Checks if a resource is a standard resource ({@link https://docs.devprod.prod.corp.mongodb.com/ipa/103 IPA-103}) based on the paths for the | ||
* resource. The resource may have custom methods. | ||
* resource. The resource may have custom methods. Use {@link getResourcePaths} to get all paths of a resource. | ||
* | ||
* @param resourcePaths all paths for the resource as an array of strings | ||
* @param resourcePaths all paths for the resource to be evaluated as an array of strings | ||
* @returns {boolean} | ||
*/ | ||
export function isStandardResource(resourcePaths) { | ||
|
@@ -58,6 +58,9 @@ export function hasGetMethod(pathObject) { | |
*/ | ||
export function getResourcePaths(parent, allPaths) { | ||
const childPathPattern = new RegExp(`^${parent}/{[a-zA-Z]+}$`); | ||
const customMethodPattern = new RegExp(`^${parent}/{[a-zA-Z]+}:+[a-zA-Z]+$`); | ||
return allPaths.filter((p) => parent === p || childPathPattern.test(p) || customMethodPattern.test(p)); | ||
const customChildMethodPattern = new RegExp(`^${parent}/{[a-zA-Z]+}:+[a-zA-Z]+$`); | ||
const customMethodPattern = new RegExp(`^${parent}:+[a-zA-Z]+$`); | ||
return allPaths.filter( | ||
(p) => parent === p || childPathPattern.test(p) || customMethodPattern.test(p) || customChildMethodPattern.test(p) | ||
Comment on lines
+61
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The added tests revealed that we were missing a custom case, as it can be either |
||
); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we unit test these, and if not can we add some examples to things like
isSingletonResource
orisStandardResource
it's a bit hard to follow the intention of the method and some examples either in text or as actual tests would be of great help in the future if we need to debug or change those functionsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, makes sense, I'll add unit tests for them as well (getResourcePaths, isStandardResource and isSingletonResource as they are probably the tricky ones)