pomm-project / Foundation

Foundation package for the Pomm database framework.
MIT License
70 stars 36 forks source link

Dissociate credentials from connection parameters #77

Open chanmix51 opened 8 years ago

chanmix51 commented 8 years ago

It is often annoying to declare several sessions that share the same connection parameters with different users. They must declare the same session builder every time. What would be nice would be a list of profiles (login, passwords etc.) and a list of session configurations (host, db name, connection settings etc).

$pomm = new Pomm(
    [
        'session_1' =>
            [
                'dsn' => 'pgsql://host:5432/db_name',
                'pomm:default' => true,
                'class:session_builder' => '\Some\Class',
                'default_profile' => 'my_profile',
            ],
        'session_2' => …
    ],
    [
        'my_profile' => ['username' => 'unpriv_user', 'password' => 'p4ßw0rD'],
        'administrator' => ['username' => 'postgres', 'password' => 'p4ßw0rD'],
    ]
    );

$session = $pomm->getDefaultSession(); // Default session (1) with default profile
$session = $pomm['session_1']; // Session 1 with default profile
$session = $pomm->getSession('session_1'); // same as above
$session = $pomm->getSession('session_1', 'administrator'); // session 1 with administrator privileges.

Of course the old form of DSN would still be available and inline the default profile to use.

$pomm = new Pomm(
    [
        'session_1' =>
            [
                'dsn' => 'pgsql://user:p4sS@host:5432/db_name',
            ]
    ],
    [
        'administrator' => ['username' => 'postgres', 'password' => 'p4ßw0rD'],
    ]
);

$session = $pomm->getSession('session_1', 'administrator');

Any thoughts ?

stof commented 8 years ago

The issue with this API is that getSession() returns the same instance each time you call it for a given name. Your proposal can lead to several behaviors:

chanmix51 commented 8 years ago

Thank you for helping :+1:

Imho, the good answer is the number 2. To ensure backward compatibility we need to keep the way the sessions used to be returned by the Pomm instance. In case of a profile enforcement, it must be explicit in the session call every time hence it must be cached the same way.

$pomm->getSession('my_session'); // create and return the session using default profile.
$pomm->getSession('my_session'); // return the same instance.
$pomm->getSession('my_session', 'administrator'); // creates a new instance with administrator profile.
$pomm->getSession('my_session'); // return the instance cached with default profile.
$pomm->getSession('my_session', 'administrator'); // returns the previously created session with administrator profile.
$pomm['my_session']; // return the cached session with default profile.

The idea behind this logic is to make developers not to bother about creating or caching sessions. They explicitly ask the service locator what they need. Whatever the session exists or not, it is returned if possible. This way, $pomm['my_session'] returns a predictable session using its default profile which is what you would generally expect.

There is a need for different kind of sessions in tests where administrator rights are required to build database objects or in CLI commands where the database user may not have the same rights granted on database objects than the default one used in the web application.

webaaz commented 8 years ago

:+1:

sanpii commented 8 years ago

A simple ->grantTo($user, $password) is not enough?

stof commented 8 years ago

@sanpii mutating the existing session is a bad idea. It can impact other code already using this Session object (and even worse in case there is a running transaction, as disconnecting and reconnecting with a different user cannot keep the transaction)

sanpii commented 8 years ago

@stof I thought cloning the session.

stof commented 8 years ago

@sanpii but then, you end up with a new Session now know by your Pomm registry. Thus, cloning might be hard (what about all clients registered on the session, should they be forced to be cloneable, or should they be registered without being aware of the clone ?)

stood commented 8 years ago

For me, the configuration for profiles will create confusion with application profiles. I think each session equals a profile, and this is the application who will consume those sessions. You only need to create and use variables in order to setup configuration's redundancy and dsn

chanmix51 commented 8 years ago

The idea is to be able in a test suite to

  1. Create structure & import fixtures
  2. Eventually launch CLI commands using a specific database user
  3. Launch the tests with an unprivileged database user

The database session must use the same configuration (session builder, session parameters etc.) but with a different user. Once the config file is parsed and variables expanded it is not possible for now to ask Pomm to do this.