Skip to content
This repository has been archived by the owner on Jun 28, 2024. It is now read-only.

feat: (experiment) set id and *at fields readOnly in OAS3 #166

Closed
wants to merge 3 commits into from

Conversation

displague
Copy link
Member

@displague displague commented Oct 18, 2023

I was curious what effect more accurate readOnly marking would have on the generated go. Apparently none 😅

This draft can serve as a patch for upstream accuracy changes. It makes id, *_at (hand-picked, alll?), and status fields readOnly.

It is somewhat academic since the request schemas are generally distinct from the response schemas. I think it would be safe to argue that every field in the request schemas should be writeOnly today while every field in the response schemas should be readOnly.

I'm optimistic that a representation of the OAS3 schemas can someday offer unified "Foo" schemas, serving as response, update, and create schemas by taking advantage of readOnly and writeOnly.

Until that time, it may be confusing to have inconsistency where some fields are explicitly marked readOnly while others are not, especially where the request and response schemas are separate.

I'm optimistic

Perhaps too optimistic:

The most practical approach may be to define the base common schema with readOnly and writeOnly fields and then extend that schema for Create and/or Update as needed to define immutable fields. However, offering a schema that sometimes presents Create/Update schemas may lead to confusion.

Similar improvements to make:


The change was automated and mechanical with a manual (and painful) git add -p to dismiss unwanted yaml formatting changes. (Challenge: how fast can you s, <enter>, y, <enter>, n, <enter>, ... without having to K).

If a standard formatting was applied to the schema files I could have applied that formatting after running this script. The diff would have been minimized.

import os
import yaml

def update_yaml_fields(yaml_file_path, fields_to_ro):
    """
    Update specific fields in a given YAML file to be "readOnly: true".

    Args:
    - yaml_file_path (str): Path to the YAML file to be updated.
    - fields_to_update (list): List of fields to be updated to "readOnly: true".
    """
    found = False
    # Load the YAML content
    with open(yaml_file_path, 'r') as file:
        content = yaml.safe_load(file)

    # Check if the YAML content has the OAS3 structure
    if 'properties' in content:
        for schema_name, schema_content in content['properties'].items():
            if schema_name in fields_to_ro:
                if 'readOnly' not in schema_content:
                    found = True
                    schema_content['readOnly'] = True

            if schema_name == 'id':
                if 'id' not in content.get('required', []):
                    found = True
                    content.setdefault('required', []).append('id')

    # Write the updated content back to the file
    if found:
        with open(yaml_file_path, 'w') as file:
            yaml.dump(content, file, sort_keys=False)

def main():
    # Fields to update
    fields_to_ro = ["id", "created_at", "updated_at", "ended_at", "requested_at", "last_login_at", "enforce_2fa_at", "requested_by", "status"] # Add or remove fields as needed

    # Walk through directories structured like OAS3. components/schemas/...
    for root, dirs, files in os.walk('.'):
        for file in files:
            if file.endswith('.yaml'):
                yaml_file_path = os.path.join(root, file)
                update_yaml_fields(yaml_file_path, fields_to_ro)

if __name__ == "__main__":
    main()

(updated to require id when present)

@displague displague marked this pull request as draft October 18, 2023 14:05
@displague displague changed the title feat: set id and *at fields readOnly in OAS3 feat: (experiment) set id and *at fields readOnly in OAS3 Oct 18, 2023
@ctreatma ctreatma closed this Jun 28, 2024
@ctreatma ctreatma deleted the readonly-id-and-at branch June 28, 2024 15:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants