Add support for multiple exclusion policies, add Default group (enabled by default)

[Seldaek]

Changes/Additions:

• All properties without a group defined are now considered to have the group "Default" (consistent with Validator component)
• By default, the serializer only takes the group "Default" (consistent with Validator component) - possible to change by overriding Serializer::createDefaultExclusionStrategy
• Setting an empty group array enables only "Default" - to disable groups completely one can use Serializer::setExclusionStrategy(null)
• Added Serializer::addExclusionStrategy that adds a new one in the chain, but keeps the current ones (useful to add custom strategies safely)

Fixes #107

[schmittjoh]

I'm not really comfortable with adding this here.

If this is valuable, then we need to think about adding a dedicated builder object for the serializer.

[Seldaek]

Well we actually use it already.. because in one case we have some weird strategy that depends on a who sees the data, and we exclude fields based on that, so on some actions we just add that strategy to it. I don't see what a builder would bring apart from more code to write.

[schmittjoh]

A builder would separate the concerns, and make the code more readable.

$container->get('serializer.builder')->enableGroups(array('Foo'))->setVersion('1.0')->addExclusionStrategy($foo)->build();

Right now, the serializer is container scoped, and when you retrieve it, and start adding strategies, you don't really know what internal state it is in. We would also need to add methods to reset strategies because it is stateful at the moment. The more methods we add the more complicated this gets.

[Seldaek]

I do see your point, and I agree it's probably safer. It takes away a lot of convenience though when using FOSRestBundle that means you can't just configure the serializer and then return an array of data, you'll have to create a view and give it the serializer (which isn't even possible right now, but fixable). Anyway if that's the only problem you have with the PR I happily fix it.

[schmittjoh]

It should be doable to re-add the convenience methods. Maybe we need to allow people to configure several different serializers (in JMSSerializerBundle), and then add an annotation to the FOSRestBundle to tell it which serializer to retrieve.

[Seldaek]

Ok I added a builder, it's 35°C here though so it's probably not correct. Just wanted to get some feedback. I left the getDefaultExclusionStrategy method in the Serializer, because I think it should still use grouping by default no matter what, and I think it should be easy to override by extending it.

[schmittjoh]

Hehe, no problem, I will take a look; we have a rain period here ;)

Would you mind submitting a PR just with the group changes? That part is really uncontroversial, e.g. GroupsExclusionStrategy changes plus related test changes.

[Seldaek]

I'd rather keep it all in one and work on getting it through ASAP if you
give me feedback. Maybe stop by on irc so we can discuss it.

[schmittjoh]

Basically, I'd like to move the builder functionality of the serializer into the builder object. We could also allow people to configure several different serializers beforehand:

jms_serializer:
    serializers:
        foo:
            groups: [a,b,c]
            version: 1.2
            exclusion_strategy: some_id

$container->get('foo_serializer') // gets the configured serializer
$container->get('jms_serializer.builder')->setGroups(array('foo'))->getSerializer() // allows to build one dynamically at runtime
$container->get('jms_serializer.builder')->loadConfig('foo')->setVersion('1.0')->getSerializer() // creates a new serializer using the config of the "foo" serializer as a base

The public API of the serializer would change to only serialize and deserialize with no more setExclusionStrategy etc. methods.

[schmittjoh]

Maybe change this to jms_serializer.builder to make it a bit shorter, and it should be prototype-scoped.

[Seldaek]

@schmittjoh ok I amended the builder commit, please take another look.

[schmittjoh]

Yeah, like it.

I have played a bit with the configuration to allow multiple serializers to be configured, but it takes a bit longer than I thought. For reference, I pushed my current wip here:
3d2d560

[Seldaek]

Alright, do you think it's mergeable as is or do you want to merge only once you have the config stuff done?

[schmittjoh]

I'll wait until the config is done as I don't know how much BC is kept, and I'd like to only break BC once.

[wheelsandcogs]

Hi - would this PR allow the scenario where you want different exclusion strategies for serialize and deserialize? I have the following use case:

New User object as JSON data sent to a FOSRestBundle action e.g. POST /users - password should be deserialized so it can be stored. Retrieving the same User e.g. GET /users/1 - should obviously avoid returning the hashed password.

I'm trying to do this with import / export groups, but by default no groups are applied so if you forget to set them by default everything is exposed.

I'm sorry if this is a bit off topic but I thought it'd be worth mentioning before this is set in stone.

[Seldaek]

Yup this PR should allow this fairly easily. You'd do for example
setGroups(array('Default', 'import')), and it'll take all fields without
a group + those with import, but not export.

[Seldaek]

@schmittjoh any news on the config front? It'd be nice to get this merged sooner or later because the more we wait the more likely it is that it will impact people.

[schmittjoh]

Yeah :/ didn't find time, yet.

[NicolasBadey]

+1, we also need multiple exclusion policies

[docteurklein]

+1 too

[mvrhov]

+1

[schmittjoh]

@Seldaek, I'd like to split this into two separate patches: one for the group changes, and then one for the builder refactoring. The reasoning being, that I'm not yet comfortable with the builder. I can try to extract the parts, but I'm not sure whether I can keep the single commits then, would you mind that?

@NicolasBadey, @docteurklein, @mvrhov, could you explain the use case that you have for multiple exclusion policies? It would help me better understand what we need here.

[Seldaek]

The thing is the group stuff alone is kinda useless, because as soon as you have an exclusion policy you'll want the groups to remain applied IMO. But if you like to tear the stuff apart from this PR feel free.

[mvrhov]

Right now what I'm interested in from this PR is just the same as symfony validator behavior.

[NicolasBadey]

@schmittjoh for combine Version and Groups on the same porperty

[mvrhov]

I don't need what NicolasBadey said ATM, but in the future it would be more than welcome feature.

[Seldaek]

@schmittjoh FYI I rebased this PR on latest master

[Narretz]

How much of that has been implemented? I think Default groups are there, right?

[WishCow]

+1, also waiting on this PR.