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

Nested roles #114

Open
jackqu7 opened this issue Jul 27, 2016 · 1 comment
Open

Nested roles #114

jackqu7 opened this issue Jul 27, 2016 · 1 comment
Assignees
Labels
Projects
Milestone

Comments

@jackqu7
Copy link
Collaborator

jackqu7 commented Jul 27, 2016

Description

Roles provide user of Kim with a powerful way to control what fields should be included when marshaling or serialzing a Mapper. The ability to specify a role on a nested field has been available since v1 but the existing functionality only offers so much.

This proposal outlines new functionality that would allow users to specify the name of roles attached to a Nested field from inside Role definitions on other Mappers. We will also offer greater control over how Nested fields are processed by allowing users to set specific serialize and marshal roles.

Targeting Nested field roles

class UserMapper(Mapper):
   name = field.String()
   address = field.String()

   __roles__ = {'basic': blacklist('address')}

class EventMapper(Mapper):
    name = field.String()
    location = field.String()
    user = field.Nested(UserMapper)

   __roles__ = {
       'simple': whitelist('name', 'user@basic')
   }

   mapper = EventMapper(data=json.loads(json_data)).marshal(role='simple')

So the user@basic syntax results in the nested mapper using the 'basic' role. The user@basic Role may also specify a role for a nested field too. The Nested fields would be processed the same way all the way down the chain.


Different Serialize and Marshal roles for Nested fields.

It's quite common that you wan't to allow different fields when serializing data to those permitted when marshaling. This feature will add two new properties to the NestedFieldOpts -

  1. serialize_role
  2. marshal_role
class EventMapper(Mapper):
    name = field.String()
    location = field.String()
    user = field.Nested(UserMapper, serialize_role='__default__', marshal_role='basic')

The new Nested roles syntax will allow users to specify different roles for serilaizing and marshaling

class EventMapper(Mapper):
    name = field.String()
    location = field.String()
    user = field.Nested(UserMapper)

   __roles__ = {'simple': whitelist('name', 'user@serialize:__default__', 'user@marshal:basic')}

Deciding which role to use when processing a Nested field will flow something like the following:

* if marshaling and there is a nested marshal role use that
* elif marshaling and there is a nested generic role for the field, use that`
* elif marshaling and there is a `marshal_role` option set in NestedFieldOpts, use that
* elif marshaling and there is a `role` option set in NestedFieldOpts, user that`
* else just use the `__default__` Role.
* if serializing and there is a nested serialize role, use that
* elif serializing and there is a nested generic role for the field, use that`
* elif serializing and there is a `serialize_role` option set in NestedFieldOpts, use that
* elif serializing and there is a `role` option set in NestedFieldOpts, use that`
* else just use the `__default__` Role.

Conclusion

We feel this feature is going to add a huge amount of value to Kim. Nested is already one of the best features Kim offers. Providing more options for configuring how they work will hopefully lead to some great use cases.

We would love to hear any feedback anyone has on this feature. We wan't to make sure we get it right for everyone. If you have any suggestions or even just want to let us know you like the proposed approach then please don't hesitate.

Questions

When specifying marhsal and serialize roles using the new nested role syntax should they be provided separately (as seen in the example) or should we consider another option?

Another option that we considered was specifying the role in the following form:

user@{role_name} OR serialize:{role_name},marshal:{role_name}

We might also consider using an object over a string in the Role definition.

whitelist('name', nested('field_name', role='X', 'serialize='X', marshal='Y'), 'foo', ...)

@mikeywaites mikeywaites added this to Backlog in Kim Roadmap Apr 13, 2017
@mikeywaites mikeywaites changed the title Change role of nested object within parent role Dynamic Nested roles Apr 13, 2017
@mikeywaites mikeywaites changed the title Dynamic Nested roles Nested roles Oct 1, 2017
@mikeywaites mikeywaites self-assigned this Oct 2, 2017
@mikeywaites mikeywaites modified the milestone: Release 1.2.0 Oct 6, 2017
@mikeywaites mikeywaites moved this from Backlog to In-Progress in Kim Roadmap Oct 6, 2017
@mikeywaites mikeywaites added this to the Release 1.3.0 milestone Oct 6, 2017
@mikeywaites
Copy link
Owner

I totally lost momentum with this branch but will be picking it up again next week. The TODO list I ended with was

  • implement nested_role
  • Test define nested_role
  • Handle mapper inheritance with nested_roles
  • investigate the need for bool on the nested_role set
  • deferred nested roles? - not going to be possible to support this i don't think :/
  • Require nested_role.role or nested_role.serialzie_role and nested_role.marshal role are provided (role can just be default?)
  • Support field level serialize role
  • add serialize_role field opt to nested
  • Use Nested role for nested field when serializing
  • Test serialzie
    • test get_nested_role_for(serialize)
    • test with nested_role(role='')
    • test with nested_role(serialize_role='')
    • test with Nested(seralize_role='')
    • Test serialzie many
    • Test serialzie nested collection with nested_role
    • test serialzie passes correct role to get_mapper_session
    • test nested role missing from Nested Mapper
  • Test Marshal
    • test get_nested_role_for(marshal)
    • test with nested_role(role='')
    • test with nested_role(marshal_role='')
    • test with Nested(marshal_role='')
    • Test marshal many
    • test marshal passes correct role to get_mapper_session
    • test nested role missing from Nested Mapper
  • test polymorphic mappers with nested roles ?
  • test using dynamic roles with Collection(Nested)
  • test get_mapper_session with new role opt
  • Fix Nested mappers cannot access session.parent #99. Nested role cannot access session.parent
  • Docs

It really just needs a solid test which we're going to facilitate by taking the branch for a test run by updating our mappers in the Vizibl API code base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Kim Roadmap
In-Progress
Development

No branches or pull requests

2 participants