Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

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

Closed
wants to merge 2 commits into from

3 participants

Clément Hallet Mark Story Matt Iversen
Clément Hallet

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.

Mark Story markstory commented on the diff
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) {
Mark Story Owner

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

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

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

Clément Hallet

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.

Matt Iversen

@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 :)

Clément Hallet

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

Clément Hallet

deprecated request

Clément Hallet challet closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 8, 2011
  1. remove unrelated changes

    Clément Hallet authored
This page is out of date. Refresh to see the latest.
2  lib/Cake/Model/Datasource/DataSource.php
View
@@ -90,7 +90,7 @@ public function __construct($config = array()) {
* @param mixed $data
* @return array Array of sources available in this datasource.
*/
- public function listSources($data = null) {
+ public function cachedListSources($data = null) {
Mark Story Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
if ($this->cacheSources === false) {
return null;
}
4 lib/Cake/Model/Datasource/Database/Mysql.php
View
@@ -184,7 +184,7 @@ public function enabled() {
* @return array Array of tablenames in the database
*/
public function listSources($data = null) {
- $cache = parent::listSources();
+ $cache = $this->cachedListSources();
if ($cache != null) {
return $cache;
}
@@ -201,7 +201,7 @@ public function listSources($data = null) {
}
$result->closeCursor();
- parent::listSources($tables);
+ $this->cachedListSources($tables);
return $tables;
}
}
4 lib/Cake/Model/Datasource/Database/Postgres.php
View
@@ -156,7 +156,7 @@ public function enabled() {
* @return array Array of tablenames in the database
*/
public function listSources($data = null) {
- $cache = parent::listSources();
+ $cache = $this->cachedListSources();
if ($cache != null) {
return $cache;
@@ -176,7 +176,7 @@ public function listSources($data = null) {
}
$result->closeCursor();
- parent::listSources($tables);
+ $this->cachedListSources($tables);
return $tables;
}
}
4 lib/Cake/Model/Datasource/Database/Sqlite.php
View
@@ -134,7 +134,7 @@ public function enabled() {
* @return array Array of tablenames in the database
*/
public function listSources($data = null) {
- $cache = parent::listSources();
+ $cache = $this->cachedListSources();
if ($cache != null) {
return $cache;
}
@@ -148,7 +148,7 @@ public function listSources($data = null) {
foreach ($result as $table) {
$tables[] = $table[0]['name'];
}
- parent::listSources($tables);
+ $this->cachedListSources($tables);
return $tables;
}
return array();
4 lib/Cake/Model/Datasource/Database/Sqlserver.php
View
@@ -170,7 +170,7 @@ public function enabled() {
* @return array Array of tablenames in the database
*/
public function listSources($data = null) {
- $cache = parent::listSources();
+ $cache = $this->cachedListSources();
if ($cache !== null) {
return $cache;
}
@@ -187,7 +187,7 @@ public function listSources($data = null) {
}
$result->closeCursor();
- parent::listSources($tables);
+ $this->cachedListSources($tables);
return $tables;
}
}
Something went wrong with that request. Please try again.