[PoC] Value formatters

[phansys]

Subject

Introduce value formatters.

See #293 (comment).

I am targeting this branch, because these changes should respect BC.

Closes #293.

Changelog

### Added
- `Sonata\Exporter\Formatter\BoolFormatter`, `Sonata\Exporter\Formatter\DateIntervalFormatter`, `Sonata\Exporter\Formatter\DateTimeFormatter`, `Sonata\Exporter\Formatter\EnumFormatter`, `Sonata\Exporter\Formatter\IterableFormatter`, `Sonata\Exporter\Formatter\StringableFormatter` and
  `Sonata\Exporter\Formatter\SymfonyTranslationFormatter`
  classes to be used within implementations of `Sonata\Exporter\Formatter\Writer\FormatAwareInterface`
- `sonata_exporter.writers.{writer}.formatters` configuration in order to determine which formatters will be used by each writer.
  By default, "bool", "dateinterval", "datetime", "enum", "iterable" and "stringable" formatters are configured.
  If "symfony/translations-contracts" is installed, "symfony_translator" formatter is also enabled.

### Deprecated
- `Sonata\Exporter\Writer\FormattedBoolWriter`, use `Sonata\Exporter\Formatter\BoolFormatter` instead.
- Arguments `dateTimeFormat` and `useBackedEnumValue` in `Sonata\Exporter\Source\AbstractPropertySourceIterator::__construct()` and
  their children classes. To disable the source formatting you MUST pass `true` in argument `disableSourceFormatters` and use
  `Sonata\Exporter\Formatter\Writer\FormatAwareInterface::addFormatter()` in your writers instead.

To do

• Validate the proposal;
• Evaluate if the concept of priority is required (by instance, to translate a value after a previous formatting).
• Update the documentation;
• Complete the PR's description;
• Add more formatters (like SerializableFormatter, TranslatableFormatter, etc);
• Add an upgrade note;
• Update the tests;
• Fix the findings detected in the CI pipeline;
• Determine how to cover the recursion required by the IterableFormatter formatter;

[core23]

Can you make this translatable?

[phansys]

Yes, but I think this should be responsibility of something like a TranslationFormatter. WDYT?

[core23]

I know we have this for quite a while, but do we need the brackets?

[phansys]

I think the brackets are used here to represent an array structure instead of a colloquial list.

[phansys]

@sonata-project/contributors, I'd like to know your opinion about this approach before spending more time.

Thank you in advance.

[VincentLanglet]

@sonata-project/contributors, I'd like to know your opinion about this approach before spending more time.

Thank you in advance.

I like the idea. But I'm not sure about the

protected function format(array $data): array
    {
        foreach ($this->formatters as $formatter) {
            $data = $formatter->format($data);
        }

        return $data;
    }

part.

If we have string formatter, we will have a risk to have some conflict with previous formatter (which already format some non-string data like boolean or date, to string).

Shouldn't we have something like

foreach ($this->formatters as $formatter) {
     if ($formatter->support($data);
            return $formatter->format($data);
     }
}

[phansys]

Thanks for the quick reply.

If we have string formatter, we will have a risk to have some conflict with previous formatter (which already format some non-string data like boolean or date, to string).

I have the same concern, the intention of this item in the To Do list was to cover that:

Evaluate if the concept of priority is required (by instance, to translate a value after a previous formatting).

About the supports() method, I'm currently implementing these checks in an implicit way. See this example.
BTW, please be aware that the $data variable in $formatter->support($data) is an array representing a row and not a single item.

[VincentLanglet]

Why is it false by default in the abstract and true by default here ?

[phansys]

My bad, thanks (I guess it was preventing the new deprecations).
I'll be updating this soon.

[VincentLanglet]

About the supports() method, I'm currently implementing these checks in an implicit way. See this example. BTW, please be aware that the $data variable in $formatter->support($data) is an array representing a row and not a single item.

Ok, so I thought about something like

foreach ($data as $key => $value) {
     foreach ($this->formatters as $formatter) {
         if ($formatter->support($key, $value);
            $data[$key] = $formatter->format($key, $value);
            continue 2;
         }
    }
}

This way a value is formatted only once.

And we could provide a ChainFormatter if someone want to apply multiple formatter on a specific value. (Like Boolean + Translator).
This way, If I export

• A boolean TRUE
• A string 'true'

I'll get "trans(true)" for the boolean (If I used a ChainFormatter) without impacting the "true" string.

Not sure if I'm clear @phansys

[phansys]

Not sure if I'm clear @phansys

I think I get your point, thank you.
I'll be pushing a new commit when I have something about this approach.

[phansys]

For the records, I'm dumping here some thoughts I currently have about this feature, but I'd like to debate later if this PR is merged:

1. Possibility to have mappings in the sources, allowing to determine what "columns" in the exported values must be processed by each formatter. This way, we will save iterations on values that are not covered by a formatter;
2. Support for templates (like Twig), in order to let the user to have more control on specific cases. This should bring a DX similar than the one we have for the admin mappers in SonataAdmin.
3. Improve the integration from SonataAdmin, in order to have an "export" API similar to the others ("list", "show", etc).

[VincentLanglet]

For the records, I'm dumping here some thoughts I currently have about this feature, but I'd like to debate later if this PR is merged:

1. Possibility to have mappings in the sources, allowing to determine what "columns" in the exported values must be processed by each formatter. This way, we will save iterations on values that are not covered by a formatter;

That's why I wrote the code

if ($formatter->support($key, $value);
            $data[$key] = $formatter->format($key, $value);
            continue 2;
         }

By passing the key to suport/format method, it allows to have different behavior based on the column.

[phansys]

By passing the key to suport/format method, it allows to have different behavior based on the column.

Yes, but in this case $key is the column name. We still need a mapping to provide the type for each column name. In that case, if you suggest adding something like that in this PR, I guess we should also update the sources in order to return the mapping.

IMO, building the mapping will not be so easy. In case of objects (like the ones returned by AbstractPropertySourceIterator) we could use the reflection API to guess the types, but in case of sources returning arrays it may be harder.

[Hanmac]

@phansys first i wanted to make an extra Issue about this, but then i thought i might comment it there too:

when doing the DatetimeFormater, could you maybe add a Timezone change too?

Like for example the Data is stored in UTC Time, but the Grid uses Sonata\IntlBundle to show it in User Time.
But right now, the Exporter doesn't respect that.

could there be a Hook where Sonata\IntlBundle\Timezone\TimezoneDetectorInterface can be connected to change the Timezone for the Export output?

Or would it be easier to just overwrite sonata.exporter.formatter.datetime in user code?

Edit: OR also change the Datetime Format depending on the User Locale?

[github-actions]

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Hanmac]

A different idea I had right now is to feed the export data into Symfony Serializer

Then we could use all the Normalizer Features we want

But I need to brainstorm about that idea

---

Hm for my Serializer idea, the getDataSourceIterator sadly already has translated keys,
and AbstractPropertySourceIterator already has values like DateTime turned into Strings
I would have liked, if they had been translated later, after my Serializer would have worked with their keys