Hi, David.
Thank you again for your response.
I understand your point of view. You don't think there is anything the
ORM should do to prevent fetching a non-shared instance, because that
would be the responsibility of the application. I kind of agree, to a
limited scope.
But if you allow me to press on this issue just a little longer... it
still doesn't change the fact that I can still have two different
containers: ContainerShared, that only configures/fetches shared
services, and ContainerFactory, that only fetches new instances. They
would both be PSR-11 compliant, and still, incompatible for most
cases. And then, it wouldn't be a configuration issue (a bug) on the
application end, it would be a component incompatibility - which kind
of defeats the point of having a interoperability standard. It'd still
be a little odd saying something like "this ORM can be used with
PSR-11 compatible containers, except for containers that don't have
the option to configure shared services (like ContainerFactory)".
Choosing a standardized component is not a configuration bug, right?
Should we let two PSR-11 implementations be incompatible for many (if
not most) cases, in the name of backwards compatibility (with
containers that still don't extend PSR-11)?
You tell me this concern is out of scope. I don't disagree. I'm just
saying that, IMO, the current scope is not enough to assure two
different implementations are actually compatible - and thus,
interoperable, which is kind of the point here. And there is something
we could do to fix this, without broadening the scope -- limit what
kind of services (shared or exclusive) "get" can return. This leads us
to a different issue, which is compatibility with current implementations.
To be completely honest here, with best practices in mind, I can't
even think of why a framework component (the ORM, in this case) should
even be container-aware. It should have its dependencies declared
directly in the constructor (or in separate setters) and just trust
someone else (the application) will inject them. A container should be
just one of the framework's components, not part of its core/kernel.
Em sexta-feira, 4 de novembro de 2016 13:45:29 UTC-2, David Négrier
escreveu:
Hi Pedro,
I understand your example (about the event dispatcher that MUST be
shared), but still, I disagree this is an issue with PSR-11.
You say:
/Now, my consumer, the ORM, is completely container agnostic. It
only works, however, with SOME specific implementations (that
ALWAYS return shared instances), and fails on SOME others (that
ALLOWS for factories). I don't know (and can't control) how the
entries were set, because I deferred this wiring to the
application realm, by design. I don't know (and can't control)
which container this is. If I can't guarantee it will work with
ANY PSR-11 implementation, I cannot depend on the contract
(PSR-11) - I'll have to depend of an implementation, which is bad./
When you say: /It only works, however, with SOME specific
implementations (that ALWAYS return shared instances), and fails
on SOME others (that ALLOWS for factories), /I disagree. If we use
"solution 3" I'm proposing above, you should really write: /It
only works, however, with SOME specific configurations (that
ALWAYS return shared instances), and fails on SOME others (for
containers that CAN provide non-shared services and that have been
configured accordingly)/
Now, you say: /I don't know (and can't control) how the entries
were set, because I deferred this wiring to the application realm,
by design./
When you say "I", I suppose you mean "the ORM author", am I right?
Of course, you are right to say the ORM author can't control if
the event dispatcher is shared or not. And it is not its job. And
I'm sure you agree with me if I say that the ORM is not using
ContainerInterface directly (it is the container how creates the
ORM's EntityManager and injects the event dispatcher in it.
So in the end, the wiring of the app is deferred to the
application realm (i.e. the end user). If the end user badly
screws up its configuration and decides to configure a non-shared
event dispatcher, that's its problem (it's a bug), not a problem
with PSR-11. Of course, with a more restrictive PSR-11 (that would
force shared services only), this kind of bug would not be
possible. But we loose compatibility with all the containers out
there and it's just not worth it in my opinion.
Finally, we will inevitably end up working on a PSR that defines
how we put things in a container. In this PSR, we will need to be
very strict regarding the fact that we set a shared or a
non-shared service.
If you haven't done so yet, you should definitely take a look at
container-interop/service-provider
<https://github.com/container-interop/service-provider>. This
project proposes a way to put things into a container (and I hope
will be the basis of a future PSR). I just submitted a PR (here:
https://github.com/container-interop/service-provider/pull/33/files
<https://github.com/container-interop/service-provider/pull/33/files>)
that makes sure that containers consuming a service provider MUST
provide shared services. So even if PSR-11 allows for non-shared
service, container-interop/service-provider explicitly forbids it
for services provided in service providers.
TL;DR; I understand your sample about the event dispatcher being a
non-shared service. This would definitely be an issue. I just
disagree that it is a concern with PSR-11. It is a concern with
either the user configuring the container, or the next PSR to come
that defines how we put things into a container.
Does it make more sense?
++
David.
Le jeudi 3 novembre 2016 10:58:51 UTC+1, Pedro Cordeiro a écrit :
> The typical use case for ContainerInterface is to allow a
consumer (like a router) to be "container agnostic".
I completely agree. I can't think of a use case where a router
would depend on having either shared instances or exclusive
instances of a controller, so I'll change this example a
little. Hopefully this answers @Matthieu's question too.
Let's say I have a framework component (and ORM, maybe) that
triggers events (to log queries, maybe). This part of my
framework fetches an event dispatcher (let's say, symfony's)
from the registry, and it HAS to be a shared entry (otherwise,
I'll just dispatch an event on a new instance that has no
listeners).
Now, my consumer, the ORM, is completely container agnostic.
It only works, however, with SOME specific implementations
(that ALWAYS return shared instances), and fails on SOME
others (that ALLOWS for factories). I don't know (and can't
control) how the entries were set, because I deferred this
wiring to the application realm, by design. I don't know (and
can't control) which container this is. If I can't guarantee
it will work with ANY PSR-11 implementation, I cannot depend
on the contract (PSR-11) - I'll have to depend of an
implementation, which is bad.
The only instances where I could depend on a 100% generic
PSR-11 container is when I don't care at all if the service
being retrieved is exclusive or shared. And there only needs
to be ONE place where I need a specific kind of instance, and
my entire framework will depend on a specific implementation.
The event dispatcher service is one example where I need a
shared instance -- the ORM could be another. I don't want new
connections being opened, I don't want two separate units of
work with two different in-memory caches. If I fetch my ORM
twice from the container, I expect to be given the same
instance, not two different separate instances. If I fetch it
twice from separate consumers, I still expect to be given the
same instance.
Now, I'll reiterate that I understand strengthening the SHOULD
to a MUST would greatly hinder this PSR's adoption. I just
want to know if you guys understand my point and if you guys
still disagree that this is an issue, and why. I don't know if
I'm failing to see it, but as far as I can think about it, not
knowing what kind of instance the container will yield would
also greatly restrict the possible uses and hinder its
adoption -- and I'm not even talking about things out of scope
here, because I'm failing to see its usefulness even for
consumer-only frameworks, like shown on the previous examples.
Em quarta-feira, 2 de novembro de 2016 15:45:49 UTC-2, David
Négrier escreveu:
This is a continuation from the discussion about whether
PSR-11 containers should always return the same instance.
See
https://groups.google.com/forum/#!topic/php-fig/L8rDUwRFsOU
<https://groups.google.com/forum/#%21topic/php-fig/L8rDUwRFsOU> for
the beginning of the discussion.
<TL;DR;> I believe that we should keep the SHOULD word in
place because it does not hurt in practice and because we
can require a container to always return the same instance
in the yet to come PSR that will define how we put things
into a container (seecontainer-interop/service-provider
<https://github.com/container-interop/service-provider/>for
a proposal). On the other hand, requiring the container to
return always the same value would make PSR-11 de-facto
incompatible with most containers out there (so should be
avoided). However, I think we definitely need to make
improvements on the wording that is clumsy (see end of the
message). </TL;DR;>
Currently, the spec says:
/Two successive calls to|get|with the same identifier
SHOULD return the same value. However, depending on
the|implementor|design and/or|user|configuration,
different values might be returned, so|user|SHOULD NOT
rely on getting the same value on 2 successive calls.
/
Hari, Larry, Pedro are asking if we could strengthen the
SHOULD to a MUST (as in: /Two successive calls to|get|with
the same identifier MUST return the same value/).
For instance, Larry says:
> I would prefer to have get() defined to always return
the same object (lazy loaded or otherwise, not relevant
here), as that is the typical case, and another method
added (either on the same interface or a separate one, I
am flexible) to handle the factory use case. That way I
know the behavior of each object I get back, specifically
in relation to whether I have a private copy or not.
First of all, let me say if I was starting from a blank
slate, I would definitely agree with you and put a MUST
here. But we are not starting from a blank slate and many
containers out there offer the possibility to create new
services each time you call "get". For instance Pimple
(with the factory method), or Symfony (with the "shared"
option:http://symfony.com/doc/current/service_container/shared.html
<http://symfony.com/doc/current/service_container/shared.html>)
I definitely want Pimple and Symfony in PSR-11 (or at
least I want to be able to create an adapter for those)
and putting a "MUST" in the requirements makes this
impossible. So if we decide to force containers to always
return the same value, we'd better have a very good reason
to do so.
Now, what is the real impact of this?
You all seem to imply that if we don't put a "MUST" here,
then we cannot "trust" the container to give us a usable
instance. The container might be "vicious" and decide to
fool its user by giving him a new instance while the user
was expecting the same instance. But this is not how it
works. The typical use case for ContainerInterface is to
allow a consumer (like a router) to be "container
agnostic". As a user, I can decide to plug any container
to a given router. The router will then be able to fetch
the correct controller (or the correct action if you do
ADR) based on the controller name. In this scenario, the
user is the one in charge of configuring the container
(and putting the controller in the container). If the user
decides that he wants to use a container that can act as a
factory and that he wants to configure some services that
are created on each request, it is his responsibility.
Keep in mind that since we do not standardize how to put
things in the container, it has to be the user
responsibility to configure his container.
So saying that a container MUST return always the same
instance is premature. When we will start to standardize
how to put things in a container, then we can define that
configured instances MUST be the same. But we can do this
in the next PSR (the one that defines how to put things
into a container) and not in this one.
Now, even if I'm really convinced that we should keep the
"SHOULD" in place, I understand from the numerous comments
that the way the spec is written is clumsy (or rather
worrisome).
Why did I propose this sentence in the first place? I was
thinking that a "CompositeContainer" could try to cache in
an array the list of services already fetched from its
children. This is something you can do if you know the
children containers will always return the same instance
but that you cannot do if you don't know if the container
will return the same or a new instance. So really, the
"SHOULD" prevents us to write composite containers that
"cache" the result from children. This is actually not a
very big deal, as composite containers are very seldomly
used. What I was trying to say here was: "Hey, don't cache
the services returned by a container, it might return a
new instance on the next call". I proposed this wording 2
years ago in container-interop. In hindsight, this is
certainly too specific to be part of a PSR.
I see 3 possible solutions to replace this:
*Solution 1*:
Let's completely drop the 2 sentences => PSR-11 does not
address whether a container should return the same
instance or not (this is something we can deal with when
we define how to put things into a container)
*Solution 2*:
Let's only keep the first sentence: /Two successive calls
to|get|with the same identifier SHOULD return the same value./
*Solution 3*:
Let's rewrite the second sentence: /Two successive calls
to|get|with the same identifier SHOULD return the same
value. A container MAY offer the possibility to return a
new value for some identifiers if the user configured it
accordingly./
(The wording of solution 3 should be improved but you get
the idea)
@larry, @hari, @pedro: do you favor one of those solutions
or do you still think we should use a "MUST" in the spec?
@matthieu, @mathew: any preference regarding the solutions
proposed above?
We should also try to ping more container authors to get
their advice on this issue. I'd be interested in knowing
what Fabien Potencier or Taylor Otwell think about this.
++
David
--
