Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.0 - Associations and different connections #5369

Closed
patrickconroy opened this issue Dec 9, 2014 · 24 comments
Closed

3.0 - Associations and different connections #5369

patrickconroy opened this issue Dec 9, 2014 · 24 comments

Comments

@patrickconroy
Copy link
Contributor

If I have the following setup:

class ItemsTable extends Table {
    public function initialize(array $config = []) {
        $this->belongsTo('Posts');
    }
    public static function defaultConnectionName() {
        return 'custom';
    }
}

class PostsTable extends Table {
    public static function defaultConnectionName() {
        return 'default';
    }
}

And I try to get Items..

$items = TableRegistry::get('Items')->find('all')->contain(['Posts'])->toArray();

It errors out, complaining that the Posts table does not exist in the custom connection. Am I doing something wrong? I thought there might be a way to specify the connection in the association directly..
$this->belongsTo('Posts', ['connection' => ['default']);
but there isn't. And I really don't think I should need to specify default as the connection for Posts, as I'd expect Tables to use default by default even if their parent uses a custom one.

It will also error out if my Posts table is translated, since the Query doesn't know that my i18n table isn't in my custom connection, it's in the default one.

Is there a DataSource config option to make every Table use their connection's DB as a prefix? That would halfway solve this, but the i18n issue would still happen.

Thanks.

@markstory markstory added this to the 3.0.0 milestone Dec 9, 2014
@markstory
Copy link
Member

You'll have to set the strategy on your belongsTo to use the 'select' strategy. As cross datasource joins are not supported right now.

@patrickconroy
Copy link
Contributor Author

Thanks @markstory, that did it!

@lorenzo
Copy link
Member

lorenzo commented Dec 9, 2014

Another way is to call table() in the initialize method with the full table name including the database name prefix

@patrickconroy
Copy link
Contributor Author

I tried that actually but it still got confused with the i18n stuff.

@lorenzo
Copy link
Member

lorenzo commented Dec 9, 2014

Ah ok, that requires a bit of clever changes. Like creating your own i18n object in the registry

@patrickconroy
Copy link
Contributor Author

Actually, today I needed to get i18n from a connection other than default and it's a little buggy. It looks like TranslateBehavior uses the schema from the default connection when nowhere in my code am I telling it to use default for the i18n stuff. Since there's a difference in my schema between default/custom (default has a scope field), it errors out with:

Error: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'Table_title_translation.scope' in 'field list'
public function initialize(array $config = []) {
    $this->addBehavior('Translate', ['fields' => ['title', 'description']]);
}
public static function defaultConnectionName() {
    return 'custom';
}

default connection schema:

CREATE TABLE `i18n` (
  `id` int(10) NOT NULL AUTO_INCREMENT,
  `locale` varchar(6) CHARACTER SET latin1 NOT NULL,
  `model` varchar(255) CHARACTER SET latin1 NOT NULL,
  `foreign_key` int(10) NOT NULL,
  `field` varchar(255) CHARACTER SET latin1 NOT NULL,
  `content` mediumtext,
  `scope` varchar(20) CHARACTER SET utf8 COLLATE utf8_bin DEFAULT NULL,
...
) ENGINE=InnoDB AUTO_INCREMENT=580207 DEFAULT CHARSET=utf8;

custom connection schema:

CREATE TABLE `i18n` (
  `id` int(10) NOT NULL AUTO_INCREMENT,
  `locale` varchar(6) CHARACTER SET latin1 NOT NULL,
  `model` varchar(255) CHARACTER SET latin1 NOT NULL,
  `foreign_key` int(10) NOT NULL,
  `field` varchar(255) CHARACTER SET latin1 NOT NULL,
  `content` mediumtext CHARACTER SET utf8,
  PRIMARY KEY (`id`),
  UNIQUE KEY `uniquegrouping` (`locale`,`model`,`field`,`foreign_key`),
 ...
) ENGINE=InnoDB AUTO_INCREMENT=154 DEFAULT CHARSET=utf8 COLLATE=utf8_bin;

This happens even if I set the translationTable option..

$this->addBehavior('Translate', ['translationTable' => 'custom.i18n', 'fields' => ['title', 'description']]);

Adding the scope field to the custom connection works, and I get the correct translated data, although I think this schema mixup is a bug.

@patrickconroy patrickconroy reopened this Dec 11, 2014
@patrickconroy
Copy link
Contributor Author

A much easier way to have described the bug I'm seeing is that if your default connection has no i18n table, it'll error out even though your Table is using a custom connection. When trying to do a find, cake is trying to do the following without a context for which connection it should be using:

2014-12-11 00:29:11 Debug: SHOW FULL COLUMNS FROM `i18n`

@markstory
Copy link
Member

Well there is only 1 I18n table object, and if it is undefined it will use 'default' connection. Do you have i18n tables in multiple connections?

@patrickconroy
Copy link
Contributor Author

Hm, where is that I18nTable? I have multiple i18n tables in different connections, yes. I tried creating a App\Model\Table\I18nsTable but it's never called. I think it might make sense if you could do something like:

$this->addBehavior('Translate', ['translationTableClass' => 'App\Model\Table\I18nsTable', 'fields' => ['title', 'description']]);

I don't know how else I could have a single I18nTable, unless you'd allow specifying the connection like:

$this->addBehavior('Translate', ['connection' => 'custom', 'fields' => ['title', 'description']]);

@markstory
Copy link
Member

The default table alias name is i18n.

@patrickconroy
Copy link
Contributor Author

But right now I can't change that on a per-Table basis, right? I can specify a different table but the framework doesn't know which connection to get its schema from, it always uses the default connection's schema.

@patrickconroy
Copy link
Contributor Author

After looking at this more, it looks like the right way to do it is to change the fundamental way the i18n table is declared. If you allow specifying the translated table class instead of the table name, and edit the core TranslateBehavior, things will work as expected..

$this->addBehavior('Translate', ['translationTable' => 'Plugin.TranslatedField', 'fields' => ['title', 'description']]);

Then in that class, define stuff..

<?php
namespace Plugin\Model\Table;

use Cake\ORM\Table;

class TranslatedFieldTable extends Table {
    public function initialize(array $config = []) {
        $this->table('i18n');
    }
    public static function defaultConnectionName() {
        return 'whatev';
    }
}

public function setupFieldAssociations($fields, $table) {
    $alias = $this->_table->alias();
    foreach ($fields as $field) {
        $name = $this->_table->alias() . '_' . $field . '_translation';
        $this->_table->hasOne($name, [
            'className' => $table,
            'foreignKey' => 'foreign_key',
            'joinType' => 'LEFT',
            'conditions' => [
                $name . '.model' => $alias,
                $name . '.field' => $field,
            ],
            'propertyName' => $field . '_translation'
        ]);
    }

    $this->_table->hasMany($table, [
        'foreignKey' => 'foreign_key',
        'strategy' => 'subquery',
        'conditions' => ["$table.model" => $alias],
        'propertyName' => '_i18n',
        'dependent' => true
    ]);
}

@markstory
Copy link
Member

@patrickconroy That sounds like a good idea to me. Would you be able to make a pull request for this change?

@patrickconroy
Copy link
Contributor Author

@markstory, I started this at patrickconroy@d3f3533 , but I've gotten to a point where I don't know which function is supposed to be merging the results found in the i18n table back to the main record. The beforeFind seems to want to do that but it isn't right now. This is the an example Entity I'm getting right now.. I'd need those properties in the 'en_us' Entity to be applied to the main Entity. Paging Dr. @lorenzo as he wrote the behavior and might have some insight..

[properties] => Array
        (
            [id] => 1
            [img] => Text..
            [_locale] => en_us
            [_translations] => Array
                (
                    [en_us] => Cake\ORM\Entity Object
                        (
                            [new] => 
                            [accessible] => Array
                                (
                                    [*] => 1
                                )

                            [properties] => Array
                                (
                                    [tagline] => Text..
                                    [title] => Text..
                                    [locale] => en_us
                                )

                            [dirty] => Array
                                (
                                )

                            [original] => Array
                                (
                                )

                            [virtual] => Array
                                (
                                )

                            [errors] => Array
                                (
                                )

                            [repository] => 
                        )

@AD7six
Copy link
Member

AD7six commented Dec 15, 2014

which function is supposed to be merging the results found in the i18n table back to the main record

That's _rowMapper. If you inject a debug($row); die; into that method:

protected function _rowMapper($results, $locale) {
    return $results->map(function ($row) use ($locale) {
        debug ($row); die;

You will probably be able to see the wizard behind the curtain.

@patrickconroy
Copy link
Contributor Author

I tried debugging in there, but the translated fields are NULL, so they're skipped when this happens:

if ($translation === null || $translation === false) {
    unset($row[$name]);
    continue;
}

The fields shouldn't be NULL, though..

Cake\ORM\Entity Object
(
    [new] => 
    [accessible] => Array
        (
            [*] => 1
        )

    [properties] => Array
        (
            [id] => 1
            [img] => TEXT...
            [tagline_translation] => NULL
            [_i18n] => Array
                (
                    [0] => Cake\ORM\Entity Object
                        (
                            [new] => 
                            [accessible] => Array
                                (
                                    [*] => 1
                                )

                            [properties] => Array
                                (
                                    [id] => 1393
                                    [locale] => en_us
                                    [model] => Features
                                    [foreign_key] => 1
                                    [field] => tagline
                                    [content] => Stuff is here so why isn't it within `tagline_translation` ?
                                )

                            [dirty] => Array
                                (
                                )

                            [original] => Array
                                (
                                )

                            [virtual] => Array
                                (
                                )

                            [errors] => Array
                                (
                                )

                            [repository] => I18n
                        )

@AD7six
Copy link
Member

AD7six commented Dec 15, 2014

Oh extra pointer:

This is the an example Entity I'm getting right now.. I'd need those properties in the 'en_us' Entity to be applied to the main Entity

I believe you are currently calling find('translations') - since that's the only time _i18n is populated. There isn't any merge-back-to-main-entity when calling find('translations') (look at the executed sql, for the presence of foo_translation joins).

@patrickconroy
Copy link
Contributor Author

Hm, yeah in my Controller I have..
TableRegistry::get('Features')->find('translations');
but those fields aren't NULL in _rowMapper when I do this with the current 3.0 TranslateBehavior..

Cake\ORM\Entity Object
(
    [new] => 
    [accessible] => Array
        (
            [*] => 1
        )

    [properties] => Array
        (
            [id] => 1
            [img] => Text..
            [tagline_translation] => Cake\ORM\Entity Object
                (
                    [new] => 
                    [accessible] => Array
                        (
                            [*] => 1
                        )

                    [properties] => Array
                        (
                            [id] => 1393
                            [locale] => en_us
                            [model] => Features
                            [foreign_key] => 1
                            [field] => tagline
                            [content] => Hey..
                        )

                    [dirty] => Array
                        (
                        )

                    [original] => Array
                        (
                        )

                    [virtual] => Array
                        (
                        )

                    [errors] => Array
                        (
                        )

                    [repository] => Features_tagline_translation
                )

(updated the debug from _rowMapper)

@AD7six
Copy link
Member

AD7six commented Dec 15, 2014

I took a closer look at what you've done.

Here's a gist for direct comparison: https://gist.github.com/AD7six/971b4773e7819277449f

The I18n key shouldn't be present and more or less indicates that the property name isn't doing what is expected

The problem is related to this: $targetAlias = $target->alias(); - that's always going to be I18n.

@patrickconroy
Copy link
Contributor Author

I don't get how that's possible. Shouldn't propertyName in setupFieldAssociations be changing that? I don't know where the I18n key is coming from.. I don't have an I18n key, but I have an _i18n key, that has all the translated stuff in it.. And targetAlias shouldn't always be I18n, it can be any Table specified with translationTable (my goal with this change, really).

@lorenzo
Copy link
Member

lorenzo commented Dec 24, 2014

@patrickconroy are you able to follow up with the proposed changes? Let us know if you need help

@lorenzo lorenzo modified the milestones: 3.0.0, 3.1.0 Jan 2, 2015
@patrickconroy
Copy link
Contributor Author

I updated the TranslateBehavior @ patrickconroy@2bea4c1 with the changes that will make this work. Let me know if I should make a PR for that (it kind of diverges from the subject of this issue - it's to allow setting Tables for I18n purposes).

Thanks

@lorenzo
Copy link
Member

lorenzo commented Jan 6, 2015

Yeas, please. Make it a PR

@lorenzo
Copy link
Member

lorenzo commented Jan 15, 2015

Closing as the PR for translate behavior was merged

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

No branches or pull requests

4 participants