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

Pondering the architecture #45

Open
phil-lavin opened this issue Jan 21, 2013 · 0 comments
Open

Pondering the architecture #45

phil-lavin opened this issue Jan 21, 2013 · 0 comments

Comments

@phil-lavin
Copy link

Sorry... this has turned into a bit of an essay. It's probably a good one for toilet reading.

I haven't previously taken the time to look at Auth because it's always seemed quite a complex and mystical beast. Today I had an issue where a module's config/auth.php was not used by Auth and when that module referenced \Auth it used the auth driver configured in app.

It turns out that \Auth::$_instance is only set in 2 places:

  • Auth::_init()
  • Auth::instance() (if it's not previously set, at which point it's forged)

What the problem turned out to be is that a package's bootstrap referenced \Auth, triggering the _init() method and thus almost irreversibly setting \Auth::$_instance to be an instance of the Auth driver configured in app.

Actually, in this case, the package bootstrap didn't need to reference \Auth. If it did, the most logical option would be to always reference \Auth::instance('name') from the module but this removes the usefulness that is the __callStatic() interface.

This got me thinking about the architecture. Although Auth supports multitons in the form of Auth::$_instances, the singleton Auth::$_instance is quite critical to the operation because driver methods are accessed through Auth::__callStatic(). However, this variable is very inflexible in terms of how it can be set. It's set based on the configs of the context Fuel is in at the time \Auth is first referenced.

I'm thinking one or more of the following would be of use:

  • Set static::$_instance at the end of forge() to the forged instance
  • Add a setter for static::$_instance - possibly just take in an instance id and copy the ref from static::$_instances
  • \Config::load() each time we forge() so that configs that have joined the paths array since the _init() may be included. This means that forge() with no params will cause the driver configured for the current context to load
  • Somehow compare contexts between \Auth calls and repopulate the static::$_instances array and thus the static::$_instance singleton if the context has changed. Possibly a bit drastic and is negated if the above suggestion is implemented and it becomes habit to call \Auth::forge() before driver methods are called via \Auth::__callStatic().

I guess the underlying issue is that if you're writing re-usable code and rely on _init() for setting the correct value into static::$_instance, you're ballsed if something else in the app gets there first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant