container icon indicating copy to clipboard operation
container copied to clipboard

Question: How to "inflect once" for shared services?

Open beryllium opened this issue 4 years ago • 10 comments

I'm running a Container with defaultToShared=true, and if I add an inflector, it runs the inflector every time the shared class exits the container. This is causing some unexpected behaviour with an EventDispatcher ListenerSubscriber class.

Is there something I should be doing differently so that the inflector doesn't run every time & end up subscribing dozens of duplicate event listeners? Or should the code be checking some combination of ->isShared() or a hypothetical new ->hasBeenInflected() helper before calling ->inflect?

beryllium avatar Aug 11 '21 20:08 beryllium

Here is a possible approach that might sort of work: https://github.com/thephpleague/container/compare/4.x...beryllium:2021-08-13-container-track-inflected-state

However, it has some drawbacks and may not fully solve all possible scenarios.

(In my case, the ListenerSubscriber code I'm working with that would benefit from this change is using an accumulator, not a setter, and therein lies the issue. Multiple calls to the inflector and suddenly the whole system grinds to a halt because identical events are being dispatched multiple times.)

beryllium avatar Aug 14 '21 02:08 beryllium

I think to be honest that making the assumption that a shared object does not need further inflection is the way to go. I need to take a look at whether I'd be comfortable with that in a minor release though. Give me a little time and I'll come back to you.

philipobenito avatar Aug 16 '21 06:08 philipobenito

Sounds good! I've got a local workaround in place (using the power of method static variables 😂 😭 🤣 ), which is working well for the time being.

beryllium avatar Aug 16 '21 07:08 beryllium

Encountered this issue again in a project, when I forgot to add the static variable workaround :facepalm: Any recent thoughts on a solution? Let me know if I can help with a PR proposal or something.

beryllium avatar Jan 20 '22 18:01 beryllium

@beryllium yeah I have a few things queued up to release, within a week 👍

philipobenito avatar Jan 20 '22 18:01 philipobenito

Awesome! Glad to hear it :)

beryllium avatar Jan 20 '22 18:01 beryllium

Sorry for the delay again, something came up, aiming for Monday release now (7 Feb)

philipobenito avatar Feb 02 '22 09:02 philipobenito

Just hit this too. Is there anything we can do to help @philipobenito ?

kraftner avatar May 09 '22 15:05 kraftner

I would also like to see this shipped. Please let me know if I can help :)

beryllium avatar May 11 '22 17:05 beryllium

Sorry for the delay, been dealing with some health issues.

Planning a merge/release session next week so will look properly.

philipobenito avatar May 19 '22 13:05 philipobenito

I've got a fix for this, going to roll it in to the 5.x release next week though as don't really want to touch 4.x at this point.

5.x won't change the public API so will be a very simple update process.

See #255

philipobenito avatar Mar 14 '24 08:03 philipobenito

Also see this commit https://github.com/thephpleague/container/commit/d4863766993ff24d2d245984c7861e35fc1a9384 for the code change, I wouldn't recommend using 5.x yet as I may inadvertently change the public API without realising before release early next week.

philipobenito avatar Mar 14 '24 09:03 philipobenito

Thanks for taking care of this!

beryllium avatar Mar 14 '24 16:03 beryllium