Trigger AEM Event listener : Declaring resolver object globally to be available inside activate and onevent method to be thread safe | Community
Skip to main content
P
Adobe Champion
P_V_NairAdobe Champion
Adobe Champion
April 9, 2022
Solved

Trigger AEM Event listener : Declaring resolver object globally to be available inside activate and onevent method to be thread safe

  • April 9, 2022
  • 1 reply
  • 7205 views

I have a event listener class to call a service method as below. This piece of code works well and calls the service method when the event is triggered. But,when this code went through the automatic code scanning tool sonar cube in Adobe cloud, we got a critical finding on the resolver object declaration as highlighted in bold red below. Below was the critical finding - 

Usage of org.apache.sling.api.resource.ResourceResolver as a field is not thread safe.

 

So I removed the global declaration of resolver object and tried to create the object inside the try method as below

try (ResourceResolver resourceResolver = resolverFactory.getServiceResourceResolver(ServiceUser.REVIEW_TASK_SERVICE_USER.getAuthMap())) {

 

This will automatically handle the resolver.close() as well. But the issue is, in this approach, we cannot declare the resolver object globally, which will be available both in activate and onevent methods. This declaration of resolver object inside try method will not trigger the listener and my listener is no longer getting triggered.

 

 

 

package com.inc.aeminc.core.listeners;

import javax.jcr.RepositoryException;
import javax.jcr.Session;
import javax.jcr.observation.Event;
import javax.jcr.observation.EventIterator;
import javax.jcr.observation.EventListener;
import javax.jcr.observation.ObservationManager;

import org.apache.sling.api.resource.LoginException;
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.api.resource.ResourceResolverFactory;
import org.apache.sling.jcr.api.SlingRepository;
import org.osgi.service.component.ComponentContext;
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 org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.inc.aeminc.core.services.CodeGeneratorService;
import com.inc.aeminc.core.sling.ServiceUser;

@8220494(service = EventListener.class, immediate = true)
public class ModelFragmentListener implements EventListener {

/**
* Logger
*/
private static final Logger log = LoggerFactory.getLogger(ModelFragmentListener.class);

/**
* Resource Resolver Factory
*/
@3214626
private ResourceResolverFactory resolverFactory;

@3214626
private SlingRepository repository;

@3214626
private CodeGeneratorService CodeGenerator;

private ObservationManager observationManager;

private ResourceResolver resourceResolver;

@580286
protected void activate(ComponentContext componentContext) {

log.info("Activating the observation");

try {
resourceResolver = resolverFactory.getServiceResourceResolver(ServiceUser.REVIEW_TASK_SERVICE_USER.getAuthMap());

/**
* Adapting the resource resolver to session object
*/
Session session = resourceResolver.adaptTo(Session.class);
if (null != session) {
observationManager = session.getWorkspace().getObservationManager();
}

log.info("Session created for ModelFragmentListener");

/**
* Adding the event listener
*/
observationManager.addEventListener(this,
Event.PROPERTY_ADDED | Event.PROPERTY_CHANGED, "/content/dam/testpath",
true, null, null, false);
} catch (RepositoryException | LoginException e) {
log.error("An exception Occured in code Generator ", e);
}
}

@3038739
protected void deactivate() throws RepositoryException {

if (observationManager != null) {
observationManager.removeEventListener(this);
}
}

@9944223
public void onEvent(EventIterator events) {
try {

while (events.hasNext()) {

final Event event = events.nextEvent();
String path = "";
String property = "";
try {
switch (event.getType()) {
case Event.PROPERTY_ADDED:
path = event.getPath();
property = path.substring(path.lastIndexOf("/") + 1);
if ((property.equals("lastFragmentSave")) && ((path.startsWith("/content/dam/testpath/creative-assets")) ||
(path.startsWith("/content/dam/testpath/team-assets")))) {
log.info("Property added: {} ", event.getPath());
path = path.substring(0, path.lastIndexOf("/"));
CodeGenerator.createCode(resourceResolver, path);
}

break;
case Event.PROPERTY_CHANGED:
path = event.getPath();
property = path.substring(path.lastIndexOf("/") + 1);
if ((property.equals("lastFragmentSave")) && ((path.startsWith("/content/dam/testpath/creative-assets")) ||
(path.startsWith("/content/dam/testpath/team-assets")))) {
log.info("Property added: {} ", event.getPath());
path = path.substring(0, path.lastIndexOf("/"));
CodeGenerator.createCode(resourceResolver, path);

}

break;
default:
log.info("Property noaction: {} ", event.getPath());
}

} catch (RepositoryException e) {
log.error("RepositoryException during template change listener ", e);
}

}

} catch (Exception e) {

log.error("Exception occurred", e);
}
}

}

 

This should be a common issue for any listener class in AEM. How do we declare the resolver object globally and make sure the same resolver object is available inside activate() and onevent() inside a listener class, also making sure it is to be thread safe?

Any leads would be much appreciated.

Best answer by lukasz-m

@lukasz-m  Do you think there is a way here not to use session object as well as a field?


@p_v_nair, I do not think this is possible. Nevertheless you can try below options:

  1. Use SuppressWarnings annotation against session field on class level
    @SuppressWarnings("AEM Rules:AEM-3")
private Session session;
    What is interesting, this approach is applied on one of the class from ACS Commons - here is direct link. As you can see this is exactly the same scenario/case that you have. So this looks to be proper way to go.
  2. You can create and use session object only on activate method level, but in that case you will need to close the session in the same method. I am afraid in that case your event listener will not be registered properly. You could of course not close the session - but this is nasty option I would not even consider.

1 reply

lukasz-m
Community Advisor
lukasz-mCommunity Advisor
Community Advisor
April 10, 2022

Hi @p_v_nair,

In general in this specific case you do not need ResourceResolver at all. Looking into implementation, you are using ResourceResolver for 2 things:

  1. To get jcr session.
  2. To pass ResourceResolver object to other OSGi service.

You are not using any specific method from ResourceResolver on ModelFragmentListener class level. By specific method I mean, e.g. getResource etc.

I would suggest below changes to improve your implementation:

  1. Use SlingRepository to get jcr session, this can be done like that:
    // additional field on class level
private Session session;

@Activate
protected void activate() {
    log.info("Activating the observation");
    try {
        session = createSession();
        if (session != null) {
            observationManager = session.getWorkspace().getObservationManager();
            log.info("Session created for ModelFragmentListener");

            /**
            * Adding the event listener
            */
            observationManager.addEventListener(this,
                Event.PROPERTY_ADDED | Event.PROPERTY_CHANGED, "/content/dam/testpath",
                true, null, null, false);
        }
    } catch (RepositoryException | LoginException e) {
        log.error("An exception Occured in code Generator ", e);
    }
}

@Deactivate
protected void deactivate() throws RepositoryException {
    if (observationManager != null) {
        observationManager.removeEventListener(this);
    }
    if (session != null) {
        session.logout();
    }
 }

// additional method dedicated to create jcr session
private Session createSession() {
    Session result = null;
    try {
	String workspace = repository.getDefaultWorkspace();
        /**
        * probably you can get subServiceName easier - but I do not know how ServiceUser 
        * class is implemented in your code
        */
        String subServiceName = ServiceUser
            .REVIEW_TASK_SERVICE_USER
            .getAuthMap()
            .get("sling.service.subservice").toString();
    
	result = repository.loginService(subServiceName, workspace);
    } catch (RepositoryException e) {
        log.error("Unable to create session", e);
    }
    return result;
}
    As you can see, above changes will give you full control on jcr session in activate and deactivate method. You do not need ResourceResolver to achieve that.
  2. You should avoid implementation where you are passing ResourceResolver object into other class. While doing it, you can easily loose control related to ResourceResolver lifecycle. For instance, you can close it in one of the class you are passing it to. In given scenario I would recommend to refactor method createCode from your CodeGeneratorService. Proposed change:
    // current version
createCode(ResourceResolver resourceResolver, String path);

// alternative options
createCode(ResourceResolverFactory resourceResolverFacrory, String path);

// proper way
createCode(String path);
    Managing dedicated ResourceResolver on CodeGeneratorService class level is the right way. This will be safe and will guarantee you have full control when you are creating and closing your ResourceResolver object.
    Alternatively you can pass ResourceResolverFactory instead of ResourceResolver, but the idea is still the same.

To sum up. It is a good practice to create ResourceResolver in class when it is really needed, so when you have to use method from API its exposing. In all other cases use different options. Design your code in the way that it is not needed to pass ResourceResolver object outside specific class.

    P
    Adobe Champion
    P_V_NairAdobe ChampionAuthor
    Adobe Champion
    April 11, 2022

    @lukasz-m  Thanks a lot for the above suggestions and explanation.

     

    private Session session

      But in my code I was already using session object as a private variable as the same in your above suggestion and Adobe Sonar rules flagged this as below critical finding

     Usage of javax.jcr.Session as a field is not thread safe.,Bug,Critical

     

    Hence, I have modified my code as in my initial description to fix that issue so that session object is not created as a variable and I created it inside the method i need it.

    As you see in my code, session object finding was fixed by that approach and the only one critical left was resolver object.

    Do you think we can fix this by not declaring both session object and  resolver object as variables?

      P
      Adobe Champion
      P_V_NairAdobe ChampionAuthor
      Adobe Champion
      April 11, 2022

      @lukasz-m  Do you think there is a way here not to use session object as well as a field?

        Terms & ConditionsAccessibility statement

        Loading footer...