2
\$\begingroup\$

I have three different types of schema possibilities as below:

obj_1 = {
    'title': 'deposit-test-schema',
    'type': 'object',
    'properties': {
        'title': {'type': 'string', 'x-cap-permission': {"users": ['[email protected]']}},
        'date': {'type': 'string', 'x-cap-permission': {"users": ['[email protected]']}},
        'test': {
            'type': 'array',
            'items': {
                'properties': {
                    'title': {'type': 'string', 'x-cap-permission': {"users": ['[email protected]']}},
                    'date': {'type': 'string', 'x-cap-permission': {"users": ['[email protected]']}}
                },
                "type": "object"
            },
        }
    },
}
obj_2 = {
    'title': 'deposit-test-schema-one',
    'type': 'array',
    'items': {
        'properties': {
            'title': {'type': 'string', 'x-cap-permission': {"users": ['[email protected]']}},
            'date': {'type': 'string', 'x-cap-permission': {"users": ['[email protected]']}}
        },
        "type": "object"
    },
}
obj_3 = {
    'title': 'deposit-test-schema-two',
    'type': 'string',
    'x-cap-permission': {"users": ['[email protected]']}
}

I want the output to be the nested structure with x-cap-fields. The expected output is:

{
   "title":{
      "users":[
         "[email protected]"
      ]
   },
   "date":{
      "users":[
         "[email protected]"
      ]
   },
   "test.items.properties.title":{
      "users":[
         "[email protected]"
      ]
   },
   "test.items.properties.date":{
      "users":[
         "[email protected]"
      ]
{
   "items.title":{
      "users":[
         "[email protected]"
      ]
   },
   "items.date":{
      "users":[
         "[email protected]"
      ]
   }
}
{'.': {'users': ['[email protected]']}}

I have implemented the following solution to have the expected output:

def parse_schema_permission_info(schema):
    x_cap_fields = {}

    def extract_permission_field(field, parent_field):
        for field, value in field.items():
            if field == 'x-cap-permission':
                x_cap_fields.update({parent_field: value})
            if isinstance(value, dict):
                key = parent_field + '.' + field
                if value.get('x-cap-permission'):
                    x_cap_fields.update(
                        {key: value.get('x-cap-permission')}
                    )
                extract_permission_field(value, key)

    schema_type = schema.get("type")
    pre_field = ''
    if schema_type == 'object':
        schema = schema.get('properties', {})
    elif schema_type == 'array':
        schema = schema.get('items',{}).get('properties', {})
        pre_field = 'items'
    elif schema_type == 'string':
        extract_permission_field(schema, '.')
        print(x_cap_fields)
        return

    for field in schema:
        p_field = pre_field + '.' + field if pre_field else field
        extract_permission_field(schema.get(field), p_field)

    print(x_cap_fields)

What can be improved here to have the same expected output from the above method?

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

First I will say, the fact that you're doing any of this raises just all of the red flags. This code and the data it consumes and outputs have all of the hallmarks of poorly-structured data lasagna. Better-structured code would have intermediate models with well-defined types and fields. Also, a better architecture would know which of the three formats it's consuming based on context, and more narrowly tailor a separate schema transformation to each case. But anyway:

This is a recursive implementation, and Python sucks at recursion. For untrusted data, it would be trivially easy to construct a stack bomb and crash your program.

You have good test cases! You should convert them into unit tests.

Add PEP484 type hints.

Notice that you've duplicated this logic:

if field == 'x-cap-permission':

once for the current node and once for the child node. Just do it once.

You abuse update(). When there's only one key to update, just [] = assign it.

Extract extract_permission_field away from being nested, and convert it from a mutator of x_cap_fields into a function that has no side effects and yields an iterator of key-value pairs.

Consider using the new match/case syntax for your schema_type check.

pre_field can be simplified by using the literal items., and unconditionally pre-pending.

Suggested

This is certainly only a half-measure and doesn't address broader architectural problems, but it's better than nothing:

from typing import Any, Iterator


def extract_permission_field(node: dict[str, Any], parent_field: str) -> Iterator[tuple[str, Any]]:
    for field, value in node.items():
        if field == 'x-cap-permission':
            yield parent_field, value
        if isinstance(value, dict):
            yield from extract_permission_field(value, parent_field + '.' + field)


def parse_schema_permission_info(schema: dict[str, Any]) -> dict[str, Any]:
    match schema.get('type'):
        case 'string':
            return dict(extract_permission_field(schema, '.'))
        case 'object':
            schema = schema.get('properties', {})
            pre_field = ''
        case 'array':
            schema = schema.get('items', {}).get('properties', {})
            pre_field = 'items.'
        case _:
            raise ValueError('Bad schema type')

    x_cap_fields = {}
    for field, value in schema.items():
        x_cap_fields.update(extract_permission_field(value, pre_field + field))

    return x_cap_fields


def test() -> None:
    obj_1 = {
        'type': 'object',
        'title': 'deposit-test-schema',
        'properties': {
            'title': {'type': 'string', 'x-cap-permission': {'users': ['[email protected]']}},
            'date': {'type': 'string', 'x-cap-permission': {'users': ['[email protected]']}},
            'test': {
                'type': 'array',
                'items': {
                    'type': 'object',
                    'properties': {
                        'title': {'type': 'string', 'x-cap-permission': {'users': ['[email protected]']}},
                        'date': {'type': 'string', 'x-cap-permission': {'users': ['[email protected]']}},
                    },
                },
            },
        },
    }
    actual = parse_schema_permission_info(obj_1)
    expected = {
       'title': {'users': ['[email protected]']},
       'date': {'users': ['[email protected]']},
       'test.items.properties.title': {'users': ['[email protected]']},
       'test.items.properties.date': {'users': ['[email protected]']},
    }
    assert actual == expected

    obj_2 = {
        'type': 'array',
        'title': 'deposit-test-schema-one',
        'items': {
            'type': 'object',
            'properties': {
                'title': {'type': 'string', 'x-cap-permission': {'users': ['[email protected]']}},
                'date': {'type': 'string', 'x-cap-permission': {'users': ['[email protected]']}},
            },
        },
    }
    actual = parse_schema_permission_info(obj_2)
    expected = {
       'items.title': {'users': ['[email protected]']},
       'items.date': {'users': ['[email protected]']},
    }
    assert actual == expected

    obj_3 = {
        'type': 'string',
        'title': 'deposit-test-schema-two',
        'x-cap-permission': {'users': ['[email protected]']},
    }
    actual = parse_schema_permission_info(obj_3)
    expected = {'.': {'users': ['[email protected]']}}
    assert actual == expected


if __name__ == '__main__':
    test()
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for your valuable suggestions. The schema architectural problems are indeed true but i can't do much about it. On the code side, I have included a number of unit tests and integration tests to make sure the code runs fine. I will keep your pointers in mind and improve. \$\endgroup\$
    – ParthS007
    Sep 28, 2022 at 14:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.