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

CASMCMS-8959: Correct errors in API spec #112

Merged
merged 2 commits into from
Apr 4, 2024
Merged

Conversation

mharding-hpe
Copy link
Contributor

@mharding-hpe mharding-hpe commented Apr 4, 2024

This corrects several errors I found when reviewing the CFS API spec recently:

  • Several description fields have minor errors (usually seemed to be copy-paste errors when the schema was created based on another schema). A couple of example fields had the same issue.
  • The Version schema claims that the major, minor, and patch fields are integers, but in truth CFS has always returned them as string values. Looking at the source code, I can see why. Given that CFS has always behaved this way, it seems better to update the API spec to reflect reality, rather than change the API behavior.
  • The version endpoints claim (in summary and description texts) that they return lists of versions, but in truth they just return the current version.
  • For paginated responses, the next field is null when there are not additional pages. The API spec does not convey that this field may take a null value.
  • The V3SessionIdCollection schema says it is a list of sessions, but as the name implies, it actually is a list of session IDs. The actual CFS behavior reflects this.
  • Several paginated response schemas had copy-paste errors for the data array property (all of them say the property is named components, even though it actually varies based on the endpoint being paginated).
  • In several cases, the spec used $ref and specified options as siblings of it. However, in OAS 3.0.x, any siblings of a $ref are ignored. For example, if you say that a given property follows some schema (by using a $ref) but then also list that this property is readOnly or nullable, those latter options are ignored. In OAS 3.0.x, there are two ways this is generally handled.
    1. Use allOf, listing the referenced schema, and then an empty schema which has the other desired values. This option in some ways is the cleanest choice, but unfortunately a number of people have reported that a lot of tools do not handle parsing it very well. Because of that, I opted to do:
    2. Make a new schema that is a copy of the one being referenced, but with the additional settings added to it. Then reference the new schema (or just embed it).

Most of them are updates to schemas which the actual CFS code never uses, and thus will have no functional impacts. Only two of the changes impact schemas which are used by the code (V2ComponentState and Version). I reviewed the code that uses these models to make sure that the changes should not impact them. I also deployed this on mug and ran both manual CFS tests and the cmsdev CFS regression tests, and verified that it worked.

Backports:
CSM 1.5: #113
CSM 1.4: #114

@mharding-hpe mharding-hpe force-pushed the casmcms-8959-csm-1.6 branch from cc0554f to 63fc880 Compare April 4, 2024 17:44
@mharding-hpe mharding-hpe changed the base branch from develop to master April 4, 2024 17:44
@mharding-hpe mharding-hpe marked this pull request as ready for review April 4, 2024 18:22
@mharding-hpe mharding-hpe requested a review from a team as a code owner April 4, 2024 18:22
@mharding-hpe mharding-hpe merged commit aa3f589 into master Apr 4, 2024
4 checks passed
@mharding-hpe mharding-hpe deleted the casmcms-8959-csm-1.6 branch April 4, 2024 19:44
mharding-hpe added a commit that referenced this pull request Apr 4, 2024
…sm-1.6-1712259894

[chore] master -> develop from PR #112 (casmcms-8959-csm-1.6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants