This Java class (aka application component) role is to handle a series of XML files stored in the file system. It must convert those XML files to the corresponding application domain model, and it needs to maintain them always synchronized with the XML representation. It must handle changes, deletion and (re)creation. And everything must be thread-safe.

A note: I didn't use a ConcurrentHashMap because the DomainChange event which I fire is synchronous and I need to wait for all the observers to finish.

I would like to have an opinion for my code, to understand if I got thread safety right, or where it is not needed / needed more. Unfortunately I have noone to ask face-to-face ;)

The code is pretty documented and self-explanatory but I had to translate JavaDoc and inline comments from Italian, so pardon errors, if any.

This component is pretty much CDI managed.
At costruction time I inject a Configuration (JavaDoc) object for later usage.

The core field is the managedDomains map. This map uses the unique domain name as key and the objects related to that domain (inside an instance of the holder class ManagedDomain) as value.

I removed a couple of "secret" constants in the code (you'll read REMOVED or ...).

I'm running this on a Java 7 environment.

/**
 * XML-file persisted domains controller.
 * <p>
 * This component publishes a {@link DomainChange} event every time a persisted
 * domain is changed, removed or (re)created. It is possible to listen for this
 * event, also by qualifier: see {@link Change}, {@link Creation} and {@link Removal}.
 *
 * @author Edoardo
 */
@Singleton
@ThreadSafe
class XmlFileDomainsPersistenceController
      extends FileAlterationListenerAdaptor
      implements DomainsPersistenceController {
   private static final Logger logger =
         LoggerFactory.getLogger(XmlFileDomainsPersistenceController.class);

   private static final String PREFIX_FOR_PERSISTENCE_KEYS = "REMOVED";
   private static final String SCHEMA_NAME = "schema.xsd";

   private final ReadWriteLock managedDomainsReadWriteLock = new ReentrantReadWriteLock(true);
   private final Lock writeLock = managedDomainsReadWriteLock.writeLock();
   private final Lock readLock = managedDomainsReadWriteLock.readLock();

   @GuardedBy("managedDomainsReadWriteLock")
   private final Map<String, ManagedDomain> managedDomains = new HashMap<>(16);

   @Nonnull private final Configuration configuration;
   @Nonnull private final JAXBContext jaxbContext;
   @Nonnull private final Schema schema;

   @Inject private Event<ApplicationError> applicationErrorEvent;
   @Inject private Event<DomainChange> domainChangeEvent;

   @Inject
   XmlFileDomainsPersistenceController(@Nonnull final Configuration configuration) throws Exception {
      this.configuration = configuration;
      jaxbContext = JAXBContext.newInstance(Domain.class);

      try (final InputStream schemaInputStream =
            Thread.currentThread().getContextClassLoader().getResourceAsStream(SCHEMA_NAME)) {
         final SchemaFactory schemaFactory =
               SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
         schema = schemaFactory.newSchema(new StreamSource(schemaInputStream));
      }
   }

   @Nonnull
   @Override
   public Optional<Domain> getDomain(@Nonnull final String domainName) {
      final ManagedDomain managedDomain;
      readLock.lock();

      try {
         managedDomain = managedDomains.get(domainName);
      } finally {
         readLock.unlock();
      }

      return managedDomain == null
            ? Optional.<Domain> absent()
            : Optional.of(managedDomain.domain);
   }

   /**
    * Tell this component to read and translate to the domain model {@link Domain} the
    * XML file containing the information for the domain which name is equal to the input one.
    *
    * @param domainName
    *       The unique name of the domain
    * @throws DomainPersistenceException
    *       In case the configured domain's file's path is invalid
    */
   @Nonnull
   @Contract("_ -> new")
   @Override
   public Domain addDomain(@Nonnull final String domainName) throws DomainPersistenceException {
      final ManagedDomain managedDomain;
      readLock.lock();

      try {
         managedDomain = managedDomains.get(domainName);
      } finally {
         readLock.unlock();
      }

      // If the domain is already managed by this component, it is an application error.
      if (managedDomain != null) {
         throw new IllegalArgumentException(
               "The domain " + domainName + " is already being managed");
      }

      // Build the prefix to access the domain configuration keys.
      final String configurationKeysPrefix = String.format(PREFIX_FOR_PERSISTENCE_KEYS, domainName);
      final String filePathConfigurationKey = configurationKeysPrefix + "REMOVED";
      final String filePath = configuration.getString(filePathConfigurationKey);

      // Verify the file's path.
      if (StringUtils.isEmpty(filePath)) {
         final String message =
               "Invalid path for XML file: '"
                     + filePath
                     + "'. Check configuration key "
                     + filePathConfigurationKey;

         throw buildAndPublishDomainPersistenceException(message);
      }

      final File file = getFilePathConsideringEnvironment(filePathConfigurationKey, filePath);
      final FileAlterationObserver fileAlterationObserver;
      final FileAlterationMonitor fileMonitor;

      try {
         // Build the objects which will listen for XML file's modifications.
         fileAlterationObserver =
               new FileAlterationObserver(
                     file.getParentFile(),
                     FileFilterUtils.nameFileFilter(file.getName()));
         fileAlterationObserver.addListener(this);

         final int frequency =
               configuration.getInt(configurationKeysPrefix + "REMOVED", 5);

         // Convert seconds to milliseconds.
         fileMonitor = new FileAlterationMonitor(frequency * 1000L);
         fileMonitor.addObserver(fileAlterationObserver);
      } catch (final Exception ignored) {
         final String message =
               "Invalid path for XML file: '"
                     + filePath
                     + "'. Check configuration key '"
                     + filePathConfigurationKey + "'";

         throw buildAndPublishDomainPersistenceException(message);
      }

      // Convert the XML file to the domain model.
      final Domain domain = unmarshalXmlToDomainModel(domainName, file);

      writeLock.lock();

      try {
         // Store everything in a map which uses the unique domain name as key.
         // I will need these objects for other operations.
         managedDomains.put(
               domainName,
               ManagedDomain.of(domain, fileAlterationObserver, fileMonitor));

         return domain;
      } finally {
         writeLock.unlock();
      }
   }

   /**
    * Remove and return the domain under our control.
    * If the domain is not managed a {@link com.google.common.base.Absent} is returned.
    *
    * @param domainName
    *       The unique name of the domain
    */
   @Nonnull
   @Override
   public Optional<Domain> removeDomain(@Nonnull final String domainName) {
      final ManagedDomain managedDomain;
      writeLock.lock();

      try {
         managedDomain = managedDomains.remove(domainName);

         if (managedDomain == null) {
            return Optional.absent();
         }

         final FileAlterationObserver fileAlterationObserver = managedDomain.fileAlterationObserver;
         fileAlterationObserver.removeListener(this);

         try {
            final FileAlterationMonitor fileAlterationMonitor = managedDomain.fileAlterationMonitor;
            fileAlterationMonitor.removeObserver(fileAlterationObserver);
            fileAlterationMonitor.stop(1);
         } catch (final Exception ignored) {
            // No-op.
         }

         domainChangeEvent.select(RemovalAnnotationLiteral.INSTANCE)
                          .fire(DomainChange.of(managedDomain.domain));
      } finally {
         writeLock.unlock();
      }

      // After returning managedDomains should be elegible for garbage collection.
      return Optional.of(managedDomain.domain);
   }

   /**
    * Update the domain under our control and persist changes to the XML file.
    *
    * @param domain
    *       The domain model to use for persisting changes
    * @throws DomainPersistenceException
    *       In case of errors during persistence operations
    */
   @Override
   public void updateDomain(@Nonnull final Domain domain) throws DomainPersistenceException {
      final String domainName = domain.getName();
      final ManagedDomain managedDomain;

      readLock.lock();

      try {
         managedDomain = managedDomains.get(domainName);
      } finally {
         readLock.unlock();
      }

      if (managedDomain == null) {
         throw new IllegalArgumentException("Cannot update the non-managed domain " + domainName);
      }

      // Having a listener for the XML file's state, doing a marshal operation would produce
      // an unneeded callback to onFileChange().
      // To avoid this, before marshalling I have to remove myself from the observer's listeners
      // and stop the scheduled monitor.
      // After marshalling is completed I need to create a new XML file's observer for having
      // "clean" file attributes, and to add myself as listener again.
      // In the end I have to start the scheduled monitor.

      // Remove myself from the listeners.
      final FileAlterationObserver observer = managedDomain.fileAlterationObserver;
      observer.removeListener(this);

      final FileAlterationMonitor fileAlterationMonitor = managedDomain.fileAlterationMonitor;

      // Save the monitor status prior to marshalling.
      final boolean wasMonitorRunning = getMonitorRunningStatus(fileAlterationMonitor);

      // Signal to the monitor to no more call the observer.
      fileAlterationMonitor.removeObserver(observer);

      final File file = observer.getDirectory();

      try {
         // Stop the scheduled monitor.
         fileAlterationMonitor.stop(1);

         final Marshaller marshaller = jaxbContext.createMarshaller();
         marshaller.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, true);
         marshaller.setSchema(schema);
         marshaller.marshal(domain, file);

         writeLock.lock();

         try {
            managedDomain.domain = domain;

            domainChangeEvent.select(ChangeAnnotationLiteral.INSTANCE)
                             .fire(DomainChange.of(domain));

            // Create the new observer with clean file's attributes.
            final FileAlterationObserver newFileAlterationObserver =
                  new FileAlterationObserver(
                        file.getParentFile(),
                        FileFilterUtils.nameFileFilter(file.getName()));

            // Add myself as a listener again.
            newFileAlterationObserver.addListener(this);

            // Signal to the monitor to call the observer again.
            fileAlterationMonitor.addObserver(newFileAlterationObserver);

            // Restore the monitor status.
            if (wasMonitorRunning) {
               fileAlterationMonitor.start();
            }

            // Update the reference of the stored observer with the new one.
            managedDomain.fileAlterationObserver = newFileAlterationObserver;
         } finally {
            writeLock.unlock();
         }
      } catch (final Exception e) {
         final String message =
               "Error writing domain " + domainName
                     + " to '" + file
                     + "' using schema " + SCHEMA_NAME;

         throw buildAndPublishDomainPersistenceException(e, message);
      }
   }

   /**
    * Return the running/stopped status of a FileAlterationMonitor via reflection, as
    * it is not exposed.
    *
    * @param monitor
    *       The monitor from which to extract the status
    */
   private boolean getMonitorRunningStatus(@Nonnull final FileAlterationMonitor monitor) {
      try {
         // noinspection JavaReflectionMemberAccess
         final Field running = monitor.getClass().getField("running");
         running.setAccessible(true);
         return running.getBoolean(monitor);
      } catch (final NoSuchFieldException | IllegalAccessException e) {
         throw new RuntimeException("Couldn't extract the monitor running status", e);
      }
   }

   @Override
   public void startObserving() {
      for (final ManagedDomain managedDomain : managedDomains.values()) {
         try {
            // noinspection SynchronizationOnLocalVariableOrMethodParameter
            synchronized (managedDomain) {
               managedDomain.fileAlterationMonitor.start();
            }
         } catch (final Exception ignored) {
            // No-op.
         }
      }
   }

   @Override
   public void startObserving(@Nonnull final String domainName) {
      final ManagedDomain managedDomain = managedDomains.get(domainName);

      if (managedDomain == null) {
         throw new IllegalArgumentException(
               "Cannot start observing the non-managed domain " + domainName);
      }

      try {
         // noinspection SynchronizationOnLocalVariableOrMethodParameter
         synchronized (managedDomain) {
            managedDomain.fileAlterationMonitor.start();
         }
      } catch (final Exception ignored) {
         // No-op.
      }
   }

   @Override
   public void stopObserving() {
      for (final ManagedDomain managedDomain : managedDomains.values()) {
         try {
            // noinspection SynchronizationOnLocalVariableOrMethodParameter
            synchronized (managedDomain) {
               managedDomain.fileAlterationMonitor.stop(1);
            }
         } catch (final Exception ignored) {
            // No-op.
         }
      }
   }

   @Override
   public void stopObserving(@Nonnull final String domainName) {
      final ManagedDomain managedDomain = managedDomains.get(domainName);

      if (managedDomain == null) {
         throw new IllegalArgumentException(
               "Cannot stop observing the non-managed domain " + domainName);
      }

      try {
         // noinspection SynchronizationOnLocalVariableOrMethodParameter
         synchronized (managedDomain) {
            managedDomain.fileAlterationMonitor.stop();
         }
      } catch (final Exception ignored) {
         // No-op.
      }
   }

   @Override
   public void onFileChange(final File file) {
      updateDomainModel(file, ChangeAnnotationLiteral.INSTANCE);
   }

   @Override
   public void onFileCreate(final File file) {
      updateDomainModel(file, CreationAnnotationLiteral.INSTANCE);
   }

   private <T extends Annotation> void updateDomainModel(
         final File file,
         final AnnotationLiteral<T> eventQualifier) {
      // At this point I expect the file I'm being notified for correspond to a domain
      // already contained in the managedDomains map. If not, it is an application error.
      final Domain domain;

      try {
         domain = unmarshalXmlToDomainModel(null, file);
      } catch (final DomainPersistenceException e) {
         applicationErrorEvent.fire(ApplicationError.of(e));
         return;
      }

      writeLock.lock();

      try {
         final ManagedDomain managedDomain = managedDomains.get(domain.getName());

         if (managedDomain == null) {
            throw new RuntimeException("The XML file does not match any of the configured domains");
         }

         // Overwrite the old domain model with the new one.
         managedDomain.domain = domain;
         domainChangeEvent.select(eventQualifier).fire(DomainChange.of(domain));
      } finally {
         writeLock.unlock();
      }
   }

   /**
    * Ritorna il path del file XML considerando il tipo di ambiente dove e' installato il prodotto.
    *
    * @param filePathConfigurationKey
    *       La chiave di configurazione per accedere al path del file XML
    * @param filePath
    *       Il path del file XML
    * @throws DomainPersistenceException
    *       In caso ci si trovi in ambiente ... ed il path del file XML non sia valido
    */
   @Nonnull
   @ParametersAreNonnullByDefault
   @Contract("_, _ -> new")
   private File getFilePathConsideringEnvironment(
         final String filePathConfigurationKey,
         final String filePath) throws DomainPersistenceException {
      // ... REMOVED
      return new File(filePath);
   }

   /**
    * Trasforma il file XML in domain model.
    *
    * @param domainName
    *       Il nome univoco del dominio da caricare in memoria.
    *       Passare {@code null} nel caso in cui non si voglia effettuare il confronto con il nome
    *       trovato nel file XML
    * @param file
    *       Il path del file XML
    * @throws DomainPersistenceException
    *       Se {@code domainName} e' diverso da {@code null} e non e' uguale al nome del dominio trovato
    *       nel file XML
    */
   @Nonnull
   @Contract("_, _ -> new")
   private Domain unmarshalXmlToDomainModel(
         @Nullable final String domainName,
         @Nonnull final File file) throws DomainPersistenceException {
      try {
         final Unmarshaller unmarshaller = jaxbContext.createUnmarshaller();

         // Set the XSD schema for unmarshalling validation.
         unmarshaller.setSchema(schema);

         final Domain domain = (Domain) unmarshaller.unmarshal(file);

         // Check that the configured domain name is equal to the domain name
         // stored in the XML file. If not, it is an application error.
         if (domainName != null && !domainName.equals(domain.getName())) {
            final String message =
                  "Domain name '" + domain.getName()
                        + "' in file " + file
                        + " does not match domain name '" + domainName
                        + "' declared in the configuration.";

            throw buildAndPublishDomainPersistenceException(message);
         }

         return domain;
      } catch (final UnmarshalException e) {
         final String message =
               "Error parsing XML content from '" + file + "' using schema " + SCHEMA_NAME;
         throw buildAndPublishDomainPersistenceException(e, message);
      } catch (final JAXBException e) {
         throw buildAndPublishDomainPersistenceException(e, "Error loading domain from " + file);
      }
   }

   @Contract("_ -> new")
   private DomainPersistenceException buildAndPublishDomainPersistenceException(
         @Nonnull final String message) {
      return buildAndPublishDomainPersistenceException(null, message);
   }

   @Contract("_, _-> new")
   private DomainPersistenceException buildAndPublishDomainPersistenceException(
         @Nullable final Throwable cause,
         @Nonnull final String message) {
      // noinspection ConstantConditions
      final DomainPersistenceException e = new DomainPersistenceException(message, cause);
      applicationErrorEvent.fire(ApplicationError.of(e));
      return e;
   }

   /**
    * Contenitore degli oggetti necessari alla gestione di un dominio.
    * Non utilizzare per altri scopi.
    */
   private static class ManagedDomain {
      @Nonnull Domain domain;
      @Nonnull FileAlterationObserver fileAlterationObserver;
      @Nonnull final FileAlterationMonitor fileAlterationMonitor;

      @ParametersAreNonnullByDefault
      private ManagedDomain(
            final Domain domain,
            final FileAlterationObserver fileAlterationObserver,
            final FileAlterationMonitor fileAlterationMonitor) {
         this.domain = domain;
         this.fileAlterationObserver = fileAlterationObserver;
         this.fileAlterationMonitor = fileAlterationMonitor;
      }

      @ParametersAreNonnullByDefault
      static ManagedDomain of(
            final Domain domain,
            final FileAlterationObserver fileAlterationObserver,
            final FileAlterationMonitor fileAlterationMonitor) {
         return new ManagedDomain(domain, fileAlterationObserver, fileAlterationMonitor);
      }
   }
}
  • Maybe using tryLock() is better. That's the one I can see doable. – LppEdd Oct 7 at 15:01

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Browse other questions tagged or ask your own question.