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

djangorestframework_camel_case.camelize_serializer_fields does not handle "ignore_keys" #945

Closed
tomashchuk opened this issue Feb 24, 2023 · 6 comments
Labels
bug Something isn't working fix confirmation pending issue has been fixed and confirmation from issue reporter is pending

Comments

@tomashchuk
Copy link
Contributor

drf_spectacular.contrib.djangorestframework_camel_case.camelize_serializer_fields does not handle "ignore_keys"

Define for example

REST_FRAMEWORK = {
    "JSON_UNDERSCOREIZE": {
          "ignore_keys": ("version_8test", ),
    },
} 

then add

SPECTACULAR_SETTINGS = {
    'POSTPROCESSING_HOOKS': [
        'drf_spectacular.contrib.djangorestframework_camel_case.camelize_serializer_fields'
    ],
}

Actual behavior
{"version8test": 4294967295}

Expected behavior
{"version_8test": 4294967295}

Let me know if it is expected behavior by default otherwise, I will create a PR with the fix.

@tfranzel
Copy link
Owner

tfranzel commented Feb 24, 2023

good catch! looks like a bug and not expected behavior. I think this was kind of working before, but broke with bugfixing another issue.

we used camelize (which includes the ignore list) before, but that was only intended for modifying data and not a schema as far as I remember. there were issues like #861. we went one step deeper and used their building blocks instead. it was an incomplete transition from the looks of it.

yes, PR a is welcome.

code is here:

https://github.com/tfranzel/drf-spectacular/blob/master/drf_spectacular/contrib/djangorestframework_camel_case.py

@tfranzel tfranzel added the bug Something isn't working label Feb 24, 2023
@tomashchuk
Copy link
Contributor Author

@tfranzel It could be a possible solution for the issue with "ignore_keys". But djangorestframework_camel_case also handles "ignore_fields" and it needs to be solved too. I could not run tests locally because of django.core.exceptions.ImproperlyConfigured: Could not find the GDAL library. Maybe I will try later to setup it up locally and fix these issues but I did not run tests for these changes. Tested only with my project.

import re


def camelize_serializer_fields(result, generator, request, public):
    from djangorestframework_camel_case.util import camelize_re, underscore_to_camel
    from djangorestframework_camel_case.settings import api_settings

    # ignore_fields = api_settings.JSON_UNDERSCOREIZE.get("ignore_fields") or () should be handled too
    ignore_keys = api_settings.JSON_UNDERSCOREIZE.get("ignore_keys") or ()

    def camelize_str(key: str):
        new_key = re.sub(camelize_re, underscore_to_camel, key) if "_" in key else key
        if key in ignore_keys or new_key in ignore_keys:
            return key
        return new_key

    def camelize_component(schema: dict):
        if schema.get('type') == 'object':
            if 'properties' in schema:
                schema['properties'] = {
                    camelize_str(field_name): camelize_component(field_schema)
                    for field_name, field_schema in schema['properties'].items()
                }
            if 'required' in schema:
                schema['required'] = [camelize_str(field) for field in schema['required']]
        elif schema.get('type') == 'array':
            camelize_component(schema['items'])
        return schema

    for (_, component_type), component in generator.registry._components.items():
        if component_type == 'schemas':
            camelize_component(component.schema)

    # inplace modification of components also affect result dict, so regeneration is not necessary
    return res

@tfranzel
Copy link
Owner

sry about that GDAL thing. I need to find a mitigation for that. Can't expect anyone to install libgdal for this.

uncomment this line and the tests should run (exept of course the 2 GIS tests):

'rest_framework_gis',

tfranzel added a commit that referenced this issue Feb 25, 2023
 #775 #777

will coventiently make ``./runtests.py`` brush over system library
requirements to help contributors run the test suite locally without
installing extra stuff.

full tox run will require it though.
@tfranzel
Copy link
Owner

./runtests.py should now work without the library installed. full tox run will require it though.

@tomashchuk
Copy link
Contributor Author

@tfranzel Ok, I will try when having some time

tfranzel added a commit that referenced this issue Mar 4, 2023
…re_fields" #945

Thx to @tomashchuk for raising the issue and providing part of the fix

Co-authored-by: tomashchuk <[email protected]>
@tfranzel tfranzel added the fix confirmation pending issue has been fixed and confirmation from issue reporter is pending label Mar 4, 2023
@tfranzel
Copy link
Owner

tfranzel commented Mar 4, 2023

closed with release 0.26.0

@tfranzel tfranzel closed this as completed Mar 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix confirmation pending issue has been fixed and confirmation from issue reporter is pending
Projects
None yet
Development

No branches or pull requests

2 participants