change DataSource::listSources to Datasource::cachedListSources #309

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

challet commented Nov 8, 2011

I made this changes to avoid design errors when using a custom Datadsource (in my case forCouchDB) which doesn't have and doesn't need sources neither a schema :

When the Model::setSource method is called there is a test :

if (method_exists($db, 'listSources')) {
    $sources = $db->listSources();
    // check that $tableName is a source
    // reset the $this->_schema
}

In my case the CouchdbDatasource shouldn't have this listSources method, but it actually has it through the Datasource inheritence.

By looking at it, I saw its purpose is to cache results from "child calls", but it can't be used as replacement.
This is why I think inheritence is not the best choice here, and I renamed it.
Calls from Core Datasource have also been changed accordingly.
Unfortunately I'm aware it might affect other custom Datasources. So maybe it should go to a major release (?)

Note : The main reason to not implement CouchdbDatasource::listSources is because the previous Model::setSource reset the _Model::schema to null when listSources exists.
Which I don't want since CouchDb doesn't have any internal schema.
In order to stick to the Model mechanism (especially at save), the $_schema is defined into each specific model definition.

@markstory markstory commented on the diff Nov 8, 2011

lib/Cake/Model/Datasource/DataSource.php
@@ -90,7 +90,7 @@ class DataSource extends Object {
* @param mixed $data
* @return array Array of sources available in this datasource.
*/
- public function listSources($data = null) {
+ public function cachedListSources($data = null) {
@markstory

markstory Nov 8, 2011

Owner

This is an API breaking change, it cannot be merged in.

Owner

markstory commented Nov 8, 2011

The next time this change could be merged in would be 3.0, which is a ways off.

Contributor

challet commented Nov 8, 2011

If you agree on that change, you may want to keep it until then.
Despite the reason why I did it, I think it's more consistent with the Model::setSource test I told before.

For my part, I'll maintain my own fork to have it. This is the only way I've found to use a schema-less Datasource.
I also heard about a MongoDB Datasource, which may have the same feature to deal with. I'll take a look at it and try to find something better for my specific case.

Contributor

Ivoz commented Nov 14, 2011

@challet - an officially supported couchedb datasource can be found here

https://github.com/cakephp/datasources/blob/master/models/datasources/couchdb_source.php

Maybe you can take some inspiration from that :)

Contributor

challet commented Nov 14, 2011

thanks @Ivoz, I wasn't aware of it ! and i'll definitely take a look at it !

It may occur that I'll pull request on top of it :p

Contributor

challet commented Mar 29, 2012

deprecated request

challet closed this Mar 29, 2012

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