This is basically a big copy/paste from the originalget() function, right? I don't like that too much (it gets out of sync very quick and requires more work when some code is changed inget()/resolveService()).

Also, please note that in Symfony 4, private services will be implemented by using a randomly generated service ID. So implementing a complete new API for private services isn't needed.

This is about separating (concerns), not copy-paste. Basically this makesget final, have a look at the diff.

About 4.x changes.. that is eventually still future talk. I compare against the current codebase.

Even then, if you have a "random id" you need to be able to get it internally, without deprecations. That would only cleanupget.. unless you make a real good guess ;-) which should still, eh trigger.

Im not even sure we need random id's, if it's bulletproof already.

Im not even sure we need random id's, if it's bulletproof already.

Using random id's makes it possible to use same logic, it be like using a normal id except you don't know the id.

Is it possible to benchmark these changes with Blackfire.io? I'm worried performance could be degraded.

Even then, if you have a "random id" you need to be able to get it internally, without deprecations. That would only cleanup get.. unless you make a real good guess ;-) which should still, eh trigger.

A compiler would generate random IDs for private services and fix the service definitions that depend on them. No need to change anything in theContainer.

@wouterj Fair enough 👍

I just dont understand how that reflects dumping..;

• generate random id
• update references
• a private service needs to be injected by some id
  • are we gonna bypass the container to get the object?

@sstok i think it's boosted (we bypassget many times now).. but im not sure. Not familiar yet with blackfire.io.. could you collab?

I just dont understand how that reflects dumping..;

1. Compiler finds private service
2. Generates random ID
3. Updates all references to the old ID with the generated one
4. Container is dumped, using the random ID for the service, just like normal.

As said by@sstok, the only difference between a private and a public service will be that you don't know the ID of the private one (as it's automatically generated).

You could start by randomizing them already, and use those. But keep an alias array with private services, pointing to the actual id 👍

Based on the current design;

• php dumper generates calls to (private) services usingget(). How will that not trigger/throw,
  unless we alterget to allow for privates? Again; are we gonna bypass the container getter?
• having a list of private id's (randomized or not) is useful
• randomizing id's is a next step

php dumper generates calls to (private) services using get(). How will that not trigger/throw? Again are gonna bypass the container methods?

If requested service is the randomized ID, it's an internal call and thus should not trigger deprecations. If it uses the original service ID, it's a call from outside world and the deprecation should be triggered (and randomized service returned).

E.g:

publicfunctionget($id, ...){if (isset($this->privates[$id])) {        @trigger_error(...,E_USER_DEPRECATED);$id =$this->privates[$id];    }// ... behave like normal, $id is either a public service or the randomized ID}

Ok, that clarifies. Leaving a minor leak whenever the user has a random id some way, some how. We could live with that, i guess 👍 i personally favor the separation by design though.

Still;get is a api method that does normalization and such. Using it internal seems overhead.

Still; get is a user method that does normalization and such. Using it internal seems overhead.

The internalget() method skips almost all of the method. The first condition matches for new services and the method is simply called. (Seehttps://github.com/symfony/dependency-injection/blob/master/Container.php#L258-L259 )

Leaving a minor leak whenever the user has a random id some way, some how.

We don't check for random ID (what's random?), we check if the ID exists in the array that contains generated IDs by the compiler.

Thanks@wouterj this was really clarifying, and probably makes a separate method obsolete.

What about if we do it right now? I really dont think we should bridge it with workarounds (no offense).

edit: about the leak; whenever you have$id = $this->privates[$id]; you can access a private service (what we're gonna do internally right? :-)). Anyway, doesnt matter they're not exposed, and nobody makes that good guesses.

The@trigger_error(..., E_USER_DEPRECATED); $id = $this->privates[$id]; is for BC only, in the future this resolving is simple removed, and as the original service-id is replaced with something random it will fail (saying the service doesn't exist).

@sstok@wouterj ping ping :-)

This works out pretty nice so far...

@ro0NL: Even if your work looks great,@wouterj 's approach has the benefits of keeping things simple and pragmatic to fix the related issue.
I don't know if it's a good idea both trying to fix the issue and introducing the private services id generation into the very same PR as a "bug fix". It won't be a bug fix anymore, but somehow mixing bug fix, new feature and BC for this new feature. I'd say it's too much :/

But that's only my opinion, a core member is more legitimate to hint you on this. Just saying you should probably don't work too hard on this right now ;)

Im just playing around with randomizing now.. eventually i want to revert what i did toget. (The changes are not that dramatic).

However, the tests are updated to the right behavior. That was was an effort, yes.

I understand@wouterj 's approach and motivation, im only proposing a structural solution to consider as we can currently still target master.

@fabpot as this was youridea more or less.. WDYT?

Tests are failing nicely so far;

@@ @@-        $instance->dep = $this->get('shared_private');+        $instance->dep = $this->get('2a21409629880ccd9259eda073b930c1');         return $instance;     }     /**-     * Gets the 'shared_private' service.+     * Gets the '2a21409629880ccd9259eda073b930c1' service.@@ @@      */-    protected function getSharedPrivateService()+    protected function get2a21409629880ccd9259eda073b930c1Service()     {-        return $this->services['shared_private'] = new \stdClass();+        return $this->services['2a21409629880ccd9259eda073b930c1'] = new \stdClass();     }

We have the map now inContainer.. so we can really be flexible (ie. only use the random id for code, not docs).

Status: Needs work

@sstok would it make sense to mark a random definition (old private) as public after compiling? Currently we deprecatedhas etc. for all privates (random or not), but inget we must allow for it (if requested by the random id).

This doesnt make sense to me:has('random') // false vs.get('random') // object. Marking them public simplifies things :)

We might require this kind of randomization on 4.0 when getting private service will be removed.
For now, I don't think it provides more external side effect than#19708.
If I'm not wrong, this could be closed in favor of#19708
WDYT?

Yeah, I don't think we need to do this in 3.x

I dont mind :) i just dont like how we work with privates right now. And ido think this is the cleanest solution eventually, as the container does not have to care about privates. Currently leading to all sorts of quirkiness, as you might have noticed.

But yes, the pass can be added on top of#19708 in 4.x. From the component pov i would prefer this though (understanding the scope is wider, however wedo advertise with standalone components ;-)).

So, followingthe discussion above, my stand on this is:

Then maybe the simplest and most DX friendly is to make private services leak a bit by reserving their id, throwing an exception whenever one uses or fetches an id that collides with a private service.

This meansnot randomizing private services, even in 4.0.
Given that 4.0 is far away, I think we should close this PR, at least for now.
It's way too early to work on it and the issue will naturally arise when working on 3.4/4.0 when we'll work on cleaning deprecations.

👍 eventually it comes down to track private id's yes/no. Like currently..

Personally i'd favor randomizing id's in thePhpDumper.. (privates are just normal services).

But it doesnt matter much i guess.