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

Normalizer should define the attributes to normalize #113

Open
alexander-schranz opened this issue Mar 8, 2020 · 4 comments
Open

Normalizer should define the attributes to normalize #113

alexander-schranz opened this issue Mar 8, 2020 · 4 comments
Labels
DX Only affecting the end developer

Comments

@alexander-schranz
Copy link
Member

alexander-schranz commented Mar 8, 2020

Currently the NormalizeEnhancer only define which getters should not be called for normalization. Maybe the other way around would be better instead of defining what should be excluded it should be defined what should be included like we normally do it in sulu when using jms exlude all strategy.

There are currently 2 way to define which attributes could be serialized.

The first one is by using the Normalizer Attributes parameter [AbstractNormalizer::ATTRIBUTES => ['template', 'templateData', ...].

The second one is using Serialization Groups introducing a Content serialization group would have the advantage that the developer could just use annotations and don't need to write a custom NormalizeEnhancer for just adding new property to the normalized data.

@alexander-schranz alexander-schranz added the DX Only affecting the end developer label Mar 8, 2020
@niklasnatter
Copy link
Contributor

niklasnatter commented Mar 9, 2020

in general, i think things would be clearer if we change the behaviour to explicitly define attributes that should be normalized. right now, it feels kind of magic if you dont know that the symfony serializer is used 🙂

but we should definitely avoid that the user needs to implement a NormalizeEnhancer for simple use cases in my opinion.

@niklasnatter
Copy link
Contributor

niklasnatter commented Jan 18, 2021

A way for implementing this without the need of a Normalizer service would be adding a static DimensionContentInterface::getNormalizeAttributes method.

The ContentNormalizer service then could use this method (and a similar method on all Normalizer services) to gather the attributes that should be included in the normalized data array.

@niklasnatter niklasnatter changed the title NormalizeEnhancer should define the attributes to normalize Normalizer should define the attributes to normalize Jan 26, 2021
@niklasnatter
Copy link
Contributor

After thinking about this once again, I concluded that it would probably be a cleaner solution to not use the symfony/serializer for normalization at all. I would propose to refactor the NormalizerInterface like this:

interface NormalizerInterface
{
-    /**
-     * @return string[]
-     */
-    public function getIgnoredAttributes(object $object): array;
-
-    /**
-     * @param mixed[] $normalizedData
-     *
-     * @return mixed[]
-     */
-    public function enhance(object $object, array $normalizedData): array;
-
+    /**
+     * @return mixed[]
+     */
+    public function normalize(object $object): array;
}

In additional, we can think about adding a DimensionContentInterface::getAdditionalNormalizedData method that allows to add normalized data to the result of the ContentNormalizer.

With this solution, the ContentNormalizer would just iterate over all NormalizerInterface services sorted by their priority and merge the data returned by the NormalizerInterface::normalize method.

I see the following advantages of such a solution:

  • Simplifies NormalizerInterface and makes it consistent to the other interfaces
  • No unexpected magic during normalization when adding a new property
  • No dependency to the symfony/serializer package

@alexander-schranz
Copy link
Member Author

👍 for a new interface. But removing not sure about removing the serialization and the serializer make it simple to add a additional field to the normalized object.

@alexander-schranz alexander-schranz added this to the Stable Release milestone Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Only affecting the end developer
Projects
None yet
Development

No branches or pull requests

2 participants