------------
Steps
------------

1- Install module i18n 6.x-1.0 and enable everything for node translation in two languages (in my case English and French, English being the default language);
2- Install module CCK 6.x-2.1;
3- Create a new content type containing at least one CCK field (e.g. a text field);
4- Set the following in the new content type: Multilingual support (enabled with translation), Synchronize translations (at least one field must be checked);
5- Create an instance of that node. Save it with the language set to "English";
6- Translate that node in French using the "translate" tab;
----->Problem: after the translation, the source node and the translation node are in the following state:
source: nid=32 tnid=0
translation: nid=36 tnid=32
-----> The translation node is alright, but the tnid of the source node should be 32 and not 0;

------------
Problem
------------

When creating a new translated node, and at least one field is synced between the source and the translation, here's what happens in a normal situation:

1- The "insert" operation of the nodeapi hook is invoked for the new node;
2- The nodeapi hook of translation.module is called. The tnid attribute of both the source and the translated nodes are set *directly in the database*, using a sql query.
3- The nodeapi hook of i18nsync.module is called. In order to sync the source fields with the new translated node attributes, the source node is loaded using node_load. Then it is modified and saved using node_save.

So far so good. Now, the problem when the source node has CCK fields, is that node_load is somehow called once by the CCK module *before* the step 1 above. Because of that, the source node is already present in the static cache of node_load() when step 1 is reached. And at this point, the tnid of the source node is still 0. Then, at step 2, the tnid is set directly in the database. However, at step 3, the call to node_load will get the cached source node (with tnid==0), and not the data that was saved in the database. The next call to node_save() therefore sets the tnid attribute back to 0 in the database, at the end of step 3.

---------------
Solution
---------------

The problem may be fixed in many different ways. The solution I propose is to invalidate the node_load() cache as soon as a query modifies the "node" table in the database. This fixes the problem described above. This is what my patch is about.

NOTE 1: It would improve performance to invalidate only the cache for the node that is
modified, instead of invalidating the whole cache. It would require a change to node_load.

NOTE 2: It looks like the static cache in node_load could be problematic in other situations
as well. Shall we trace all the places where the "node" table is modified directly?

-----------------
Configuration
-----------------

Drupal core 6.9
module i18n 6.x-1.0
module CCK 6.x-2.1
Linux / MAC OS X
Apache 2.0
PHP 5.2.5

Thanks,

jf

CommentFileSizeAuthor
translation-6.9-i18n-and-CCK-null-tnid-fix.patch2 KBjfduchesneau

Comments

David Lesieur’s picture

David Lesieur commented
Version: 6.9 » 6.x-dev

I have experienced the same problem on a site with i18nsync.module enabled — solved by this patch!

The patch really makes sense to me and the issue is not necessarily specific to i18n or CCK. If an implementation of hook_nodeapi() modifies a node (other than the one passed as argument) directly in the database, which is what translation_nodeapi() does, it should invalidate node_load()'s cache in case other modules try to access the changed node later on. This is what this patch does.

The only drawback (as stated in the above NOTE 1) is that the whole node_load() cache gets cleared. It would be great if node_load() allowed to reset the cache just for a specific node.
If it wasn't for this drawback, I'd mark the issue as RTBC, but others' feedback on this subject might be interesting.

The patch also applies to the current DRUPAL-6 branch.

David Lesieur’s picture

David Lesieur commented

See also: #367118: Translation relationship broken with i18nsync.

azoho’s picture

azoho commented

I am encountering this problem also, but the the patch provided seems to be for modules/translation/translation.module which I do not have installed,

the original post refers to module i18n 6.x-1.0 though

Is this patch for i18n 6.x-1.0 and where do I apply it?

Upgraded Drupal to 6.11 which apparently has improved cache handling but not made any difference for me with this issue

Thanks

David Lesieur’s picture

David Lesieur commented

