Skip to content

Commit

Permalink
Merge e65b6e1 into c430980
Browse files Browse the repository at this point in the history
  • Loading branch information
kwin committed Mar 3, 2021
2 parents c430980 + e65b6e1 commit 72af1df
Show file tree
Hide file tree
Showing 9 changed files with 161 additions and 77 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com)

### Fixed
- #2496 - Upgrade shaded Guava dependency to 30.1
- #2498 - Potential NPE in RunnableOnMaster
- #2492 - NPE in JcrPackageReplicationStatusEventHandler
- #2494 - Fixed issue with Versioned ClientLib incompatibility on 6.5.7

## 4.11.0 - 2020-12-11
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import com.adobe.acs.commons.packaging.PackageHelper;
import com.adobe.acs.commons.replication.status.ReplicationStatusManager;
import com.adobe.acs.commons.util.ClusterLeader;
import com.adobe.acs.commons.util.ParameterUtil;
import com.day.cq.replication.ReplicationAction;
import com.day.cq.replication.ReplicationEvent;
Expand Down Expand Up @@ -67,8 +68,6 @@
import java.util.Map;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.apache.sling.discovery.TopologyEvent;
import org.apache.sling.discovery.TopologyEventListener;

@Component(
label = "ACS AEM Commons - Package Replication Status Updater",
Expand Down Expand Up @@ -99,7 +98,7 @@
)
})
@Service
public class JcrPackageReplicationStatusEventHandler implements JobConsumer, EventHandler, TopologyEventListener {
public class JcrPackageReplicationStatusEventHandler implements JobConsumer, EventHandler {
private static final Logger log = LoggerFactory.getLogger(JcrPackageReplicationStatusEventHandler.class);

private static final String FALLBACK_REPLICATION_USER_ID = "Package Replication";
Expand Down Expand Up @@ -160,7 +159,8 @@ private enum ReplicatedAt {
@Reference
private JobManager jobManager;

private boolean isLeader = false;
@Reference
private ClusterLeader clusterLeader;

// Previously "Package Replication"
private static final String DEFAULT_REPLICATED_BY_OVERRIDE = "";
Expand Down Expand Up @@ -198,31 +198,28 @@ private enum ReplicatedAt {
@Override
@SuppressWarnings("squid:S3776")
public final void handleEvent(final Event event) {
if (this.isLeader) {
// Only run on master
final Map<String, Object> jobConfig = getInfoFromEvent(event);
final String[] paths = (String[]) jobConfig.get(PROPERTY_PATHS);
final Map<String, Object> jobConfig = getInfoFromEvent(event);
final String[] paths = (String[]) jobConfig.get(PROPERTY_PATHS);

try (ResourceResolver resourceResolver = resourceResolverFactory.getServiceResourceResolver(AUTH_INFO)) {

for (String path : paths) {
if (!this.containsJcrPackagePath(path)) {
continue;
}

final JcrPackage jcrPackage = this.getJcrPackage(resourceResolver, path);
if (jcrPackage != null) {
// Close jcrPackages after they've been used to check if a Job should be invoked.
jcrPackage.close();
jobConfig.put(PROPERTY_PATH, path);
// trigger one job per package to make one exception not affect other packages
jobManager.addJob(JOB_TOPIC, jobConfig);
}
try (ResourceResolver resourceResolver = resourceResolverFactory.getServiceResourceResolver(AUTH_INFO)) {

for (String path : paths) {
if (!this.containsJcrPackagePath(path)) {
continue;
}
} catch (LoginException e) {
log.error("Could not obtain a resource resolver.", e);
}
}

final JcrPackage jcrPackage = this.getJcrPackage(resourceResolver, path);
if (jcrPackage != null) {
// Close jcrPackages after they've been used to check if a Job should be invoked.
jcrPackage.close();
jobConfig.put(PROPERTY_PATH, path);
// trigger one job per package to make one exception not affect other packages
jobManager.addJob(JOB_TOPIC, jobConfig);
}
}
} catch (LoginException e) {
log.error("Could not obtain a resource resolver.", e);
}
}

@Override
Expand Down Expand Up @@ -497,9 +494,4 @@ protected void activate(final Map<String, String> config) throws LoginException
log.info("Package Replication Status - Replicated At: [ {} ]", this.replicatedAt);
log.info("Package Replication Status - Node Types and Path Restrictions: [ {} ]", pathRestrictionByNodeType);
}

@Override
public void handleTopologyEvent(TopologyEvent te) {
this.isLeader = te.getNewView().getLocalInstance().isLeader();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.adobe.acs.commons.util.RunnableOnMaster;
import com.adobe.acs.commons.util.ClusterLeader;

import java.util.Collections;
import java.util.Map;
Expand All @@ -44,7 +44,7 @@
@Property(name = "scheduler.expression", value = "0 0/15 * * * ?", label = "Refresh Interval",
description = "Twitter Feed Refresh interval (Quartz Cron Expression)"),
@Property(name = "scheduler.concurrent", boolValue = false, propertyPrivate = true) })
public final class TwitterFeedScheduler extends RunnableOnMaster {
public final class TwitterFeedScheduler implements Runnable {

private static final Logger log = LoggerFactory.getLogger(TwitterFeedScheduler.class);

Expand All @@ -61,8 +61,14 @@ public final class TwitterFeedScheduler extends RunnableOnMaster {
@Reference
private TwitterFeedUpdater twitterFeedService;

/**
* This service only becomes active if this instance is the cluster leader
*/
@Reference
private ClusterLeader clusterLeader;

@Override
public void runOnMaster() {
public void run() {

try (ResourceResolver resourceResolver = resourceResolverFactory.getServiceResourceResolver(AUTH_INFO) ){
log.debug("Master Instance, Running ACS AEM Commons Twitter Feed Scheduler");
Expand Down
33 changes: 33 additions & 0 deletions bundle/src/main/java/com/adobe/acs/commons/util/ClusterLeader.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* #%L
* ACS AEM Commons Bundle
* %%
* Copyright (C) 2021 Adobe
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
package com.adobe.acs.commons.util;

import org.apache.sling.discovery.DiscoveryService;
import org.apache.sling.discovery.InstanceDescription;

/**
* Marker interface.
* Available as OSGi service only on instances being the cluster leader.
* @see InstanceDescription#isLeader
* @see DiscoveryService#getTopology
*/
public interface ClusterLeader {

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@
import org.apache.sling.discovery.TopologyEventListener;

/**
* Abstact base class for scheduled job to be run only on the cluster master.
* Abstract base class for scheduled job to be run only on the cluster master.
* @deprecated Depend on marker interface {@link ClusterLeader} instead.
*/
@Deprecated
@ConsumerType
public abstract class RunnableOnMaster implements TopologyEventListener, Runnable {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
* #%L
* ACS AEM Commons Bundle
* %%
* Copyright (C) 2021 Adobe
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
package com.adobe.acs.commons.util.impl;

import org.apache.sling.discovery.DiscoveryService;
import org.apache.sling.discovery.TopologyEvent;
import org.apache.sling.discovery.TopologyEventListener;
import org.apache.sling.discovery.TopologyView;
import org.osgi.framework.BundleContext;
import org.osgi.framework.ServiceRegistration;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Deactivate;
import org.osgi.service.component.annotations.Reference;

import com.adobe.acs.commons.util.ClusterLeader;

/**
* This component registers a {@link ClusterLeader} service in case the current topology instance is the leader and
* unregisters that service if that is no longer the case.
*
* Components which are only supposed to be executed on the cluster leader can depend on the
* service with marker interface {@link ClusterLeader}.
*/
@Component
public class DiscoveryServiceHelper implements TopologyEventListener {

@Reference
DiscoveryService discoveryService;

private BundleContext bundleContext;

private ServiceRegistration<ClusterLeader> clusterLeaderServiceRegistration;

@Override
public void handleTopologyEvent(TopologyEvent event) {
TopologyView newView = event.getNewView();
if (newView != null) {
if (newView.getLocalInstance().isLeader()) {
registerClusterLeader();
} else {
unregisterClusterLeader();
}
}
}

@Activate
public void activate(BundleContext bundleContext) {
this.bundleContext = bundleContext;
if (discoveryService.getTopology().getLocalInstance().isLeader()) {
registerClusterLeader();
}
}

@Deactivate
public void deactivate() {
unregisterClusterLeader();
}

private synchronized void registerClusterLeader() {
if (clusterLeaderServiceRegistration == null) {
clusterLeaderServiceRegistration = bundleContext.registerService(ClusterLeader.class, new ClusterLeader() {}, null);
}
}

private synchronized void unregisterClusterLeader() {
if (clusterLeaderServiceRegistration != null) {
clusterLeaderServiceRegistration.unregister();
clusterLeaderServiceRegistration = null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
/**
* Miscellaneous Utilities.
*/
@Version("4.5.0")
@Version("4.6.0")

package com.adobe.acs.commons.util;
import org.osgi.annotation.versioning.Version;
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@
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.ValueMap;
import org.apache.sling.api.wrappers.ValueMapDecorator;
import org.apache.sling.event.jobs.Job;
import org.apache.sling.event.jobs.JobManager;
import org.junit.Assert;
Expand All @@ -56,9 +54,6 @@
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;
import org.apache.sling.discovery.InstanceDescription;
import org.apache.sling.discovery.TopologyEvent;
import org.apache.sling.discovery.TopologyView;

import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.anyMap;
Expand Down Expand Up @@ -194,13 +189,6 @@ public void testHandleEvent() throws LoginException {
final Event event = new Event(ReplicationAction.EVENT_TOPIC, eventParams);

final ArgumentCaptor<Map> captor = ArgumentCaptor.forClass(Map.class);
TopologyEvent te = mock(TopologyEvent.class);
TopologyView view = mock(TopologyView.class);
InstanceDescription instanceDescription = mock(InstanceDescription.class);
when(te.getNewView()).thenReturn(view);
when(view.getLocalInstance()).thenReturn(instanceDescription);
when(instanceDescription.isLeader()).thenReturn(true);
eventHandler.handleTopologyEvent(te);
eventHandler.handleEvent(event);

verify(jobManager, times(1)).addJob(eq("acs-commons/replication/package"), captor.capture());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@
*/
package com.adobe.acs.commons.twitter.impl;

import static org.junit.Assert.*;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.verify;
import junitx.util.PrivateAccessor;
Expand All @@ -34,10 +32,6 @@
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;

import org.apache.sling.discovery.InstanceDescription;
import org.apache.sling.discovery.TopologyEvent;
import org.apache.sling.discovery.TopologyView;

@RunWith(MockitoJUnitRunner.class)
public class TwitterFeedSchedulerTest {

Expand All @@ -64,42 +58,20 @@ public void setUp() throws Exception {

when(resourceResolverFactory.getServiceResourceResolver(anyMap())).thenReturn(
resourceResolver);

}

public TopologyEvent createLeaderChangeEvent(boolean isLeader) {
TopologyEvent te = mock(TopologyEvent.class);
TopologyView view = mock(TopologyView.class);
InstanceDescription instanceDescription = mock(InstanceDescription.class);
when(te.getNewView()).thenReturn(view);
when(view.getLocalInstance()).thenReturn(instanceDescription);
when(instanceDescription.isLeader()).thenReturn(isLeader);
return te;
}

@Test
public void testDefaultInstanceBehaviour() throws NoSuchFieldException {
boolean isLeader = (Boolean) PrivateAccessor.getField(scheduler,
"isLeader");
assertFalse(isLeader);
}

@Test
public void test_GivenItsMasterInstance_WhenRunIsInvoked_ThenCallsService()
throws Exception {
scheduler.handleTopologyEvent(createLeaderChangeEvent(true));
scheduler.run();
verify(twitterFeedService).updateTwitterFeedComponents(resourceResolver);

}

@Test
public void test_GivenItsMasterInstance_WhenRunIsInvoked_ThenFinallyResourceResolverGetsClosed()
throws Exception {
scheduler.handleTopologyEvent(createLeaderChangeEvent(true));
scheduler.run();
verify(resourceResolver).close();

}

}

0 comments on commit 72af1df

Please sign in to comment.