Reputation: 399
In one Spring bean annoted with @Service we inject the interface IConfigurationService by using the @Autowired annotation. This interface is implemented by the ConfigurationService bean. According to the tool that controls our code's quality the ConfigurationService bean is not thread safe. This tool recommends us to define a constructor that initializes the values of two fields of ConfigurationService : "partConfigGacsiPath" and "partConfigMegPath". But when I look at our code there is already one constructor doing such a thing. What is lacking in our code to make the bean thread-safe?
package com.myCompany.canalnet.gacsi.wspl.service.configuration;
import java.util.MissingResourceException;
import java.util.ResourceBundle;
import javax.servlet.http.HttpServletRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
import com.myCompany.canalnet.wcm.exception.WCMException;
import com.myCompany.canalnet.wcm.mapper.IConfigXMLMapper;
import com.myCompany.canalnet.common.core.logging.CoreAlert;
import com.myCompany.canalnet.common.core.logging.CoreMarkers;
import com.myCompany.canalnet.common.core.logging.ErrorMessage;
import com.myCompany.canalnet.gacsi.common.core.ErrorCode;
import com.myCompany.canalnet.gacsi.wspl.service.configuration.bean.ConfigurationGacsi;
import com.myCompany.canalnet.gacsi.wspl.service.configuration.bean.ConfigurationMeg;
@Service
public class ConfigurationService implements IConfigurationService {
/**
* The logger
*/
private static final Logger LOGGER = LoggerFactory.getLogger(ConfigurationService.class);
/**
* Message if error when reading configuration file
*/
private static final String ERROR_CONF_XML = "Error when getting back XML configuration file";
/**
* Mapper of the configuration files
*/
@Autowired
private transient IConfigXMLMapper configXMLMapper;
private transient String partConfigGacsiPath;
private transient String partConfigMegPath;
/**
* Constructor with initialization of the config paths
*/
public ConfigurationService() {
initConfigPaths();
}
/**
* Initializes the paths to the conf_main.xml of the application
*/
private void initConfigPaths() {
try {
final ResourceBundle resource = ResourceBundle.getBundle("configuration");
partConfigGacsiPath = resource.getString("configuration.gacsi.publication.part");
partConfigMegPath = resource.getString("configuration.meg.publication.part");
} catch (final MissingResourceException e) {
CoreAlert.traceAlert("411","Bundle not found");
LOGGER.error(CoreMarkers.CONFIG, ErrorMessage.slf4jFormat("Bundle not found", ErrorCode.BUNDLE_NOT_FOUND.getCode()), e);
}
}
/**
* {@inheritDoc}
*/
@Override
public ConfigurationGacsi getConfigurationGacsi(final HttpServletRequest request) {
ConfigurationGacsi configurationGacsi = null;
try {
configurationGacsi = configXMLMapper.loadConfigXML(partConfigGacsiPath, ConfigurationGacsi.class);
} catch (final WCMException e) {
CoreAlert.traceAlert("415", ERROR_CONF_XML);
LOGGER.error(CoreMarkers.BUSINESS, ErrorMessage.slf4jFormat(ERROR_CONF_XML, ErrorCode.CONF_NOT_FOUND), e);
}
return configurationGacsi;
}
/**
* {@inheritDoc}
*/
@Override
public ConfigurationMeg getConfigurationMeg(final HttpServletRequest request) {
ConfigurationMeg configurationMeg = null;
try {
configurationMeg = configXMLMapper.loadConfigXML(partConfigMegPath, ConfigurationMeg.class);
} catch (final WCMException e) {
CoreAlert.traceAlert("415", ERROR_CONF_XML);
LOGGER.error(CoreMarkers.BUSINESS, ErrorMessage.slf4jFormat(ERROR_CONF_XML, ErrorCode.CONF_NOT_FOUND), e);
}
return configurationMeg;
}
public final void setConfigXMLMapper(final IConfigXMLMapper configXMLMapper) {
this.configXMLMapper = configXMLMapper;
}
}
Upvotes: 0
Views: 77
Reputation: 3324
What it is important is not to allow this
to "escape", which means don't let the instance be passed, to another kind of "process".
The reason is that your instance may not be completely initialized when the private gets it, which can lead to some inconsistencies.
But here at first glance it seems OK
..
Upvotes: 1