Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Multiple Write Handlers #19

Closed
MGatner opened this issue Nov 19, 2021 · 4 comments · Fixed by #22
Closed

Feature: Multiple Write Handlers #19

MGatner opened this issue Nov 19, 2021 · 4 comments · Fixed by #22

Comments

@MGatner
Copy link
Member

MGatner commented Nov 19, 2021

Every time I think about adding new handlers I run into the same logical issue: how to deal with multiple handlers that need to write a state. I would like to hear a bit more of the logic behind the single-handler approach, in case I am missing something; then maybe consider expanding it to support any registered handler.

@lonnieezell
Copy link
Member

I'm not 100% sure what you mean. It was originally intended for a very specific purpose - work with the database to save settings. If you're wanting to expand it to work with Redis/Firebase/whatever then it should be fairly straightforward. Or were you talking multiple handlers being used simultaneously? Then we have to be careful about locking issues, etc.

Can you expound on the use case, and then maybe I'll understand better what you're trying to do.

@MGatner
Copy link
Member Author

MGatner commented Nov 19, 2021

Maybe I misunderstood the original intent. The config file uses an array for $handlers:

public $handlers = ['database'];

And then when fetching a setting it checks every available handler:

// Check each of our handlers
foreach ($this->handlers as $name => $handler) {
if ($handler->has($class, $property, $context)) {
return $handler->get($class, $property, $context);
}
}

... which made me think the intent was always to be able to support multiple handlers, except that writing is locked to just the first handler marked writable:

return $this->getWriteHandler()->set($class, $property, $value, $context);


An example of what I'm currently working on is interposing a Session handler in front of the database. This would limit database settings calls when the key was already present in the session and allow for "semi-persistent" settings (last as long as the session). I imagined creating a new handler and adding into the Config, but then it would supplant database writes.

@lonnieezell
Copy link
Member

Oh, I get what you're saying now. Then that was an oversight on my part. I had honestly forgotten I set it up to use multiple handlers lol. I'm all for changing that if we can do it without a BC break.

@MGatner
Copy link
Member Author

MGatner commented Nov 20, 2021

I don't think it will be a breaking change since the write handling is all private, but a conceptual & behavior change for sure. However seeing how version 2 hasn't even been out a week I'm not too worried about surprising people's extensions 😊

@MGatner MGatner linked a pull request Nov 21, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants