Skip to content

Conversation

@f2cmb
Copy link
Contributor

@f2cmb f2cmb commented Dec 17, 2025

#22377

Fixes cache issue for translation override: Include override files hash in Translator cache key to ensure cache is invalidated when locale override files are modified.

@f2cmb f2cmb marked this pull request as ready for review December 17, 2025 10:10
Copy link
Contributor

@AdrienClairembault AdrienClairembault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if theses files are supposed to be modified manually in between official releases.

Isn't the solution to just let the user clean his cache if he does so?

@trasher
Copy link
Contributor

trasher commented Dec 18, 2025

Isn't the solution to just let the user clean his cache if he does so?

Agree

@f2cmb
Copy link
Contributor Author

f2cmb commented Dec 18, 2025

#22377 (comment) User already cleared the cache (CLI + GUI) and it does not seems to work.

Without the override hash in the cache key, the key stays identical before/after adding override files, so cache clearing alone doesn't force a proper reload. I think this modification is relevant.

In addition, the changes in the cache behaviour is targeted only on languages.

unlink(GLPI_LOCAL_I18N_DIR . '/core/en_GB.mo');
}

public function testTranslationOverrideCacheInvalidation()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is green without the fix

@f2cmb f2cmb requested a review from trasher December 18, 2025 14:07
Copy link
Contributor

@trasher trasher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested, loadLanguage seems correct; but getOverrideFilesHash shoudl be tested (if not possible to test really, at least this hash calculation should be).

@AdrienClairembault
Copy link
Contributor

User already cleared the cache (CLI + GUI) and it does not seems to work.

It seems wrong, look at this unrelated PR I've worked on yesterday: https://github.com/glpi-project/glpi/pull/22436/changes

After re-enabling the translation cache for the units tests, I've had to update a test that modify the locales files.
The test was fixed without issue by clearing the cache.

I don't think the user in the original issue cleared the cache properly tbh.

@f2cmb
Copy link
Contributor Author

f2cmb commented Dec 19, 2025

getOverrideFilesHash() method is private static, so it cannot be tested directly without reflection (which GLPI test suite doesn't use).

However, testLocalI18nCacheInvalidation() indirectly proves the hash logic works correctly

@trasher
Copy link
Contributor

trasher commented Jan 5, 2026

getOverrideFilesHash() method is private static, so it cannot be tested directly without reflection (which GLPI test suite doesn't use).

I see several use of ReflectionClass in test suite; but anyway. Please rebase to solve conflicts and run tests suite.

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.

3 participants