RFC: Setup authentication through method calling (bonus fluent interface) about authentication HOT 17 CLOSED

@inoas I've updated the documentation with the OOP style config. See the readme.md in the persistence branch: https://github.com/cakephp/authentication/tree/persistence

@markstory @ADmad can we close this ticket?

from authentication.

Adding methods to set every config option seems like it would get unwieldy at least on the surface. We could add checking for typos in configuration keys, and raise exceptions when unexpected keys are provided.

from authentication.

OK, so we can close this and keep it as it is because it already features both flavours?

from authentication.

As mentioned here configuring the service class using fluent interface is already possible.

from authentication.

Setting username, password fields, custom finders etc, should also be possible through a fluent interface - is it?

Could you update the example to see if it flows good?

from authentication.

@inoas please review the code before making suggestions.

Setting username, password fields, custom finders etc, should also be possible through a fluent interface - is it?

AuthenticationService::loadAuthenticator() is already doing what you propose, 2nd arg is the config. It just returns the authenticator not the current objects instance. So in theory a change would be trivial.

We could add a loadIdentifier method but this would just be a proxy to the collection object. So you can already do $service->identifiers()->load('Identifier');

What is the advantage of chaining here that would justify the change?

from authentication.

Chaining isn't the key - just would/is nice to have.

The array way of setting up component should be discouraged unless the new code throws very explicit warnings including warnings about array keys/values that were not expected.

The examples should feature the new way first, the old way last.

If the setting is done through single calls or chains matters little to me, though fluent is just nicer.

from authentication.

So what happens if someone enters this:

        $service->identifiers()->load('Authentication.Orm', [
            'fields' => [
                'username' => 'email',
                'passwort' => 'pass'
                     // ^
            ]
        ]);

Could these be method calls like setUsernameField() and setPasswordField()?

from authentication.

Nothing happens then. Unit test your login. :)

from authentication.

@markstory this becomes a ton of checks for some config arrays. Especially the nested ones like the fields.

from authentication.

Wouldn't it also be a ton of methods?

from authentication.

It's not praction to have tons of method nor array key checks. At some point devs just have to be careful.

In AuthComponent you would end up with a dozen keys in config array making typos difficult to spot but here having separate methods for setting up identifiers and authentication means each function call would have only 2-3 keys making typos easy to spot.

from authentication.

We should remove the possibility to pass the whole config in a giant array in the constructor then? This would force people to use the two methods.

from authentication.

I would keep it. Let people choose their preferred way.

from authentication.

I agree. Some might have it configured and pass the Configure::read() stuff or sth.

from authentication.

@markstory wrote:

We could add checking for typos in configuration keys, and raise exceptions when unexpected keys are provided.

How about we consume each supported key (read out then unset it). If anything is left afterwards, the intention was unclear and an exception would be thrown listing (dotnotation) some path/paths could not be consumed?

That would also keep the ability to set stuff via Configure::write() as @dereuromark pointed out.

from authentication.

I still think it is kind of error prone. This is the moment things change and I'd like to see less error-prone-ness.

While I agree, just test your code. :) You should get failing tests for your login if you have such typos.

How about we read out every key then unset it. If anything is left afterwards, the intention was unclear and an exception would be thrown listing (dotnotation) the path that could not be consumed?

@inoas This has been discussed before and the idea was to have config objects. Basically something like an entity that can validate it's config and the config be read from. However, if you're interested in chasing this idea any further please open a ticket in the cakephp repository.

You might show how other frameworks do it (I don't have the time for the research) and might come up with a sample implementation? A BC compatible way would be a config object that implements ArrayAccess. So we can do checks on it while staying BC when passing it. This could be even done as a plugin. If you implement getters and setters like the entity class you can implement custom checks on each of the array values. So while calling $config->password() it would write to $config['fields']['password'].

$config = new Config($existingArray);
$config->validate(); // Throw exception if something is wrong
$config['fields']['password'] = 'foo'; // Should set password
$config->password('foo'); // Should set password, throws exception if invalid value(s) is passed
echo $config['password']; // Should return foo

Classes receiving that could check if they receive an array or config object and call validate() when it's an object. This could replace the InstanceConfigTrait as well I think.

This here should pretty much do what you want:

class Config extends ArrayObject {

	protected $_defaultConfig = [
		'defaultValue' => 'my default value'
	];

	public function __construct(array $input = []) {
		$input = array_merge($this->_defaultConfig, $input);
		parent::__construct($input, $flags = 0, $iteratorClass = 'ArrayIterator');
	}

	public function __call($func, $argv) {
		if (substr($func, 0, 6) === 'array_') {
			return call_user_func_array($func, array_merge([$this->getArrayCopy()], $argv));
		}

		if (count($argv) === 0) {
			return $this[$func];
		}

		$this[$func] = $argv[0];;
		return $this;
	}
}

Then do your checks inside your methods, you need to implement them.

from authentication.