Skip to content

Conversation

@jakejohns
Copy link
Member

I think this should allow us to create a schema object without needing to know the
driver type by getting the information from the pdo via
PDO::getAttribute(PDO::ATTR_DRIVER_NAME), or by passing a name.

I think this makes it easier to get a schema object when the drive type is
defined in configuration. For example, defining a key in an .env file as
DB_DRIVER=sqlite.

function define(Container $di) {
    $di->set(
        'aura/schema:schema_factory',
        $di->lazyNew('Aura\SqlSchema\SchemaFactory')
    );

    $di->set(
        'schema',
        $di->lazyGetCall(
            'aura/schema:schema_factory',
            'newSchema',
            $di->lazyGet('pdo'),
            $_ENV['DB_DRIVER']
        )
    );

    // or, since I think we can just get this info from PDO:

    $di->set(
        'schema',
        $di->lazyGetCall(
            'aura/schema:schema_factory',
            'newSchema',
            $di->lazyGet('pdo')
        )
    );
}

@harikt
Copy link
Member

harikt commented Aug 16, 2015

Actually you can do something like this in Aura.Di.

$di->params['Aura\SqlSchema\MysqlSchema']['column_factory'] = $di->lazyNew('Aura\SqlSchema\ColumnFactory');
$di->params['Aura\SqlSchema\MysqlSchema']['pdo'] = $di->lazyGet('pdo');
$di->set('schema', $di->lazyNew('Aura\SqlSchema\MysqlSchema'));

I am not in favor for Factory for the Di usage.

@harikt
Copy link
Member

harikt commented Aug 16, 2015

Hm.. Regarding the Env may be you can do the same inside for params ?

@jakejohns
Copy link
Member Author

re: factories and di

I am not in favor for Factory for the Di usage.

Really? I'd love some further explanation on that.

I initially found myself duplicating a lot of the config/Common code from the
aura components in my project and component configs, since the provided
configs use the 2.x naming (Container, not ContainerConfig) (I guess I might be
able to use class_alias or something instead).

However, I thought using the factories and lazyGetCall, might be a better
solution, since it seems like my consuming code needs to know a little too much
about the internals of the aura packages to be able to configure them. For
example, if I'm using aura/session
I dont think my project needs to know about Randval and PhpFunc. I thought
it made more sense to use the API defined by the factory and session, and let
aura/session take care of itself. If some "internal" dependency changes in a
package, that doesn't break the public API, I shouldn't need to update my
configs.

Also, I was under the impression, that the presence of the _Config/Common stuff
was going away in favor of a more "factory oriented approach". I figured this
approach was more in line with the future.

re: knowing which object to create

I could just do the translation of a string like "sqlite" to
"Aura\SqlSchema\SqliteSchema" in the config. I think it seems a little
clunkier every way I've written it. I end up with what seems an unnecessary
amount of logic in my config, and again, seem to need to know quite a bit about
the internals of the aura package.

$drivers = [
    'mysql' => 'Aura\SqlSchema\MysqlSchema',
    'pgsql' => 'Aura\SqlSchema\PgsqlSchema',
    'sqlite' => 'Aura\SqlSchema\SqliteSchema',
    'sqlsrv' => 'Aura\SqlSchema\SqlsrvSchema'
];

$di->set('schema', $di->lazyNew($drivers[$_ENV['DB_DRIVER']]));

// or maybe something like this? but this seems really convoluted

$di->set('schema', $di->lazyValue('db_driver'));

$di->values['db_driver'] = $di->lazy(
    function () use ($di, $drivers) {
        $driver = $di->get('pdo')->getAttribute(PDO::ATTR_DRIVER_NAME);
        return $drivers[$driver];
    }
);

Do people think these types of solutions are preferable?

Thoughts?

@harikt
Copy link
Member

harikt commented Aug 17, 2015

I am showing the examples in 2.x and sorry that it is not tested though. I haven't looked 3.x. My assumption is it will not be different.

In .env file.

driver="Aura\SqlSchema\MysqlSchema"

Assuming you have already loaded the .env variables via something like

<?php
require __DIR__ . '/vendor/autoload.php';
$dotenv = new Dotenv\Dotenv(__DIR__);
$dotenv->load();

Now in the config

$di->params[getenv('driver')] = $di->lazyNew('Aura\SqlSchema\ColumnFactory');
$di->params[getenv('driver')]['pdo'] = $di->lazyGet('pdo');
$di->set('schema', $di->lazyNew('Aura\SqlSchema\MysqlSchema'));

Doesn't this was working for you ?

@jakejohns
Copy link
Member Author

Thanks for the response @harikt

I assume you mean this as the last line?

$di->set('schema', $di->lazyNew(getenv('driver')));

Certainly that would work. However, I am hoping to reduce redundancy in the
configuration, and be able to specify the driver in a more universal way.

For example, this seems ideal to me:

# env file
DSN="sqlite:/foo/bar.db"
$di->set( // parse the DSN
    'dsn',
    $di->lazyNew(
        'AD7six\Dsn\DbDsn',
        ['url' => getenv('DSN')]
    )
); 

$di->set( // pdo
    'pdo',
    $di->lazyNew(
        'Aura\Sql\ExtendedPdo',
        ['dsn' => $di->lazyGetCall('dsn', 'toUrl')]
    )
); 

$di->set( // get the correct schema class based on the PDO
    'schema',
    $di->lazyGetCall(
        'aura/schema:schema_factory',
        'newSchema',
        $di->lazyGet('pdo')
    )
); 

// get the right query factory based on parsing the dsn
$di->params['Aura\SqlQuery\QueryFactory']
    ['db'] = $di->lazyGetCall('dsn', 'getEngine');

See, here the only thing I need to specify in my .env is the dsn.

I can parse the dsn to determine the engine
(using this for now AD7six\Dsn)

See, I can create an instance of Aura\SqlQuery\QueryFactory with a "common
name" for the engine (eg. mysql or sqlite). see here
But right now I have no way to do the same for "Schema".

My initial intention was just to make it so I could create a schema by using a
common name, and then I realized that I think we can just get that information
from the PDO itself.

Given the solution I think you are purposing, my .env would need to specify
essentially redundant information in triplicate, when I believe we can get the
available information from a single value.

# redundant
DSN="sqlite:/foo/bar.db"                    # specifies sqlite
QUERY_FACTORY_DB="sqlite"                   # specifies sqlite again
SCHEMA_DRIVER="Aura\SqlSchema\SqliteSchema" # specifies sqlite again again

Even, if we ignore the whole "parsing the DSN to get the engine" thing, and we
forget about the idea of "just getting the driver name from the PDO itself",
I still like the idea of being able to only specify the engine once.
(in addition to the dsn)

DSN="sqlite:/foo/bar.db"
DB_ENGINE="sqlite"

Thoughts?

Also, still curious about your objection to factory in the di in general.

@harikt
Copy link
Member

harikt commented Aug 17, 2015

Thanks for your detailed answer . In my use I never switched between 2 different db, so I never needed something like this. If you really need I guess @pmjones will look and give you further thoughts / merge this. I am ok with the factory use case you have given.

Also, still curious about your objection to factory in the di in general.

One reason is in some cases we may need to make use of the same objects . When we use factories it may be creating a new instance of another object. It may not be a satifactory explanation though.

Thank you.

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 this pull request may close these issues.

2 participants