This is an automated email from the ASF dual-hosted git repository. joerghoh pushed a commit to branch SLING-13394 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-i18n.git
commit e0714f7fbbe21b32f8896e4ac28bc3878c1e691e Author: Joerg Hoh <[email protected]> AuthorDate: Mon Oct 19 14:23:58 2026 +0200 SLING-13394 avoid full reloads when individual messages are updated --- README.md | 13 + .../sling/i18n/impl/JcrResourceBundleProvider.java | 57 +- .../i18n/impl/JcrResourceBundleProviderTest.java | 582 ++++++++++++++++++++- .../sling/i18n/it/ResourceBundleProviderIT.java | 161 +++++- 4 files changed, 788 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 64dc559..cb7440f 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,19 @@ resources. Further details in <https://sling.apache.org/documentation/bundles/in ## Dictionary updates +Entry additions, edits, and removals below known dictionary roots refresh only +the bundles using those roots and their dependent locales, including all +matching basenames. Unrelated cached bundles remain unchanged. Changes to +dictionary roots (including locale or basename metadata), removal of their +ancestors, and discovery of new dictionaries still trigger a full refresh. +Observation respects the configured included and excluded paths. A legitimate +full refresh still clears the cache before loading replacements; this does not +eliminate the availability gap during that refresh. + +JSON content events below a known file root are targeted. Providers that report +content updates at the dictionary root, including the JCR provider's `nt:file` +events, retain full-refresh behavior. + When `preload.bundles` is enabled, an individual bundle refresh reloads parents before their dependent locales so that refreshed bundles use current fallback translations. With preloading disabled, affected bundles are invalidated and diff --git a/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java b/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java index 97047ab..0a1b1ec 100644 --- a/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java +++ b/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java @@ -50,6 +50,7 @@ import org.apache.sling.api.resource.ValueMap; import org.apache.sling.api.resource.observation.ExternalResourceChangeListener; import org.apache.sling.api.resource.observation.ResourceChange; import org.apache.sling.api.resource.observation.ResourceChangeListener; +import org.apache.sling.api.resource.path.Path; import org.apache.sling.commons.scheduler.ScheduleOptions; import org.apache.sling.commons.scheduler.Scheduler; import org.apache.sling.i18n.ResourceBundleProvider; @@ -261,8 +262,15 @@ public class JcrResourceBundleProvider change.getPath()); status.reloadAll = true; } else { + boolean withinKnownDictionary = false; for (final String root : languageRootPaths) { - if (change.getPath().startsWith(root)) { + if (change.getType() == ResourceChange.ChangeType.REMOVED && new Path(change.getPath()).matches(root)) { + // Removing an ancestor also removes its dictionaries. + status.reloadAll = true; + return; + } + if (new Path(root).matches(change.getPath())) { + withinKnownDictionary = true; // figure out which JcrResourceBundles from the cached ones is affected for (JcrResourceBundle bundle : resourceBundleRegistry.getResourceBundles()) { if (bundle.getLanguageRootPaths().contains(root)) { @@ -281,23 +289,45 @@ public class JcrResourceBundleProvider if (status.resourceResolver == null) { status.resourceResolver = createResourceResolver(); } - if (isDictionaryResource(status.resourceResolver, change)) { + if (requiresDictionaryRediscovery(status.resourceResolver, change, withinKnownDictionary)) { status.reloadAll = true; } } } - private boolean isDictionaryResource(final ResourceResolver resolver, final ResourceChange change) { - // language node changes happen quite frequently (https://issues.apache.org/jira/browse/SLING-2881) - // therefore only consider changes either for sling:MessageEntry's - // or for JSON dictionaries - // get valuemap + /** + * Checks whether the changed resource requires dictionary rediscovery and a full reload, + * rather than a targeted entry refresh. + * + * <p>Resources with {@code mix:language} and {@code jcr:language} require rediscovery, + * including new roots nested inside known dictionaries and JSON dictionary roots. + * {@code sling:MessageEntry} resources and resources with the {@code sling:Message} + * mixin require rediscovery only outside known dictionaries. Inside known dictionaries, + * the caller already collects the affected bundles for targeted refresh. + * + * <p>The caller separately handles changes to known roots and removal of their ancestors. + * Missing resources return {@code false}; targeted invalidations already collected for + * entry removals are retained. + * + * @param resolver resolver used to inspect the current resource + * @param change the observed resource change + * @param withinKnownDictionary whether the changed path is below a known dictionary root + * @return {@code true} if the resource indicates that dictionaries must be rediscovered + */ + private boolean requiresDictionaryRediscovery( + final ResourceResolver resolver, final ResourceChange change, final boolean withinKnownDictionary) { final Resource resource = resolver.getResource(change.getPath()); if (resource == null) { log.trace("Could not get resource for '{}' for event {}", change.getPath(), change.getType()); return false; } - if (resource.getResourceType() == null) { + final ValueMap valueMap = resource.getValueMap(); + // A new language root can also be nested inside a known dictionary. + if (hasMixin(valueMap, JcrResourceBundle.MIXIN_LANGUAGE) && valueMap.containsKey(PROP_LANGUAGE)) { + log.debug("Found dictionary root change at '{}'", change.getPath()); + return true; + } + if (withinKnownDictionary || resource.getResourceType() == null) { return false; } if (resource.isResourceType(JcrResourceBundle.RT_MESSAGE_ENTRY)) { @@ -307,7 +337,6 @@ public class JcrResourceBundleProvider change.getPath()); return true; } - final ValueMap valueMap = resource.getValueMap(); // FIXME: derivatives from mix:Message are not detected if (hasMixin(valueMap, JcrResourceBundle.MIXIN_MESSAGE)) { log.debug( @@ -316,16 +345,6 @@ public class JcrResourceBundleProvider change.getPath()); return true; } - if (change.getPath().endsWith(".json")) { - // check for mixin - if (hasMixin(valueMap, JcrResourceBundle.MIXIN_LANGUAGE)) { - log.debug( - "Found new dictionary: New {} resource in '{}' detected", - JcrResourceBundle.MIXIN_LANGUAGE, - change.getPath()); - return true; - } - } return false; } diff --git a/src/test/java/org/apache/sling/i18n/impl/JcrResourceBundleProviderTest.java b/src/test/java/org/apache/sling/i18n/impl/JcrResourceBundleProviderTest.java index 3c335fc..c35b173 100644 --- a/src/test/java/org/apache/sling/i18n/impl/JcrResourceBundleProviderTest.java +++ b/src/test/java/org/apache/sling/i18n/impl/JcrResourceBundleProviderTest.java @@ -18,18 +18,596 @@ */ package org.apache.sling.i18n.impl; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; import java.util.Locale; - +import java.util.Map; +import java.util.ResourceBundle; +import java.util.stream.Collectors; + +import org.apache.sling.api.resource.ModifiableValueMap; +import org.apache.sling.api.resource.Resource; +import org.apache.sling.api.resource.ResourceResolver; +import org.apache.sling.api.resource.ResourceResolverFactory; +import org.apache.sling.api.resource.observation.ResourceChange; +import org.apache.sling.api.resource.observation.ResourceChange.ChangeType; +import org.apache.sling.commons.scheduler.ScheduleOptions; +import org.apache.sling.commons.scheduler.Scheduler; +import org.apache.sling.serviceusermapping.ServiceUserMapped; import org.apache.sling.testing.mock.sling.junit.SlingContext; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; - +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.mockito.Mockito; +import org.osgi.framework.Constants; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; + +/** + * Covers locale handling and observation-driven refresh with eager and lazy loading. + * Observation tests explicitly run queued work, then check translations, dependent + * bundle references, and preservation of unrelated cached bundles. + */ +@RunWith(Parameterized.class) public class JcrResourceBundleProviderTest { + private static final String KEY = "checkout-label"; + private static final String[] BASENAMES = {null, "checkout", "store", "other"}; + private static final String RELOAD_ALL = "ResourceBundleProvider: reload all resource bundles"; + @Rule public final SlingContext context = new SlingContext(); + @Parameterized.Parameters(name = "preload_bundles={0}") + public static Iterable<Boolean> preloadBundles() { + return Arrays.asList(Boolean.FALSE, Boolean.TRUE); + } + + @Parameterized.Parameter + public boolean preload; + + private JcrResourceBundleProvider provider; + private final List<String> dictionaryPaths = new ArrayList<>(); + private final Map<String, Runnable> scheduledJobs = new LinkedHashMap<>(); + private String scheduledJobName; + + private void setupObservation() throws Exception { + Resource english = createDictionary("/apps/i18n/en", "en", "checkout", "store", "other"); + createEntry(english.getPath() + "/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "English"); + createDictionary("/apps/i18n/de", "de"); + createDictionary("/apps/i18n/fr", "fr"); + + ResourceResolver resolver = Mockito.spy(context.resourceResolver()); + Mockito.doNothing().when(resolver).close(); + Mockito.doAnswer(query -> dictionaries().iterator()) + .when(resolver) + .findResources(JcrResourceBundle.QUERY_LANGUAGE_ROOTS, "xpath"); + Mockito.doAnswer(query -> dictionaries().stream() + .map(root -> { + Map<String, Object> row = new HashMap<>(root.getValueMap()); + row.put("jcr:path", root.getPath()); + return row; + }) + .iterator()) + .when(resolver) + .queryResources(JcrResourceBundle.QUERY_LANGUAGE_ROOTS, "xpath"); + ResourceResolverFactory factory = Mockito.mock(ResourceResolverFactory.class); + Mockito.when(factory.getServiceResourceResolver(null)).thenReturn(resolver); + context.registerService(ResourceResolverFactory.class, factory, Constants.SERVICE_RANKING, 1000); + context.registerService(ServiceUserMapped.class, Mockito.mock(ServiceUserMapped.class)); + + Scheduler scheduler = context.registerService(Scheduler.class, Mockito.mock(Scheduler.class)); + ScheduleOptions options = Mockito.mock(ScheduleOptions.class); + Mockito.when(scheduler.NOW()).thenReturn(options); + Mockito.when(scheduler.AT(any(Date.class))).thenReturn(options); + Mockito.doAnswer(invocation -> { + scheduledJobName = invocation.getArgument(0); + return options; + }) + .when(options) + .name(any(String.class)); + Mockito.doAnswer(invocation -> { + scheduledJobs.put(scheduledJobName, invocation.getArgument(0)); + return true; + }) + .when(scheduler) + .schedule(any(Runnable.class), any(ScheduleOptions.class)); + Mockito.doAnswer(invocation -> scheduledJobs.remove(invocation.getArgument(0)) != null) + .when(scheduler) + .unschedule(any(String.class)); + + Map<String, Object> config = new HashMap<>(); + config.put("preload.bundles", preload); + config.put("locale.default", "en"); + config.put("included.paths", new String[] {"/apps", "/libs"}); + config.put("excluded.paths", new String[] {"/apps/excluded", "/apps/i18n/de/excluded"}); + provider = context.registerInjectActivateService(JcrResourceBundleProvider.class, config); + runScheduledWork(); + cacheBundles(); + } + + private Resource createDictionary(String path, String language, String... basenames) { + Resource root = context.create() + .resource( + path, + "jcr:primaryType", + "nt:unstructured", + "jcr:mixinTypes", + new String[] {JcrResourceBundle.MIXIN_LANGUAGE}, + "jcr:language", + language, + "sling:basename", + basenames); + dictionaryPaths.add(path); + return root; + } + + private List<Resource> dictionaries() { + return dictionaryPaths.stream() + .map(path -> context.resourceResolver().getResource(path)) + .filter(root -> root != null + && root.getValueMap().containsKey("jcr:language") + && Arrays.asList(root.getValueMap().get("jcr:mixinTypes", new String[0])) + .contains(JcrResourceBundle.MIXIN_LANGUAGE)) + .collect(Collectors.toList()); + } + + private Resource createEntry(String path, String type, String message) { + return context.create().resource(path, "jcr:primaryType", type, "sling:key", KEY, "sling:message", message); + } + + private Map<String, List<ResourceBundle>> cacheBundles() { + Map<String, List<ResourceBundle>> bundles = new LinkedHashMap<>(); + for (String basename : BASENAMES) { + bundles.put( + basename, + Arrays.asList( + provider.getResourceBundle(basename, Locale.ENGLISH), + provider.getResourceBundle(basename, Locale.GERMAN), + provider.getResourceBundle(basename, Locale.GERMANY), + provider.getResourceBundle(basename, Locale.FRENCH))); + } + return bundles; + } + + private void runScheduledWork() { + List<Runnable> jobs = new ArrayList<>(scheduledJobs.values()); + scheduledJobs.clear(); + jobs.forEach(Runnable::run); + } + + private void assertTargetedChange(String path, ChangeType type, String expected, String... affectedBasenames) { + Map<String, List<ResourceBundle>> before = cacheBundles(); + provider.onChange(Collections.singletonList(new ResourceChange(type, path, false))); + assertEquals(affectedBasenames.length, scheduledJobs.size()); + assertFalse("Entry edits must not schedule rediscovery", scheduledJobs.containsKey(RELOAD_ALL)); + Map<String, List<ResourceBundle>> pending = cacheBundles(); + before.forEach((basename, bundles) -> { + for (int i = 0; i < bundles.size(); i++) { + assertSame( + "Changes must wait for scheduled work", + bundles.get(i), + pending.get(basename).get(i)); + } + }); + runScheduledWork(); + Map<String, List<ResourceBundle>> after = cacheBundles(); + before.forEach((basename, bundles) -> { + boolean affected = Arrays.asList(affectedBasenames).contains(basename); + List<ResourceBundle> refreshed = after.get(basename); + assertSame("English fallback is unchanged", bundles.get(0), refreshed.get(0)); + assertSame("French is unrelated", bundles.get(3), refreshed.get(3)); + if (affected) { + assertNotSame(bundles.get(1), refreshed.get(1)); + assertNotSame("Dependent country bundle must refresh", bundles.get(2), refreshed.get(2)); + assertSame(refreshed.get(1), ((JcrResourceBundle) refreshed.get(2)).getParent()); + assertEquals(expected, refreshed.get(1).getString(KEY)); + assertEquals(expected, refreshed.get(2).getString(KEY)); + } else { + assertSame("Unrelated basename is unchanged", bundles.get(1), refreshed.get(1)); + assertSame(bundles.get(2), refreshed.get(2)); + } + }); + } + + private void assertFullReload(ResourceChange... changes) { + Map<String, List<ResourceBundle>> before = cacheBundles(); + provider.onChange(Arrays.asList(changes)); + assertEquals(Collections.singleton(RELOAD_ALL), scheduledJobs.keySet()); + runScheduledWork(); + Map<String, List<ResourceBundle>> after = cacheBundles(); + before.forEach((basename, bundles) -> { + for (int i = 0; i < bundles.size(); i++) { + assertNotSame( + "Discovery changes must invalidate every bundle", + bundles.get(i), + after.get(basename).get(i)); + } + }); + } + + private void assertIgnoredChange(ResourceChange... changes) { + Map<String, List<ResourceBundle>> before = cacheBundles(); + provider.onChange(Arrays.asList(changes)); + assertTrue(scheduledJobs.isEmpty()); + runScheduledWork(); + Map<String, List<ResourceBundle>> after = cacheBundles(); + before.forEach((basename, bundles) -> { + for (int i = 0; i < bundles.size(); i++) { + assertSame(bundles.get(i), after.get(basename).get(i)); + } + }); + } + + /** + * Adds, updates, and removes a typed entry in a known German dictionary. + * Only its bundles and dependent country bundles should refresh; removal restores the English fallback. + */ + @Test + public void knownMessageEntryChangesAreTargeted() throws Exception { + setupObservation(); + Resource entry = createEntry("/apps/i18n/de/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "Added"); + assertTargetedChange(entry.getPath(), ChangeType.ADDED, "Added", (String) null); + entry.adaptTo(ModifiableValueMap.class).put("sling:message", "Changed"); + assertTargetedChange(entry.getPath(), ChangeType.CHANGED, "Changed", (String) null); + context.resourceResolver().delete(entry); + assertTargetedChange(entry.getPath(), ChangeType.REMOVED, "English", (String) null); + } + + /** + * Verifies that entries using the message mixin receive the same targeted refresh + * treatment as entries using the message primary type. + */ + @Test + public void knownMessageMixinChangesAreTargeted() throws Exception { + setupObservation(); + Resource entry = createEntry("/apps/i18n/de/" + KEY, "nt:unstructured", "Added"); + entry.adaptTo(ModifiableValueMap.class).put("jcr:mixinTypes", new String[] {JcrResourceBundle.MIXIN_MESSAGE}); + assertTargetedChange(entry.getPath(), ChangeType.ADDED, "Added", (String) null); + entry.adaptTo(ModifiableValueMap.class).put("sling:message", "Changed"); + assertTargetedChange(entry.getPath(), ChangeType.CHANGED, "Changed", (String) null); + context.resourceResolver().delete(entry); + assertTargetedChange(entry.getPath(), ChangeType.REMOVED, "English", (String) null); + } + + /** + * Preserves observation support for entries loaded by their message properties alone, + * without requiring a message primary type or mixin. + */ + @Test + public void knownUntypedEntryChangesRemainTargeted() throws Exception { + setupObservation(); + Resource entry = createEntry("/apps/i18n/de/" + KEY, "nt:unstructured", "Added"); + assertTargetedChange(entry.getPath(), ChangeType.ADDED, "Added", (String) null); + entry.adaptTo(ModifiableValueMap.class).put("sling:message", "Changed"); + assertTargetedChange(entry.getPath(), ChangeType.CHANGED, "Changed", (String) null); + context.resourceResolver().delete(entry); + assertTargetedChange(entry.getPath(), ChangeType.REMOVED, "English", (String) null); + } + + /** + * A language mixin without a locale does not define a new dictionary root. + * Such an entry must still use the surrounding dictionary's targeted refresh. + */ + @Test + public void languageMixinWithoutLocaleDoesNotIntroduceDictionary() throws Exception { + setupObservation(); + Resource entry = createEntry("/apps/i18n/de/" + KEY, "nt:unstructured", "Added"); + entry.adaptTo(ModifiableValueMap.class).put("jcr:mixinTypes", new String[] {JcrResourceBundle.MIXIN_LANGUAGE}); + assertTargetedChange(entry.getPath(), ChangeType.ADDED, "Added", (String) null); + } + + /** + * A shared root contributes to unnamed bundles and multiple named bundles. + * Refresh every matching basename, but preserve bundles using only the other root. + */ + @Test + public void sharedRootRefreshesEveryAffectedBasename() throws Exception { + setupObservation(); + context.resourceResolver() + .getResource("/apps/i18n/de") + .adaptTo(ModifiableValueMap.class) + .put("sling:basename", new String[] {"checkout", "store"}); + Resource other = createDictionary("/apps/i18n/de-other", "de", "store", "other"); + context.create().resource(other.getPath() + "/secondary", "sling:message", "Other dictionary"); + provider.clearCache(); + cacheBundles(); + Resource entry = createEntry("/apps/i18n/de/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "Added"); + assertTargetedChange(entry.getPath(), ChangeType.ADDED, "Added", null, "checkout", "store"); + entry.adaptTo(ModifiableValueMap.class).put("sling:message", "Changed"); + assertTargetedChange(entry.getPath(), ChangeType.CHANGED, "Changed", null, "checkout", "store"); + context.resourceResolver().delete(entry); + assertTargetedChange(entry.getPath(), ChangeType.REMOVED, "English", null, "checkout", "store"); + } + + /** + * A typed entry outside known roots must trigger discovery and expose its translation. + * The sibling path de-extra must not be mistaken for a descendant of the known de root. + */ + @Test + public void unknownMessageEntryTriggersDiscovery() throws Exception { + setupObservation(); + createDictionary("/apps/i18n/de-extra", "de"); + Resource entry = createEntry("/apps/i18n/de-extra/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "Discovered"); + assertFullReload(new ResourceChange(ChangeType.ADDED, entry.getPath(), false)); + assertEquals("Discovered", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } + + /** + * A changed entry carrying the message mixin can reveal an unknown dictionary, + * even when the event is not an addition. + */ + @Test + public void unknownMessageMixinTriggersDiscovery() throws Exception { + setupObservation(); + createDictionary("/apps/i18n/de-extra", "de"); + Resource entry = createEntry("/apps/i18n/de-extra/" + KEY, "nt:unstructured", "Discovered"); + entry.adaptTo(ModifiableValueMap.class).put("jcr:mixinTypes", new String[] {JcrResourceBundle.MIXIN_MESSAGE}); + assertFullReload(new ResourceChange(ChangeType.CHANGED, entry.getPath(), false)); + assertEquals("Discovered", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } + + /** + * Adding a language root must trigger discovery even when its entries have no message type. + */ + @Test + public void unknownLanguageRootTriggersDiscovery() throws Exception { + setupObservation(); + Resource root = createDictionary("/apps/i18n/de-extra", "de"); + createEntry(root.getPath() + "/" + KEY, "nt:unstructured", "Discovered"); + assertFullReload(new ResourceChange(ChangeType.ADDED, root.getPath(), false)); + assertEquals("Discovered", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } + + /** + * A new root nested below a known dictionary can introduce a different locale and basename. + * Rediscovery must replace the previously cached empty Italian bundle. + */ + @Test + public void nestedLanguageRootTriggersDiscovery() throws Exception { + setupObservation(); + ResourceBundle italian = provider.getResourceBundle("checkout", Locale.ITALIAN); + Resource root = createDictionary("/apps/i18n/de/nested", "it", "checkout"); + createEntry(root.getPath() + "/" + KEY, "nt:unstructured", "Italian"); + assertFullReload(new ResourceChange(ChangeType.ADDED, root.getPath(), false)); + assertNotSame(italian, provider.getResourceBundle("checkout", Locale.ITALIAN)); + assertEquals( + "Italian", + provider.getResourceBundle("checkout", Locale.ITALIAN).getString(KEY)); + } + + /** + * An unknown dictionary in an event batch must promote the batch to a full reload, + * even if an earlier event only affected a known dictionary. + */ + @Test + public void discoveryInBatchIsNotHiddenByKnownEntry() throws Exception { + setupObservation(); + Resource known = createEntry("/apps/i18n/de/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "German"); + createDictionary("/apps/i18n/fr-extra", "fr"); + Resource unknown = createEntry("/apps/i18n/fr-extra/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "French"); + assertFullReload( + new ResourceChange(ChangeType.CHANGED, known.getPath(), false), + new ResourceChange(ChangeType.ADDED, unknown.getPath(), false)); + assertEquals("German", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + assertEquals("French", provider.getResourceBundle(Locale.FRENCH).getString(KEY)); + } + + /** + * Changing a root's locale must rediscover ownership: its translation moves to Italian, + * while German returns to its English fallback. + */ + @Test + public void rootLocaleChangeTriggersDiscovery() throws Exception { + setupObservation(); + cacheGermanEntry(); + Resource root = context.resourceResolver().getResource("/apps/i18n/de"); + root.adaptTo(ModifiableValueMap.class).put("jcr:language", "it"); + assertFullReload(new ResourceChange(ChangeType.CHANGED, root.getPath(), false)); + assertEquals("English", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + assertEquals("German", provider.getResourceBundle(Locale.ITALIAN).getString(KEY)); + } + + /** + * Changing root basename metadata must invalidate cached named bundles so that + * newly matching basenames receive the dictionary and nonmatching ones retain their fallback. + */ + @Test + public void rootBasenameChangeTriggersDiscovery() throws Exception { + setupObservation(); + cacheGermanEntry(); + Resource root = context.resourceResolver().getResource("/apps/i18n/de"); + root.adaptTo(ModifiableValueMap.class).put("sling:basename", new String[] {"checkout"}); + assertFullReload(new ResourceChange(ChangeType.CHANGED, root.getPath(), false)); + assertEquals( + "German", provider.getResourceBundle("checkout", Locale.GERMAN).getString(KEY)); + assertEquals( + "English", provider.getResourceBundle("store", Locale.GERMAN).getString(KEY)); + } + + /** + * Removing the language mixin makes a known root ineligible for discovery. + * A full refresh must discard its previously cached translations. + */ + @Test + public void rootLanguageMixinRemovalTriggersDiscovery() throws Exception { + setupObservation(); + cacheGermanEntry(); + Resource root = context.resourceResolver().getResource("/apps/i18n/de"); + root.adaptTo(ModifiableValueMap.class).remove("jcr:mixinTypes"); + assertFullReload(new ResourceChange(ChangeType.CHANGED, root.getPath(), false)); + assertEquals("English", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } + + /** + * Removing a known root requires a full refresh even though the resource can no longer + * be inspected; its former translations must fall back to English. + */ + @Test + public void rootRemovalTriggersDiscovery() throws Exception { + setupObservation(); + cacheGermanEntry(); + context.resourceResolver().delete(context.resourceResolver().getResource("/apps/i18n/de")); + assertFullReload(new ResourceChange(ChangeType.REMOVED, "/apps/i18n/de", false)); + assertEquals("English", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } + + private void cacheGermanEntry() { + createEntry("/apps/i18n/de/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "German"); + provider.clearCache(); + cacheBundles(); + assertEquals("German", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } + + /** + * Removing the locale property invalidates a known dictionary just like removing its + * language mixin, so cached translations must not survive rediscovery. + */ + @Test + public void rootLanguagePropertyRemovalTriggersDiscovery() throws Exception { + setupObservation(); + cacheGermanEntry(); + Resource root = context.resourceResolver().getResource("/apps/i18n/de"); + root.adaptTo(ModifiableValueMap.class).remove("jcr:language"); + assertFullReload(new ResourceChange(ChangeType.CHANGED, root.getPath(), false)); + assertEquals("English", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } + + /** + * A removal event on a dictionary container must invalidate all roots below it, + * without relying on separate removal events for each descendant. + */ + @Test + public void dictionaryAncestorRemovalTriggersDiscovery() throws Exception { + setupObservation(); + context.resourceResolver().delete(context.resourceResolver().getResource("/apps/i18n")); + assertFullReload(new ResourceChange(ChangeType.REMOVED, "/apps/i18n", false)); + assertEquals(KEY, provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } + + /** + * Shared string prefixes do not imply dictionary ancestry. + * Changes and removals under decorations must not invalidate the dictionary at de. + */ + @Test + public void rootPrefixDoesNotMatchSiblingPaths() throws Exception { + setupObservation(); + Resource entry = createEntry("/apps/i18n/decorations/" + KEY, "nt:unstructured", "Not a dictionary"); + assertIgnoredChange(new ResourceChange(ChangeType.CHANGED, entry.getPath(), false)); + context.resourceResolver().delete(entry.getParent()); + assertIgnoredChange(new ResourceChange(ChangeType.REMOVED, "/apps/i18n/decorations", false)); + } + + /** + * Path filters apply before both targeted refresh and discovery, including excluded + * descendants of known roots, excluded roots, and paths outside the include set. + */ + @Test + public void excludedAndNonIncludedChangesAreIgnored() throws Exception { + setupObservation(); + Resource known = createEntry("/apps/i18n/de/excluded/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "Excluded"); + Resource excludedRoot = createDictionary("/apps/excluded/de", "de"); + Resource excluded = + createEntry(excludedRoot.getPath() + "/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "Excluded"); + Resource outsideRoot = createDictionary("/content/i18n/de", "de"); + Resource outside = + createEntry(outsideRoot.getPath() + "/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "Outside"); + assertIgnoredChange( + new ResourceChange(ChangeType.ADDED, known.getPath(), false), + new ResourceChange(ChangeType.CHANGED, excluded.getPath(), false), + new ResourceChange(ChangeType.CHANGED, excludedRoot.getPath(), false), + new ResourceChange(ChangeType.REMOVED, known.getPath(), false), + new ResourceChange(ChangeType.ADDED, outside.getPath(), false)); + } + + private Resource createJsonDictionary(String path, String message) { + Resource root = createDictionary(path, "de"); + root.adaptTo(ModifiableValueMap.class).put("jcr:primaryType", "nt:file"); + context.create() + .resource( + path + "/jcr:content", + "jcr:primaryType", + "nt:resource", + "jcr:mimeType", + "application/json", + "jcr:data", + jsonData(message)); + return root; + } + + private byte[] jsonData(String message) { + return ("{\"" + KEY + "\":\"" + message + "\"}").getBytes(StandardCharsets.UTF_8); + } + + /** + * Direct events on a known JSON dictionary's content child allow targeted refresh. + * Replacing, removing, and restoring the data must update translations without replacing unrelated bundles. + */ + @Test + public void knownJsonContentChangesAreTargeted() throws Exception { + setupObservation(); + Resource root = createJsonDictionary("/apps/i18n/de.json", "Original"); + provider.clearCache(); + cacheBundles(); + Resource content = root.getChild("jcr:content"); + content.adaptTo(ModifiableValueMap.class).put("jcr:data", jsonData("Changed")); + assertTargetedChange(content.getPath(), ChangeType.CHANGED, "Changed", (String) null); + context.resourceResolver().delete(content); + assertTargetedChange(root.getPath() + "/jcr:content", ChangeType.REMOVED, "English", (String) null); + context.create() + .resource( + root.getPath() + "/jcr:content", + "jcr:primaryType", + "nt:resource", + "jcr:mimeType", + "application/json", + "jcr:data", + jsonData("Added")); + assertTargetedChange(root.getPath() + "/jcr:content", ChangeType.ADDED, "Added", (String) null); + } + + /** + * Adding an unknown JSON root must rediscover dictionaries and expose its translations. + */ + @Test + public void unknownJsonDictionaryTriggersDiscovery() throws Exception { + setupObservation(); + Resource root = createJsonDictionary("/apps/i18n/de.json", "Discovered"); + assertFullReload(new ResourceChange(ChangeType.ADDED, root.getPath(), false)); + assertEquals("Discovered", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } + + /** + * Events reported at the JSON root retain full-refresh behavior, unlike content-child events. + * Both a changed root and its subsequent removal must update the returned translations. + */ + @Test + public void knownJsonRootChangesTriggerDiscovery() throws Exception { + setupObservation(); + Resource root = createJsonDictionary("/apps/i18n/de.json", "Original"); + provider.clearCache(); + cacheBundles(); + root.getChild("jcr:content").adaptTo(ModifiableValueMap.class).put("jcr:data", jsonData("Changed")); + assertFullReload(new ResourceChange(ChangeType.CHANGED, root.getPath(), false)); + assertEquals("Changed", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + context.resourceResolver().delete(root); + assertFullReload(new ResourceChange(ChangeType.REMOVED, root.getPath(), false)); + assertEquals("English", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } + @Test public void testToLocale() { // empty string must return default locale diff --git a/src/test/java/org/apache/sling/i18n/it/ResourceBundleProviderIT.java b/src/test/java/org/apache/sling/i18n/it/ResourceBundleProviderIT.java index b6e7255..2524109 100644 --- a/src/test/java/org/apache/sling/i18n/it/ResourceBundleProviderIT.java +++ b/src/test/java/org/apache/sling/i18n/it/ResourceBundleProviderIT.java @@ -19,13 +19,22 @@ package org.apache.sling.i18n.it; import javax.inject.Inject; +import javax.jcr.Binary; import javax.jcr.Node; import javax.jcr.RepositoryException; import javax.jcr.Session; +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; +import java.util.Dictionary; +import java.util.Hashtable; +import java.util.List; import java.util.Locale; import java.util.ResourceBundle; +import java.util.concurrent.CopyOnWriteArrayList; +import org.apache.sling.api.resource.observation.ResourceChange; +import org.apache.sling.api.resource.observation.ResourceChangeListener; import org.apache.sling.i18n.ResourceBundleProvider; import org.apache.sling.jcr.api.SlingRepository; import org.junit.After; @@ -35,9 +44,15 @@ import org.junit.runner.RunWith; import org.ops4j.pax.exam.junit.PaxExam; import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy; import org.ops4j.pax.exam.spi.reactors.PerClass; +import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceRegistration; +import org.osgi.util.tracker.ServiceTracker; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @RunWith(PaxExam.class) @@ -55,7 +70,9 @@ public class ResourceBundleProviderIT extends I18nTestSupport { private SlingRepository repository; @Inject - private ResourceBundleProvider resourceBundleProvider; + private BundleContext bundleContext; + + private ServiceTracker<ResourceBundleProvider, ResourceBundleProvider> providerTracker; private Session session; private Node i18nRoot; @@ -89,6 +106,9 @@ public class ResourceBundleProviderIT extends I18nTestSupport { @Before public void setup() throws RepositoryException { + // Startup configuration can replace the service after Pax Exam injects test fields. + providerTracker = new ServiceTracker<>(bundleContext, ResourceBundleProvider.class, null); + providerTracker.open(); session = repository.loginAdministrative(null); final Node root = session.getRootNode(); final Node libs; @@ -111,6 +131,13 @@ public class ResourceBundleProviderIT extends I18nTestSupport { i18nRoot.remove(); session.save(); session.logout(); + providerTracker.close(); + } + + private ResourceBundleProvider getProvider() { + ResourceBundleProvider provider = providerTracker.getService(); + assertNotNull("No active ResourceBundleProvider service", provider); + return provider; } private Node addLanguageNode(Node parent, String language) throws RepositoryException { @@ -135,8 +162,8 @@ public class ResourceBundleProviderIT extends I18nTestSupport { @Override protected void exec() { { - final ResourceBundle resourceBundle = resourceBundleProvider.getResourceBundle( - basename, locale); // this is the resource bundle for de_DE + final ResourceBundle resourceBundle = + getProvider().getResourceBundle(basename, locale); // this is the resource bundle for de_DE assertNotNull(resourceBundle); assertEquals(value, resourceBundle.getString(key)); } @@ -179,7 +206,7 @@ public class ResourceBundleProviderIT extends I18nTestSupport { * Verifies that repository observation propagates an English fallback update * to German, German-country, and French bundles after they have been cached. * The nt:unstructured entry is supported by message loading but avoids the - * sling:MessageEntry detection that promotes edits to full rediscovery. + * sling:MessageEntry detection, covering untyped entries as well. * This exercises targeted refresh, where stale parent references would * otherwise leave dependent locales returning the old English value. */ @@ -196,6 +223,11 @@ public class ResourceBundleProviderIT extends I18nTestSupport { assertMessages(MSG_KEY2, "new English fallback", "new English fallback", "new English fallback"); } + /** + * Exercises repository observation for locale-specific entries, shared English fallbacks, + * and named dictionaries. Isolated German entry additions, edits, removals, and message-mixin + * additions must update German and country bundles without replacing English, French, or unrelated named bundles. + */ @Test public void testChangesDetection() throws RepositoryException { // set a key which is only available in the en dictionary @@ -237,5 +269,126 @@ public class ResourceBundleProviderIT extends I18nTestSupport { session.save(); assertMessage(MSG_KEY3, Locale.ENGLISH, null, "EN_basename_changed"); assertMessage(MSG_KEY3, Locale.ENGLISH, BASENAME, "EN_basename_changed"); + + // Wait for the preceding fallback refresh to reach cached children before checking isolation. + assertMessages(MSG_KEY3, "EN_basename_changed", "EN_basename_changed", "EN_basename_changed"); + ResourceBundle english = getProvider().getResourceBundle(Locale.ENGLISH); + ResourceBundle french = getProvider().getResourceBundle(Locale.FRENCH); + ResourceBundle named = getProvider().getResourceBundle(BASENAME, Locale.GERMAN); + ResourceBundle german = getProvider().getResourceBundle(Locale.GERMAN); + ResourceBundle country = getProvider().getResourceBundle(Locale.GERMANY); + + final String isolatedKey = "isolated"; + setMessage(deRoot, isolatedKey, "DE_added"); + session.save(); + assertMessages(isolatedKey, "DE_added", "DE_added", isolatedKey); + assertNotSame(german, getProvider().getResourceBundle(Locale.GERMAN)); + assertNotSame(country, getProvider().getResourceBundle(Locale.GERMANY)); + assertSame(english, getProvider().getResourceBundle(Locale.ENGLISH)); + assertSame(french, getProvider().getResourceBundle(Locale.FRENCH)); + assertSame(named, getProvider().getResourceBundle(BASENAME, Locale.GERMAN)); + + // A property change on an existing sling:MessageEntry must not rediscover every dictionary. + setMessage(deRoot, isolatedKey, "DE_changed"); + session.save(); + assertMessages(isolatedKey, "DE_changed", "DE_changed", isolatedKey); + assertSame(english, getProvider().getResourceBundle(Locale.ENGLISH)); + assertSame(french, getProvider().getResourceBundle(Locale.FRENCH)); + assertSame(named, getProvider().getResourceBundle(BASENAME, Locale.GERMAN)); + + deRoot.getNode("node_" + isolatedKey).remove(); + session.save(); + assertMessages(isolatedKey, isolatedKey, isolatedKey, isolatedKey); + assertSame(english, getProvider().getResourceBundle(Locale.ENGLISH)); + assertSame(french, getProvider().getResourceBundle(Locale.FRENCH)); + assertSame(named, getProvider().getResourceBundle(BASENAME, Locale.GERMAN)); + + // Re-add the entry using the supported message mixin instead of its primary type. + Node mixedMessage = deRoot.addNode("node_" + isolatedKey, "nt:unstructured"); + mixedMessage.addMixin("sling:Message"); + mixedMessage.setProperty("sling:key", isolatedKey); + mixedMessage.setProperty("sling:message", "DE_mixin"); + session.save(); + assertMessages(isolatedKey, "DE_mixin", "DE_mixin", isolatedKey); + assertSame(english, getProvider().getResourceBundle(Locale.ENGLISH)); + assertSame(french, getProvider().getResourceBundle(Locale.FRENCH)); + assertSame(named, getProvider().getResourceBundle(BASENAME, Locale.GERMAN)); + } + + /** + * Verifies JSON dictionary discovery, content updates, locale reassignment, and removal + * against real repository events. JCR reports nt:file content updates at the dictionary + * root, so these events deliberately retain full refreshes rather than targeted content-child refreshes. + */ + @Test + public void testJsonChangesDetection() throws RepositoryException { + final String key = "json-label"; + assertMessages(key, key, key, key); + Node dictionary = i18nRoot.addNode("de.json", "nt:file"); + dictionary.addMixin("mix:language"); + dictionary.setProperty("jcr:language", "de"); + Node content = dictionary.addNode("jcr:content", "nt:resource"); + content.setProperty("jcr:mimeType", "application/json"); + setJsonMessage(content, key, "Original"); + session.save(); + assertMessages(key, "Original", "Original", key); + + ResourceBundle english = getProvider().getResourceBundle(Locale.ENGLISH); + ResourceBundle french = getProvider().getResourceBundle(Locale.FRENCH); + ResourceBundle named = getProvider().getResourceBundle(BASENAME, Locale.GERMAN); + List<ResourceChange> observed = new CopyOnWriteArrayList<>(); + Dictionary<String, Object> properties = new Hashtable<>(); + final String dictionaryPath = dictionary.getPath(); + properties.put(ResourceChangeListener.PATHS, dictionaryPath); + ServiceRegistration<ResourceChangeListener> listener = + bundleContext.registerService(ResourceChangeListener.class, observed::addAll, properties); + try { + setJsonMessage(content, key, "Changed"); + session.save(); + assertMessages(key, "Changed", "Changed", key); + // The JCR provider rolls nt:file content updates up to the root, retaining full refreshes. + new Retry(RETRY_TIMEOUT_MSEC) { + @Override + protected void exec() { + assertTrue(observed.stream() + .anyMatch(change -> change.getType() == ResourceChange.ChangeType.CHANGED + && change.getPath().equals(dictionaryPath))); + } + }; + assertNotSame(english, getProvider().getResourceBundle(Locale.ENGLISH)); + assertNotSame(french, getProvider().getResourceBundle(Locale.FRENCH)); + assertNotSame(named, getProvider().getResourceBundle(BASENAME, Locale.GERMAN)); + } finally { + listener.unregister(); + } + + english = getProvider().getResourceBundle(Locale.ENGLISH); + french = getProvider().getResourceBundle(Locale.FRENCH); + assertMessage(key, Locale.ITALIAN, null, key); + dictionary.setProperty("jcr:language", "it"); + session.save(); + assertMessages(key, key, key, key); + assertMessage(key, Locale.ITALIAN, null, "Changed"); + assertNotSame(english, getProvider().getResourceBundle(Locale.ENGLISH)); + assertNotSame(french, getProvider().getResourceBundle(Locale.FRENCH)); + + english = getProvider().getResourceBundle(Locale.ENGLISH); + french = getProvider().getResourceBundle(Locale.FRENCH); + dictionary.remove(); + session.save(); + assertMessages(key, key, key, key); + assertMessage(key, Locale.ITALIAN, null, key); + assertNotSame(english, getProvider().getResourceBundle(Locale.ENGLISH)); + assertNotSame(french, getProvider().getResourceBundle(Locale.FRENCH)); + } + + private void setJsonMessage(Node content, String key, String message) throws RepositoryException { + byte[] json = ("{\"" + key + "\":\"" + message + "\"}").getBytes(StandardCharsets.UTF_8); + Binary binary = session.getValueFactory().createBinary(new ByteArrayInputStream(json)); + try { + content.setProperty("jcr:data", binary); + } finally { + binary.dispose(); + } } }
