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

Fix duplicate operation id caused by use of 2 methods GET/POST method from getKeywordById api #7586

Merged
merged 4 commits into from
Jan 5, 2024

Conversation

ianwallen
Copy link
Contributor

@ianwallen ianwallen commented Dec 29, 2023

An api should not do GET/POST in the same operation as it seems to cause issues in open api specification document.
It caused duplicates duplicate OperationID that would occur (getKeywordById and getKeywordById_1)

This PR will split 2 OperationID (getKeywordById and getKeywordByIds)

Checklist

  • I have read the contribution guidelines
  • Pull request provided for main branch, backports managed with label
  • Good housekeeping of code, cleaning up comments, tests, and documentation
  • Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
  • Clean commit messages, longer verbose messages are encouraged
  • API Changes are identified in commit messages
  • Testing provided for features or enhancements using automatic tests)
  • User documentation provided for new features or enhancements in mannual
  • Build documentation provided for development instructions in README.md files
  • Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

A get method should not generally support a POST?
It caused duplicates duplicate OperationID that would occur (getKeywordById and getKeywordById_1)
@ianwallen ianwallen added this to the 4.4.2 milestone Dec 29, 2023
@ianwallen ianwallen added backport 4.2.x api change Indicate a change in the API labels Dec 29, 2023
@ianwallen ianwallen changed the title Remove POST method from getKeywordById api Depreciated GET in favor for POST method from getKeywordById api Jan 3, 2024
@ianwallen ianwallen changed the title Depreciated GET in favor for POST method from getKeywordById api Fix duplicate operation id caused by use of 2 methods GET/POST method from getKeywordById api Jan 3, 2024
"Use POST method if you want to support larger parameters list"
)
@RequestMapping(
path = "/keywordById",
Copy link
Member

Choose a reason for hiding this comment

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

Better to keep GET unchanged because it is used in all records when XLink mode is enabled in a catalogue (so it will avoid migration in metadata records). And we can change POST (with keywordByIds) and its (probably only) usage in https://github.com/geonetwork/core-geonetwork/blob/main/web-ui/src/main/resources/catalog/components/utility/UtilityService.js#L709-L710. Then it looks good. @josegar74 can you think of any other usages of this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fxprunayre
Sorry - I had accidently changed the endpoint to /keywordById. I reverted it back to /keyword for both get and post.

@ianwallen ianwallen merged commit 6c8c450 into geonetwork:main Jan 5, 2024
6 checks passed
@ianwallen ianwallen deleted the fix_keywords_api branch January 5, 2024 10:58
ianwallen added a commit that referenced this pull request Jan 5, 2024
… from getKeywordById api (#7586)

* Remove POST method from getKeywordById. api
[BP] A get method should not generally support a POST?
It caused duplicates duplicate OperationID that would occur (getKeywordById and getKeywordById_1)

* Re added POST method for keyword and depreciated the GET api.

* Use common private getKeyword function for both api calls.

* Revert endpoint back to /keyword for GET
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api change Indicate a change in the API backport 4.2.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants