EZP-29234: As a developer, I want to read about customizing REST API response

[konradoboza]

Question	Answer
JIRA issue	EZP-29234
Bug/Improvement	yes
New feature	no
Target version	6.13
BC breaks	no (I hope so)
Tests pass	yes
Doc needed	yes

This PR contains improvement for customizing REST Accept header based response. The idea is to ged rid of overriden format generators which only responsibility is to set $vendor prefix which could be defined as an argument in services.yml. It will simplify customization of API response which was introduced in: ezsystems/PlatformUIBundle#964 and described in cookbook: ibexa/documentation-developer#288. As a followup, this configuration could be applied to PlatformUiBundle.

TODO:

• Implement feature / fix a bug.
• Fix new code according to Coding Standards ($ composer fix-cs).
• Ask for Code Review.

[konradoboza]

Lack of changes in generateMediaType method causes Travis failing.

[andrerom]

Besides failure , optionally add a default value here too

[konradoboza]

Default value added.

[andrerom]

Last failure: Warning: Declaration of EzSystems\PlatformUIBundle\Rest\Generator\Json::generateMediaType($name, $type) should be compatible with eZ\Publish\Core\REST\Common\Output\Generator::generateMediaType($name, $type, $vendor = 'vnd.ez.api')

I guess this will be fixed in the PR you propose there.

To unblock, one (A) option is to do a quick unblock PR to Platform UI where we just add the parameter, given it is optional PHP should be ok with it both for version prior to this, and with this PR.

Then once tests are green we can merge this and do followup on Platform UI where we bump requirements on kernel to be able to get changes in this PR.

The other (B) option is Hulk smash approach, merge while both PR's are read as we know it's a two way dependency issue.

[konradoboza]

@andrerom in PlatformUI composer.json I set the dependency to this branch for kernel in order to get tests green ezsystems/PlatformUIBundle@d184009#diff-b5d0ee8c97c7abd7e3fa29b9a27d1780L14. Apparently, on 1.13 there is one test which always fails and it is not relevant to my changes. Probably it could be merged, but without this dependency in composer.json. After that, kernel should be unblocked.
//edit there are some problems with dependency which need to be fixed.

[andrerom]

Side: @adamwojs / @alongosz Are we doing a BC break here? given Platform UI breaks on this I would assume anyone else's code on this would do so as well if others have extended Generator for some reason.

[konradoboza]

I can only add from myself, that I also had this on mind, yet decided to go this way, because we introduce the possibility to customize a response in a cookbook as something new. Hopefully, no one used this approach before. :) Otherwise, I guess I should think of something else, yet feedback will be welcomed.

[alongosz]

Based on the code and code alone, there's no BC break, because there's no BC policy on Core namespace. But given we're using it in PlatformUI... well... :) Using it is rather a bug in PlatformUI and a bug in our doc to even recommend that :P

BTW. What is the new usage, with this change? In the linked doc PR I see only old usage by extending.

Once again, everything that we intend for Developers to use should belong to some API namespace. If it's meant for extending - then for SPI, but we haven't really documented or promised anything on that.

Unfortunately if we documented something, then despite what I've said above it should be considered as a BC break. But maybe not that severe, AFAICS doc change was merged 7 days ago. So there is still some time slot to fix that :)

[andrerom]

Unfortunately if we documented something, then despite what I've said above it should be considered as a BC break. But maybe not that severe, AFAICS doc change was merged 7 days ago. So there is still some time slot to fix that :)

true, lets do this, greatly simplifies usage.

[andrerom]

@konradoboza lets just unblock this, please do the following:

• remove changes on generateMediaType
• deprecate it @deprecated 6.13.5 please start to use generateMediaTypeWithVendor() instead
• Introduce generateMediaTypeWithVendor() as replacement, adapt code here and platform ui
  • Optionally make it final to avoid this happening again, however I don't think we need that here, intent is rather clear. wdyt @alongosz ?

This afaik will allow kernel to pass easily, and ui PR will pass once kernel change has been merged.
And afaik also no risk any one else will be affected by this as well.

[konradoboza]

Suggestions applied, thank you @andrerom. There are no changes in PlatformUI PR, so I think once this is merged, we are good and safe to re-run tests in PlatformUI, as Travis is green here. :)

[andrerom]

@adamwojs as you are on reviewers list also, is this ok from your side?