Updated: Comment #7

Problem/Motivation

Static cache for language_list() is not cleared in _locale_rebuild_js()

Proposed resolution

The patch attached may work. However the file it is applied to no longer exists. A new patch should be created to work on the same function, which now lives in locale.module

Remaining tasks

The patch needs to be recreated for the proper file and tested

User interface changes

n/a

API changes

n/a

#1260510: Introduce a language_load($langcode)

Original report

There's a comment about consistency in runtime and database but we have a static cache for language_list() that is not cleared...

I found it working on #1260510: Introduce a language_load($langcode)
But suppose it should be fixed and backported to D7

Files: 
CommentFileSizeAuthor
#17 drupal8_static_cache-1261212-17.patch2.1 KBstefank
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,687 pass(es). View
#17 drupal8_static_cache_test_only-1261212-17.patch1.56 KBstefank
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,696 pass(es). View
#15 drupal8_static_cache-1261212-15.patch2.13 KBstefank
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,686 pass(es). View
#12 drupal8_static_cache_simpletest-1261212-11.patch1.63 KBpiyuesh23
PASSED: [[SimpleTest]]: [MySQL] 56,720 pass(es). View
#10 drupal8-static_cache_reroll-1261212-10.patch550 bytespwieck
PASSED: [[SimpleTest]]: [MySQL] 57,804 pass(es). View
#8 drupal8-static_cache_reroll-1261212.patch659 bytesKRaisor
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in drupal8-static_cache_reroll-1261212.patch. View
locale_static.patch735 bytesandypost
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch locale_static.patch. See the log in the details link for more information. View

Comments

Status: Needs review » Needs work

The last submitted patch, locale_static.patch, failed testing.

webchick’s picture

Issue tags: +needs backport to D7

Fixing tag.

Gábor Hojtsy’s picture

Issue tags: +language-ui

Adding UI language translation tag.

andypost’s picture

Currently the logic here changed - language_delete call for invalidation (deferred clean-up), but actual _locale_rebuild_js() is used language_list() so deleted language files seems get's lost

so needs testing

YesCT’s picture

Probably needs a reroll.

Tests might be able to be written, even if a fix is not obvious.

An issue summary update using the issue summary template, and remaining tasks, might help.

#1260510: Introduce a language_load($langcode) might be useful to write steps to reproduce.

xjm’s picture

andypost’s picture

probably this could be covered with unittest

KRaisor’s picture

Status: Needs work » Needs review
FileSize
659 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in drupal8-static_cache_reroll-1261212.patch. View

Patch re-rolled as the code has been significantly re-factored.

Status: Needs review » Needs work

The last submitted patch, drupal8-static_cache_reroll-1261212.patch, failed testing.

pwieck’s picture

Status: Needs work » Needs review
FileSize
550 bytes
PASSED: [[SimpleTest]]: [MySQL] 57,804 pass(es). View

re-rolled with current head

pwieck’s picture

Issue tags: -Needs reroll

#10 passed and ready for review.

piyuesh23’s picture

FileSize
1.63 KB
PASSED: [[SimpleTest]]: [MySQL] 56,720 pass(es). View

Attaching a simpletest written to test this case.

piyuesh23’s picture

piyuesh23’s picture

Issue summary: View changes

Adding summary template

jhedstrom’s picture

Issue summary: View changes
Status: Needs review » Needs work

The fix from #10 and the test from #12 should be in a single patch. It's also odd that the test passes without the fix.

stefank’s picture

Status: Needs work » Needs review
FileSize
2.13 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,686 pass(es). View

Here is a patch with the fix and the test.

andypost’s picture

@stefank please file a separate patch without fix (test only) to make sure that the test covers the bug.
Also 2 nitpicks, check another tests to format class properly

+++ b/core/modules/locale/src/Tests/LocaleStaticTest.php
@@ -0,0 +1,43 @@
+  public static $modules = array('locale');
...
+  }
+}

needs empty line

stefank’s picture

FileSize
1.56 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,696 pass(es). View
2.1 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,687 pass(es). View

Thanks @andypost.
I've corrected the format, but Im still not sure about the text- if it does make sense.

Gábor Hojtsy’s picture

+++ b/core/modules/locale/locale.module
@@ -1166,6 +1166,7 @@ function _locale_invalidate_js($langcode = NULL) {
+  drupal_static_reset('language_list');

Does this do anything anymore? language_list is just a deprecated wrapper now for the language manager. That does have an internal cache that could be reset, but this static reset will not do that right? So looks like the bug is gone, no? That the test only patch passes supports that too :)

andypost’s picture

Yep, seems only d7 affected now or test is outdated

jhedstrom’s picture

Version: 8.0.x-dev » 7.x-dev
Status: Needs review » Needs work

Moving to 7 based on #19 and #18.