Core: [Proposal] make request attributes more OOP-way

Created on 13 Jul 2018  Â·  10Comments  Â·  Source: api-platform/core

For example, if I want to change custom Data Provider behavior based on $context variable, I have to put a breakpoint within particular method and find out what does this variable contain, especially attributes contains more and more keys when the APIP grows up.

It's quite non-OOP that a developer has to guess array keys instead of use an object with adequate getters and interface, what do you think?

RFC enhancement

Most helpful comment

@dunglas — I do not mean to mess up with events, just the $context array is the wild puzzle which you won't solve if you don't dig up the core's sources.

All 10 comments

Storing non-scalar values in the request attributes is not supported, according to @fabpot:

https://github.com/symfony/symfony/pull/17800#issuecomment-184110509
https://github.com/symfony/symfony/pull/17800#issuecomment-191753027

I don't mean to store directly but to wrap this into more familiar objects. Looking for keys within vendor sources is bad.

@e1rz It has to be in the request context. The only other solution is to create a service with a global state, which is bad design and leads to issues when using PHP PM and the like (we had one in v1).
Closing as won't fix, at least while until using objects is supported in Symfony.

@dunglas I think you've misunderstood. We could create objects for using these attributes, basically like what we've done with the RequestAttributesExtractor, but OO.

So instead of working with key-value pairs, we work with well-defined objects instead. I think that is something that we should aim for.

I just ask for considering that relying on array keys is a pain in the… whereas whole library is Object-Oriented. For example, have to guess if the key is request_uri or request_url (who hasn't ever misspelled this one). Why don't provide $context->getRequestUri() instead?

You mean using RequestAttributesExtractor? If yes, this class is marked with @internal and shouldn't be used in custom listeners. It's why we don't use a OOP API here.

Anyway, we'll introduce a more developper friendly event system (also suitable for GraphQL) soon.

The idea here is that each consumer that needs to work with some attributes should declare a Context class (value object) that makes their contract with such attributes a clearly defined one.

The RequestAttributesExtractor gives a hodgepodge of attributes for many different consumers all mixed together. Yes, of course that cannot be part of an external contract.

@dunglas — I do not mean to mess up with events, just the $context array is the wild puzzle which you won't solve if you don't dig up the core's sources.

Was this page helpful?
0 / 5 - 0 ratings