As you have noted, the patch works against the translation module. This module is in Drupal core so you should look at the modules directory that's at your site's root. Since i18n depends on translation, it should be enabled on your site already.

azoho’s picture

azoho commented

Thanks David for pointing that out.

I have added the lines of code from the patch to translate.module, and have updated the database, but still any translations I make are not getting linked back to the source nodes.

I believe I have used the patch correctly, adding the lines starting with + into code indicated by the lines of code above and below the +

I'm running Drupal 6.11 if that makes a difference to the version of the translate.module I am using

David Lesieur’s picture

David Lesieur commented

You might want to try the dev version of i18n. If the problem persists with that version, please follow up in #367118: Translation relationship broken with i18nsync.

najibx’s picture

najibx commented

hi ...

I have this problem. Created new content type without any CCK field. Problem still persist.

As per http://drupal.org/node/367118, in i18nsync.module. is already been patched/replaced with $translation = node_load($translation->nid, NULL, TRUE);

I tried disabling i18nsync, but same result.

BTW, my default language is English (try to translate to other)

najibx’s picture

najibx commented

i use patch above on 6.11 anyway, but still failing :-( Even content type without cck field
actually I was able to translate before, after been working on the site several months (adding other modules), etc ...it failing now.
i have tried disable certain modules, but no success.

any tips here...please.

andremolnar’s picture

andremolnar commented

I thought I would post here in case you were running into this issue, but it doesn't appear to be related to the specific case mentioned in this issue.

A situation which will also cause this to happen is if you happen to have workflow module enabled and have configured a workflow that includes a 'save' action as part of a transition.

If that's the case its possible that two save operations happen on the node creation. The second one will break the translation set.

Long story short: Try disabling workflows or removing save operations from your workflow transitions. See if that helps.

David Lesieur’s picture

David Lesieur commented

@andremolnar: Besides disabling workflows or removing the save operations, did you have an opportunity to check if the patch could help as well?

najibx’s picture

najibx commented

Thank youuuuuu !!! In my case, not Workflow, but I have Rules module installed, and removing save operations from the "Triggered rules" solve this long over standing issues...

Thank you again ..and not sure, where this issue should goes, and if already in discussion somewhere. Sure, it's not related in this particular issues, but as critical.

andremolnar’s picture

andremolnar commented

@David Lesieur The patch didn't help one way or another in my case. In fact when I was tracing the code I couldn't actually see a case where the node_load static variable caching could have been the cause of the problem (not to say that it isn't in other cases - it may be).

Can anyone verify that there is still a bug with core + cck + i18n?

If not that we can recycle this issue by changing its 'title' to be more descriptive of actual cases that cause the 'tnid of source node is 0 after translation'.

David Lesieur’s picture

David Lesieur commented

There is an issue with node_load's static caching nevertheless, so if the problems you have found are unrelated, then I guess they should be posted as a separate issue... But I agree, we should probably change the title.

jfduchesneau’s picture

jfduchesneau commented
Title: tnid of source node is still 0 after translation when using i18n and CCK » node_load's static cache is outdated when bypassed by a direct modification to the "node" table.

As suggested by both andremolnar and David Lesieur, the title of the issue has been changed.

The real problem is about the node_load's static cache being bypassed by a direct modification to the "node" table. One symptom of that is, as I experimented, the tnid of source nodes being stuck to a value of 0. However, it is quite possible that other symptoms may be related to this outdated node_load static cache. Therefore, the important part of the issue is not about the value of "tnids", but about node_load's cache being bypassed.

Of course this patch may not fix all tnid==0 issues, but in my case it solved the problem. Hopefully it will help others. In any case, I believe node_load's cache being outdated may lead to all sorts of weird problems. It's why I think this issue is important, and should by fixed in one way or another.

Thanks for all the feedback.

multiplextor’s picture

multiplextor commented
Status: Needs review » Closed (won't fix)

Closed. The reason: expired.