This is an automated email from the ASF dual-hosted git repository. joerghoh pushed a commit to branch SLING-13410 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-i18n.git
commit e7e8a6dbd4f18edb844d45d83faf6a9e11078b68 Author: Joerg Hoh <[email protected]> AuthorDate: Fri Nov 6 12:34:57 2026 +0100 SLING-13410 register new ResourceBundles first before unregistering the old ones --- README.md | 7 +- .../sling/i18n/impl/JcrResourceBundleProvider.java | 234 +++++++++--- .../i18n/impl/JcrResourceBundleProviderTest.java | 424 +++++++++++++++++++++ 3 files changed, 612 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index cb7440f..7876360 100644 --- a/README.md +++ b/README.md @@ -16,9 +16,10 @@ 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. +Observation respects the configured included and excluded paths. With preloading +enabled, a full refresh builds fresh bundles and fallback chains before publishing +replacements and retiring the old services. Failed construction retains the old +bundles. With preloading disabled, a full refresh clears the cache as before. 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` 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 7cb99e1..d00475f 100644 --- a/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java +++ b/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java @@ -28,6 +28,7 @@ import java.util.HashSet; import java.util.Hashtable; import java.util.IllformedLocaleException; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -132,7 +133,8 @@ public class JcrResourceBundleProvider /** * paths from which JCR resource bundles have been loaded */ - private final Set<String> languageRootPaths = Collections.newSetFromMap(new ConcurrentHashMap<String, Boolean>()); + private volatile Set<String> languageRootPaths = + Collections.newSetFromMap(new ConcurrentHashMap<String, Boolean>()); /** * Return root resource bundle as created on-demand by @@ -224,6 +226,7 @@ public class JcrResourceBundleProvider if (resourceBundleRegistry.isClosed()) { return; } + final long generation = resourceBundleRegistry.getGeneration(); final ChangeStatus status = new ChangeStatus(); try { for (final ResourceChange change : changes) { @@ -237,11 +240,15 @@ public class JcrResourceBundleProvider break; } } - if (status.reloadAll) { + if (status.reloadAll || !status.reloadBundles.isEmpty()) { + resourceBundleRegistry.recordChange(); + } + if (status.reloadAll + || (!status.reloadBundles.isEmpty() && resourceBundleRegistry.getGeneration() != generation)) { this.scheduleReloadBundles(true); } else { for (final JcrResourceBundle bundle : status.reloadBundles) { - this.scheduleReloadBundle(bundle); + this.scheduleReloadBundle(bundle, generation); } } } catch (final LoginException le) { @@ -380,14 +387,13 @@ public class JcrResourceBundleProvider @Override public void run() { log.info("Reloading all resource bundles"); - clearCache(); - preloadBundles(); + reloadAllBundles(); } }, options); } - private void scheduleReloadBundle(final JcrResourceBundle bundle) { + private void scheduleReloadBundle(final JcrResourceBundle bundle, final long generation) { final Key key = new Key(bundle.getBaseName(), bundle.getLocale()); // defer this job @@ -399,7 +405,7 @@ public class JcrResourceBundleProvider new Runnable() { @Override public void run() { - reloadBundle(key); + reloadBundle(key, generation); scheduledJobNames.remove(jobName); } }, @@ -407,9 +413,17 @@ public class JcrResourceBundleProvider } void reloadBundle(final Key key) { + reloadBundle(key, resourceBundleRegistry.getGeneration()); + } + + private void reloadBundle(final Key key, final long generation) { + if (resourceBundleRegistry.isClosed() || resourceBundleRegistry.getGeneration() != generation) { + log.debug("Skipping obsolete resource bundle reload for {}", key); + return; + } log.info("Reloading resource bundle for {}", key); if (!this.preloadBundles) { - resourceBundleRegistry.unregisterResourceBundle(key); + resourceBundleRegistry.unregisterResourceBundle(key, generation); } Collection<JcrResourceBundle> dependentBundles = new ArrayList<>(); @@ -427,11 +441,11 @@ public class JcrResourceBundleProvider } if (this.preloadBundles && !resourceBundleRegistry.isClosed()) { // reload the bundle from the repository (will also fill cache and register as a service) - getResourceBundleInternal(null, key.baseName, key.locale, true); + getResourceBundleInternal(null, key.baseName, key.locale, generation); } for (JcrResourceBundle dependentBundle : dependentBundles) { - reloadBundle(new Key(dependentBundle.getBaseName(), dependentBundle.getLocale())); + reloadBundle(new Key(dependentBundle.getBaseName(), dependentBundle.getLocale()), generation); } } @@ -487,11 +501,12 @@ public class JcrResourceBundleProvider */ private ResourceBundle getResourceBundleInternal( ResourceResolver optionalResolver, String baseName, Locale locale) { - return getResourceBundleInternal(optionalResolver, baseName, locale, false); + return getResourceBundleInternal(optionalResolver, baseName, locale, null); } private ResourceBundle getResourceBundleInternal( - ResourceResolver optionalResolver, final String baseName, Locale locale, final boolean forceReload) { + ResourceResolver optionalResolver, final String baseName, Locale locale, final Long reloadGeneration) { + final boolean forceReload = reloadGeneration != null; if (locale == null) { locale = defaultLocale; } @@ -512,31 +527,38 @@ public class JcrResourceBundleProvider log.debug("getResourceBundleInternal({}): got cache hit on second try", key); } else { log.debug("getResourceBundleInternal({}): reading from Repository", key); - ResourceResolver localResolver = null; - try { - if (optionalResolver == null) { - localResolver = createResourceResolver(); - optionalResolver = localResolver; + boolean published; + do { + final long generation = forceReload ? reloadGeneration : resourceBundleRegistry.getGeneration(); + if (forceReload && resourceBundleRegistry.getGeneration() != generation) { + return resourceBundleRegistry.getResourceBundle(key); } + ResourceResolver localResolver = null; + try { + if (optionalResolver == null) { + localResolver = createResourceResolver(); + } - resourceBundle = createResourceBundle(optionalResolver, key.baseName, key.locale); - resourceBundleRegistry.registerResourceBundle(key, resourceBundle); - - final Set<String> languageRoots = resourceBundle.getLanguageRootPaths(); - this.languageRootPaths.addAll(languageRoots); + resourceBundle = createResourceBundle( + localResolver == null ? optionalResolver : localResolver, key.baseName, key.locale); + published = resourceBundleRegistry.registerResourceBundle(key, resourceBundle, generation); + if (!published && forceReload) { + return resourceBundleRegistry.getResourceBundle(key); + } - log.debug("Key {} - added service registration and language roots {}", key, languageRoots); - logLanguageRootPaths(languageRootPaths); + log.debug("Key {} - loaded language roots {}", key, resourceBundle.getLanguageRootPaths()); + logLanguageRootPaths(languageRootPaths); - } catch (final LoginException le) { - throw (MissingResourceException) new MissingResourceException( - "Unable to create service resource resolver", baseName, locale.toString()) - .initCause(le); - } finally { - if (localResolver != null) { - localResolver.close(); + } catch (final LoginException le) { + throw (MissingResourceException) new MissingResourceException( + "Unable to create service resource resolver", baseName, locale.toString()) + .initCause(le); + } finally { + if (localResolver != null) { + localResolver.close(); + } } - } + } while (!published); } } catch (InterruptedException e) { Thread.interrupted(); @@ -664,16 +686,27 @@ public class JcrResourceBundleProvider } void clearCache() { - languageRootPaths.clear(); resourceBundleRegistry.unregisterAll(); } - private void preloadBundles() { - if (this.preloadBundles && !resourceBundleRegistry.isClosed()) { + /** + * Refreshes all bundles without clearing existing services upfront when preloading is enabled. + * Otherwise, invalidates the cache for on-demand loading. Does nothing after registry closure. + */ + void reloadAllBundles() { + if (resourceBundleRegistry.isClosed()) { + return; + } + if (!this.preloadBundles) { + clearCache(); + } else { + // Detect changes during staging so this refresh cannot overwrite newer work. + final long revision = resourceBundleRegistry.getRevision(); try (final ResourceResolver resolver = createResourceResolver()) { + // Build fresh parent chains separately; lookups continue using the current generation. + final Map<Key, JcrResourceBundle> replacements = new LinkedHashMap<>(); final Iterator<Map<String, Object>> bundles = resolver.queryResources(JcrResourceBundle.QUERY_LANGUAGE_ROOTS, "xpath"); - final Set<Key> usedKeys = new HashSet<>(); while (bundles.hasNext()) { final Map<String, Object> bundle = bundles.next(); if (bundle.containsKey(PROP_LANGUAGE) && bundle.containsKey(PROP_PATH)) { @@ -681,12 +714,14 @@ public class JcrResourceBundleProvider final String language = bundle.get(PROP_LANGUAGE).toString(); if (this.pathFilter.includePath(path)) { final Locale locale = toLocale(language); - final String baseName = bundle.containsKey(PROP_BASENAME) - ? bundle.get(PROP_BASENAME).toString() - : null; - final Key key = new Key(baseName, locale); - if (usedKeys.add(key)) { - getResourceBundleInternal(resolver, baseName, locale); + loadReplacement(resolver, new Key(null, locale), replacements); + final Object baseNames = bundle.get(PROP_BASENAME); + if (baseNames instanceof String[]) { + for (String baseName : (String[]) baseNames) { + loadReplacement(resolver, new Key(baseName, locale), replacements); + } + } else if (baseNames instanceof String) { + loadReplacement(resolver, new Key((String) baseNames, locale), replacements); } } else { log.warn( @@ -696,12 +731,36 @@ public class JcrResourceBundleProvider } } } + // Register replacements before retiring old services, including obsolete bundles. + // OSGi registration is not globally atomic; superseded staging is discarded and retried. + if (!resourceBundleRegistry.replaceAll(replacements, revision) && !resourceBundleRegistry.isClosed()) { + log.debug("Resource bundles changed during full reload; scheduling a fresh reload"); + scheduleReloadBundles(true); + } } catch (final LoginException le) { - log.error("Unable to create service user resource resolver.", le); + log.error("Unable to reload resource bundles; retaining the previous bundles.", le); + } catch (final RuntimeException e) { + log.error("Unable to complete full resource bundle reload.", e); + throw e; } } } + private JcrResourceBundle loadReplacement( + ResourceResolver resolver, Key key, Map<Key, JcrResourceBundle> replacements) { + JcrResourceBundle bundle = replacements.get(key); + if (bundle == null) { + final Locale parentLocale = getParentLocale(key.locale); + final ResourceBundle parent = parentLocale == null + ? getRootResourceBundle() + : loadReplacement(resolver, new Key(key.baseName, parentLocale), replacements); + bundle = new JcrResourceBundle(key.locale, key.baseName, resolver, locatorPaths, pathFilter); + bundle.setParent(parent); + replacements.put(key, bundle); + } + return bundle; + } + /** * Converts the given <code>localeString</code> to a valid * <code>java.util.Locale</code>. It must either be in the format specified by @@ -928,11 +987,25 @@ public class JcrResourceBundleProvider * It stores the references to the registered resource bundles and to the associated service registrations. * Publication and removal are coordinated with closure; OSGi registration callbacks run outside the registry monitor. */ - private static class ResourceBundleRegistry implements AutoCloseable { + private class ResourceBundleRegistry implements AutoCloseable { private final Logger log = LoggerFactory.getLogger(getClass()); private final BundleContext bundleContext; final AtomicBoolean closed = new AtomicBoolean(false); private final AtomicReference<ConcurrentHashMap<Key, Entry>> registrations; + /** + * Identifies the current cache generation. Advances when the entire registry is replaced or cleared, + * preventing older in-flight loads and queued targeted reloads from publishing into the new generation. + * Individual bundle updates do not advance this counter. + */ + private long generation; + + /** + * Tracks changes that can invalidate a staged full reload. Advances on bundle publication or removal, + * relevant observed repository changes, and full registry replacement or clearing. A full reload captures + * this revision before construction and may publish only if it is unchanged; otherwise, it is discarded + * and retried. Unlike {@link #generation}, this counter also advances for individual bundle updates. + */ + private long revision; private static class Entry { final JcrResourceBundle resourceBundle; @@ -960,22 +1033,38 @@ public class JcrResourceBundleProvider .collect(Collectors.toList()); } - void registerResourceBundle(Key key, JcrResourceBundle resourceBundle) { + synchronized long getGeneration() { + return generation; + } + + synchronized long getRevision() { + return revision; + } + + synchronized void recordChange() { + revision++; + } + + boolean registerResourceBundle(Key key, JcrResourceBundle resourceBundle, long expectedGeneration) { if (closed.get()) { - return; + return true; } ServiceRegistration<ResourceBundle> serviceReg = bundleContext.registerService(ResourceBundle.class, resourceBundle, serviceProps(key)); final Entry oldEntry; final boolean published; synchronized (this) { - published = !closed.get(); + published = !closed.get() && generation == expectedGeneration; oldEntry = published ? registrations.get().put(key, new Entry(resourceBundle, serviceReg)) : null; + if (published) { + languageRootPaths.addAll(resourceBundle.getLanguageRootPaths()); + revision++; + } } // Registration callbacks can close the registry before publication. if (!published) { serviceReg.unregister(); - return; + return closed.get(); } if (oldEntry != null) { oldEntry.serviceRegistration.unregister(); @@ -984,6 +1073,47 @@ public class JcrResourceBundleProvider "[ResourceBundleRegistry.updateResourceBundle] Registry updated - Nr of entries: {} - Keys: {}", registrations.get().size(), registrations.get().keySet()); + return true; + } + + boolean replaceAll(Map<Key, JcrResourceBundle> bundles, long expectedRevision) { + final ConcurrentHashMap<Key, Entry> replacements = new ConcurrentHashMap<>(); + final Set<String> roots = Collections.newSetFromMap(new ConcurrentHashMap<String, Boolean>()); + Map<Key, Entry> previous = null; + boolean published = false; + try { + for (Map.Entry<Key, JcrResourceBundle> bundle : bundles.entrySet()) { + if (closed.get() || getRevision() != expectedRevision) { + return false; + } + ServiceRegistration<ResourceBundle> registration = bundleContext.registerService( + ResourceBundle.class, bundle.getValue(), serviceProps(bundle.getKey())); + replacements.put(bundle.getKey(), new Entry(bundle.getValue(), registration)); + roots.addAll(bundle.getValue().getLanguageRootPaths()); + } + synchronized (this) { + if (!closed.get() && revision == expectedRevision) { + previous = registrations.getAndSet(replacements); + languageRootPaths = roots; + generation++; + revision++; + published = true; + } + } + } finally { + if (!published) { + for (Entry entry : replacements.values()) { + entry.serviceRegistration.unregister(); + } + } + } + if (published) { + for (Entry entry : previous.values()) { + entry.serviceRegistration.unregister(); + } + logLanguageRootPaths(roots); + } + return published; } private static Dictionary<String, Object> serviceProps(Key key) { @@ -995,13 +1125,14 @@ public class JcrResourceBundleProvider return serviceProps; } - void unregisterResourceBundle(Key key) { + void unregisterResourceBundle(Key key, long expectedGeneration) { final Entry oldEntry; synchronized (this) { - if (closed.get()) { + if (closed.get() || generation != expectedGeneration) { return; } oldEntry = registrations.get().remove(key); + revision++; } if (oldEntry != null) { oldEntry.serviceRegistration.unregister(); @@ -1030,6 +1161,9 @@ public class JcrResourceBundleProvider registrations.get().size(), registrations.get().keySet()); oldServiceReg = registrations.getAndSet(new ConcurrentHashMap<>()); + languageRootPaths = Collections.newSetFromMap(new ConcurrentHashMap<String, Boolean>()); + generation++; + revision++; } for (Entry entry : oldServiceReg.values()) { entry.serviceRegistration.unregister(); 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 6ba0a8a..2ecfeea 100644 --- a/src/test/java/org/apache/sling/i18n/impl/JcrResourceBundleProviderTest.java +++ b/src/test/java/org/apache/sling/i18n/impl/JcrResourceBundleProviderTest.java @@ -24,11 +24,21 @@ import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.ResourceBundle; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import org.apache.sling.api.resource.LoginException; @@ -49,12 +59,17 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.mockito.Mockito; import org.osgi.framework.Constants; +import org.osgi.framework.ServiceEvent; +import org.osgi.framework.ServiceListener; +import org.osgi.framework.ServiceReference; 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.assertThrows; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; import static org.mockito.ArgumentMatchers.any; /** @@ -254,6 +269,415 @@ public class JcrResourceBundleProviderTest { }); } + /** + * Pauses an eager full refresh during construction and verifies that cached lookups and old services + * remain available. After publication, checks fresh English fallback references and translations, + * registration-before-retirement ordering, and preservation of old bundle objects as snapshots. + */ + @Test + public void fullReloadKeepsOldServicesUntilFreshFallbacksAreReady() throws Exception { + assumeTrue(preload); + setupObservation(); + ResourceBundle english = provider.getResourceBundle(Locale.ENGLISH); + ResourceBundle german = provider.getResourceBundle(Locale.GERMAN); + Set<ServiceReference<ResourceBundle>> previousServices = bundleServices(); + context.resourceResolver() + .getResource("/apps/i18n/en/" + KEY) + .adaptTo(ModifiableValueMap.class) + .put("sling:message", "Updated English"); + CountDownLatch loading = new CountDownLatch(1); + CountDownLatch resume = new CountDownLatch(1); + pauseNextDictionaryLoad(loading, resume); + List<String> fallbacksDuringRetirement = new ArrayList<>(); + List<Integer> serviceEvents = new ArrayList<>(); + ServiceListener listener = event -> { + serviceEvents.add(event.getType()); + if (event.getType() == ServiceEvent.UNREGISTERING) { + fallbacksDuringRetirement.add( + provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } + }; + context.bundleContext().addServiceListener(listener, "(objectClass=java.util.ResourceBundle)"); + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future<?> reload = executor.submit(provider::reloadAllBundles); + assertTrue(loading.await(10, TimeUnit.SECONDS)); + assertSame(english, provider.getResourceBundle(Locale.ENGLISH)); + assertSame(german, provider.getResourceBundle(Locale.GERMAN)); + assertEquals("English", german.getString(KEY)); + assertEquals(previousServices, bundleServices()); + resume.countDown(); + reload.get(10, TimeUnit.SECONDS); + ResourceBundle replacement = provider.getResourceBundle(Locale.GERMAN); + assertNotSame(german, replacement); + assertEquals("Updated English", replacement.getString(KEY)); + assertSame(provider.getResourceBundle(Locale.ENGLISH), ((JcrResourceBundle) replacement).getParent()); + assertEquals("English", german.getString(KEY)); + assertFalse(fallbacksDuringRetirement.isEmpty()); + assertTrue(fallbacksDuringRetirement.stream().allMatch("Updated English"::equals)); + assertTrue(serviceEvents.lastIndexOf(ServiceEvent.REGISTERED) + < serviceEvents.indexOf(ServiceEvent.UNREGISTERING)); + for (ServiceReference<ResourceBundle> reference : previousServices) { + assertFalse(bundleServices().contains(reference)); + } + assertEquals( + "Updated English", + provider.getResourceBundle("checkout", Locale.GERMAN).getString(KEY)); + } finally { + resume.countDown(); + executor.shutdownNow(); + assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS)); + context.bundleContext().removeServiceListener(listener); + } + } + + /** + * Changes a dictionary's locale, removes a basename, and deletes another dictionary. + * A full refresh must retire obsolete services and forget the deleted language root, + * while making the new locale available with its English fallback. + */ + @Test + public void fullReloadRetiresRemovedOwnershipAndLanguageRoots() throws Exception { + assumeTrue(preload); + setupObservation(); + Resource german = context.resourceResolver().getResource("/apps/i18n/de"); + german.adaptTo(ModifiableValueMap.class).put("jcr:language", "it"); + context.resourceResolver() + .getResource("/apps/i18n/en") + .adaptTo(ModifiableValueMap.class) + .put("sling:basename", new String[] {"checkout"}); + context.resourceResolver().delete(context.resourceResolver().getResource("/apps/i18n/fr")); + + provider.reloadAllBundles(); + + assertTrue(context.bundleContext() + .getServiceReferences(ResourceBundle.class, "(locale=de)") + .isEmpty()); + assertTrue(context.bundleContext() + .getServiceReferences(ResourceBundle.class, "(locale=fr)") + .isEmpty()); + assertTrue(context.bundleContext() + .getServiceReferences(ResourceBundle.class, "(baseName=other)") + .isEmpty()); + assertEquals("English", provider.getResourceBundle(Locale.ITALIAN).getString(KEY)); + context.create().resource("/apps/i18n/fr"); + provider.onChange(Collections.singletonList(new ResourceChange(ChangeType.CHANGED, "/apps/i18n/fr", false))); + assertTrue("Deleted language roots must not remain in observation metadata", scheduledJobs.isEmpty()); + } + + /** + * Fails a repository query after replacement construction has begun. The exception must propagate + * without replacing the cached English bundle or changing any existing service registrations. + */ + @Test + public void failedFullReloadRetainsThePreviousGeneration() throws Exception { + assumeTrue(preload); + setupObservation(); + ResourceBundle english = provider.getResourceBundle(Locale.ENGLISH); + Set<ServiceReference<ResourceBundle>> previousServices = bundleServices(); + ResourceResolver resolver = factory.getServiceResourceResolver(null); + AtomicInteger queries = new AtomicInteger(); + Mockito.doAnswer(query -> { + if (queries.incrementAndGet() == 2) { + throw new IllegalStateException("Repository query failed"); + } + return dictionaries().iterator(); + }) + .when(resolver) + .findResources(JcrResourceBundle.QUERY_LANGUAGE_ROOTS, "xpath"); + + assertThrows(IllegalStateException.class, provider::reloadAllBundles); + + assertSame(english, provider.getResourceBundle(Locale.ENGLISH)); + assertEquals("English", english.getString(KEY)); + assertEquals(previousServices, bundleServices()); + } + + /** + * Denies the service resolver login for an eager full refresh. Existing cached bundles and their + * service registrations must remain intact despite the refresh being unable to start construction. + */ + @Test + public void failedFullReloadLoginRetainsThePreviousGeneration() throws Exception { + assumeTrue(preload); + setupObservation(); + ResourceBundle english = provider.getResourceBundle(Locale.ENGLISH); + Set<ServiceReference<ResourceBundle>> previousServices = bundleServices(); + Mockito.when(factory.getServiceResourceResolver(null)).thenThrow(new LoginException("Service login failed")); + + provider.reloadAllBundles(); + + assertSame(english, provider.getResourceBundle(Locale.ENGLISH)); + assertEquals(previousServices, bundleServices()); + } + + /** + * Publishes a targeted English update while an eager full refresh is paused during construction. + * The superseded full refresh must retain the newer services and schedule a retry that exposes + * the updated English translation through the German fallback chain. + */ + @Test + public void concurrentTargetedRefreshSupersedesStagedFullReload() throws Exception { + assumeTrue(preload); + setupObservation(); + CountDownLatch loading = new CountDownLatch(1); + CountDownLatch resume = new CountDownLatch(1); + pauseNextDictionaryLoad(loading, resume); + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future<?> reload = executor.submit(provider::reloadAllBundles); + assertTrue(loading.await(10, TimeUnit.SECONDS)); + context.resourceResolver() + .getResource("/apps/i18n/en/" + KEY) + .adaptTo(ModifiableValueMap.class) + .put("sling:message", "Concurrent update"); + provider.reloadBundle(new JcrResourceBundleProvider.Key(null, Locale.ENGLISH)); + ResourceBundle concurrent = provider.getResourceBundle(Locale.ENGLISH); + Set<ServiceReference<ResourceBundle>> currentServices = bundleServices(); + resume.countDown(); + reload.get(10, TimeUnit.SECONDS); + assertSame("Superseded reload must not publish", concurrent, provider.getResourceBundle(Locale.ENGLISH)); + assertEquals(currentServices, bundleServices()); + assertEquals(Collections.singleton(RELOAD_ALL), scheduledJobs.keySet()); + runScheduledWork(); + assertEquals( + "Concurrent update", + provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } finally { + resume.countDown(); + executor.shutdownNow(); + assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS)); + } + } + + /** + * Pauses an uncached Italian lookup inside its service registration callback, then changes the + * translation and completes a full refresh. The older lookup must retry against the new generation, + * returning the current translation and parent without leaving a duplicate Italian service. + */ + @Test + public void oldInFlightLookupCannotOverwriteTheFullReload() throws Exception { + assumeTrue(preload); + setupObservation(); + Resource italian = createDictionary("/apps/i18n/it", "it"); + Resource message = + createEntry(italian.getPath() + "/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "Old Italian"); + CountDownLatch registering = new CountDownLatch(1); + CountDownLatch resume = new CountDownLatch(1); + AtomicBoolean first = new AtomicBoolean(true); + ServiceListener listener = event -> { + if (event.getType() == ServiceEvent.REGISTERED && first.getAndSet(false)) { + registering.countDown(); + awaitReloadGate(resume); + } + }; + context.bundleContext().addServiceListener(listener, "(&(objectClass=java.util.ResourceBundle)(locale=it))"); + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future<ResourceBundle> lookup = executor.submit(() -> provider.getResourceBundle(Locale.ITALIAN)); + assertTrue(registering.await(10, TimeUnit.SECONDS)); + message.adaptTo(ModifiableValueMap.class).put("sling:message", "New Italian"); + provider.reloadAllBundles(); + resume.countDown(); + assertEquals("New Italian", lookup.get(10, TimeUnit.SECONDS).getString(KEY)); + ResourceBundle current = provider.getResourceBundle(Locale.ITALIAN); + assertEquals("New Italian", current.getString(KEY)); + assertSame(provider.getResourceBundle(Locale.ENGLISH), ((JcrResourceBundle) current).getParent()); + assertEquals( + 1, + context.bundleContext() + .getServiceReferences(ResourceBundle.class, "(locale=it)") + .size()); + } finally { + resume.countDown(); + executor.shutdownNow(); + assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS)); + context.bundleContext().removeServiceListener(listener); + } + } + + /** + * Deactivates the provider while the first staged replacement is inside its registration callback. + * Both the old services and the unpublished replacement must be unregistered exactly once, + * leaving no ResourceBundle services after the refresh resumes. + */ + @Test + public void shutdownDuringFullReloadRegistrationLeavesNoServices() throws Exception { + assumeTrue(preload); + setupObservation(); + int originalServices = bundleServices().size(); + CountDownLatch registering = new CountDownLatch(1); + CountDownLatch resume = new CountDownLatch(1); + Map<Long, Integer> unregistrations = new ConcurrentHashMap<>(); + ServiceListener listener = event -> { + if (event.getType() == ServiceEvent.REGISTERED) { + registering.countDown(); + awaitReloadGate(resume); + } else if (event.getType() == ServiceEvent.UNREGISTERING) { + Long id = (Long) event.getServiceReference().getProperty(Constants.SERVICE_ID); + unregistrations.merge(id, 1, Integer::sum); + } + }; + context.bundleContext().addServiceListener(listener, "(objectClass=java.util.ResourceBundle)"); + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future<?> reload = executor.submit(provider::reloadAllBundles); + assertTrue(registering.await(10, TimeUnit.SECONDS)); + provider.deactivate(); + resume.countDown(); + reload.get(10, TimeUnit.SECONDS); + assertTrue(bundleServices().isEmpty()); + assertEquals(originalServices + 1, unregistrations.size()); + unregistrations.values().forEach(count -> assertEquals(Integer.valueOf(1), count)); + } finally { + resume.countDown(); + executor.shutdownNow(); + assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS)); + context.bundleContext().removeServiceListener(listener); + } + } + + /** + * Queues a targeted German refresh, then changes the dictionary's locale to Italian and completes + * a full refresh first. Executing the older queued job must not recreate the retired German service. + */ + @Test + public void fullReloadSupersedesAlreadyScheduledTargetedJobs() throws Exception { + assumeTrue(preload); + setupObservation(); + Resource german = context.resourceResolver().getResource("/apps/i18n/de"); + Resource entry = createEntry(german.getPath() + "/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "German"); + provider.onChange(Collections.singletonList(new ResourceChange(ChangeType.CHANGED, entry.getPath(), false))); + assertFalse(scheduledJobs.isEmpty()); + german.adaptTo(ModifiableValueMap.class).put("jcr:language", "it"); + + provider.reloadAllBundles(); + runScheduledWork(); + + assertTrue(context.bundleContext() + .getServiceReferences(ResourceBundle.class, "(locale=de)") + .isEmpty()); + assertEquals("German", provider.getResourceBundle(Locale.ITALIAN).getString(KEY)); + } + + /** + * Pauses an executing targeted German refresh during registration, then moves its dictionary to + * Italian and completes a full refresh. Resuming the old refresh must neither publish its German + * replacement nor recreate dependent German country bundles. + */ + @Test + public void runningTargetedRefreshCannotResurrectARetiredLocale() throws Exception { + assumeTrue(preload); + setupObservation(); + Resource german = context.resourceResolver().getResource("/apps/i18n/de"); + createEntry(german.getPath() + "/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "Moved message"); + CountDownLatch registering = new CountDownLatch(1); + CountDownLatch resume = new CountDownLatch(1); + AtomicBoolean first = new AtomicBoolean(true); + ServiceListener listener = event -> { + if (event.getType() == ServiceEvent.REGISTERED && first.getAndSet(false)) { + registering.countDown(); + awaitReloadGate(resume); + } + }; + context.bundleContext().addServiceListener(listener, "(&(objectClass=java.util.ResourceBundle)(locale=de))"); + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future<?> targeted = executor.submit( + () -> provider.reloadBundle(new JcrResourceBundleProvider.Key(null, Locale.GERMAN))); + assertTrue(registering.await(10, TimeUnit.SECONDS)); + german.adaptTo(ModifiableValueMap.class).put("jcr:language", "it"); + provider.reloadAllBundles(); + resume.countDown(); + targeted.get(10, TimeUnit.SECONDS); + assertTrue(context.bundleContext() + .getServiceReferences(ResourceBundle.class, "(|(locale=de)(locale=de_DE))") + .isEmpty()); + assertEquals( + "Moved message", provider.getResourceBundle(Locale.ITALIAN).getString(KEY)); + } finally { + resume.countDown(); + executor.shutdownNow(); + assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS)); + context.bundleContext().removeServiceListener(listener); + } + } + + /** + * Observes a German entry update while full-refresh construction is paused, without executing the + * targeted job. Observation alone must invalidate staging, preserve the current services, and + * schedule a fresh full refresh that makes the updated translation available. + */ + @Test + public void observationDuringFullReloadRequiresAFreshGeneration() throws Exception { + assumeTrue(preload); + setupObservation(); + Set<ServiceReference<ResourceBundle>> previousServices = bundleServices(); + CountDownLatch loading = new CountDownLatch(1); + CountDownLatch resume = new CountDownLatch(1); + pauseNextDictionaryLoad(loading, resume); + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future<?> reload = executor.submit(provider::reloadAllBundles); + assertTrue(loading.await(10, TimeUnit.SECONDS)); + Resource entry = createEntry("/apps/i18n/de/" + KEY, JcrResourceBundle.RT_MESSAGE_ENTRY, "Observed update"); + provider.onChange( + Collections.singletonList(new ResourceChange(ChangeType.CHANGED, entry.getPath(), false))); + resume.countDown(); + reload.get(10, TimeUnit.SECONDS); + assertEquals("Aborted generation must not replace any service", previousServices, bundleServices()); + assertEquals(Collections.singleton(RELOAD_ALL), scheduledJobs.keySet()); + runScheduledWork(); + assertEquals( + "Observed update", provider.getResourceBundle(Locale.GERMAN).getString(KEY)); + } finally { + resume.countDown(); + executor.shutdownNow(); + assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS)); + } + } + + /** + * Runs a full refresh with preloading disabled. Services must be removed immediately, with a new + * bundle constructed only when a subsequent lookup requests it. + */ + @Test + public void fullReloadWithoutPreloadingRemainsLazy() throws Exception { + assumeTrue(!preload); + setupObservation(); + ResourceBundle old = provider.getResourceBundle(Locale.ENGLISH); + provider.reloadAllBundles(); + assertTrue(bundleServices().isEmpty()); + assertNotSame(old, provider.getResourceBundle(Locale.ENGLISH)); + } + + private Set<ServiceReference<ResourceBundle>> bundleServices() throws Exception { + return new HashSet<>(context.bundleContext().getServiceReferences(ResourceBundle.class, null)); + } + + private void pauseNextDictionaryLoad(CountDownLatch loading, CountDownLatch resume) throws Exception { + AtomicBoolean first = new AtomicBoolean(true); + ResourceResolver resolver = factory.getServiceResourceResolver(null); + Mockito.doAnswer(query -> { + if (first.getAndSet(false)) { + loading.countDown(); + awaitReloadGate(resume); + } + return dictionaries().iterator(); + }) + .when(resolver) + .findResources(JcrResourceBundle.QUERY_LANGUAGE_ROOTS, "xpath"); + } + + private static void awaitReloadGate(CountDownLatch latch) { + try { + assertTrue(latch.await(10, TimeUnit.SECONDS)); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError("Interrupted while waiting for full reload", e); + } + } + /** * 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.
