diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-api/src/main/java/org/xwiki/job/internal/JobUtils.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-api/src/main/java/org/xwiki/job/internal/JobUtils.java index f0463994e5..d914bc5ef7 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-api/src/main/java/org/xwiki/job/internal/JobUtils.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-api/src/main/java/org/xwiki/job/internal/JobUtils.java @@ -41,7 +41,8 @@ private JobUtils() */ public static boolean isSerializable(JobStatus status) { - if (status.getRequest().getId() == null || !status.isSerialized()) { + if (status == null || status.getRequest() == null || status.getRequest().getId() == null + || !status.isSerialized()) { return false; } diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/AbstractJobStatusFolderResolver.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/AbstractJobStatusFolderResolver.java index 84bb536cc4..c72f8e0672 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/AbstractJobStatusFolderResolver.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/AbstractJobStatusFolderResolver.java @@ -22,6 +22,7 @@ import java.io.File; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.List; import javax.inject.Inject; @@ -51,18 +52,29 @@ public abstract class AbstractJobStatusFolderResolver implements JobStatusFolder @Override public File getFolder(List id) { - File folder = getBaseFolder(); + File folder = this.configuration.getStorage(); - if (id != null) { - // Create a different folder for each element - for (String fullIdElement : id) { - folder = addIDElement(fullIdElement, folder); - } + for (String pathSegment : getFolderSegments(id)) { + folder = new File(folder, pathSegment); } return folder; } + @Override + public List getFolderSegments(List jobID) + { + List result = new ArrayList<>(getBaseFolderSegments()); + + if (jobID != null) { + for (String idElement : jobID) { + result.addAll(encodeAndSplit(idElement)); + } + } + + return result; + } + protected String nullAwareURLEncode(String value) { String encoded; @@ -76,10 +88,17 @@ protected String nullAwareURLEncode(String value) return encoded; } - protected File getBaseFolder() + /** + * @return the base folder segments for the job status folder, override to provide a custom base folder + */ + protected List getBaseFolderSegments() { - return this.configuration.getStorage(); + return List.of(); } - protected abstract File addIDElement(String idElement, File folder); + /** + * @param idElement the id element to encode and split + * @return the encoded and split id element + */ + protected abstract List encodeAndSplit(String idElement); } diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/DefaultJobStatusStorage.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/DefaultJobStatusStorage.java index 1ddec4c7a2..46b0e83187 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/DefaultJobStatusStorage.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/DefaultJobStatusStorage.java @@ -34,7 +34,7 @@ * * @version $Id$ * @since 4.0M1 - * @deprecated since 6.1M2, use {@link org.xwiki.job.internal.DefaultJobStatusStore} instead + * @deprecated since 6.1M2, use {@link DefaultJobStatusStore} instead */ @Component @Singleton diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/DefaultJobStatusStore.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/DefaultJobStatusStore.java index 00a290eb1e..6ab755ca44 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/DefaultJobStatusStore.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/DefaultJobStatusStore.java @@ -19,30 +19,23 @@ */ package org.xwiki.job.internal; -import java.io.File; import java.io.IOException; -import java.nio.file.DirectoryStream; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.List; -import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.ReentrantReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock; -import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock; +import java.util.concurrent.locks.Lock; import javax.inject.Inject; -import javax.inject.Singleton; -import org.apache.commons.configuration2.PropertiesConfiguration; -import org.apache.commons.configuration2.builder.FileBasedConfigurationBuilder; -import org.apache.commons.configuration2.builder.fluent.Parameters; -import org.apache.commons.io.FileUtils; +import jakarta.inject.Singleton; + import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.concurrent.BasicThreadFactory; +import org.jspecify.annotations.NonNull; import org.slf4j.Logger; import org.xwiki.cache.Cache; import org.xwiki.cache.CacheException; @@ -51,89 +44,50 @@ import org.xwiki.component.annotation.Component; import org.xwiki.component.phase.Initializable; import org.xwiki.component.phase.InitializationException; -import org.xwiki.job.AbstractJobStatus; import org.xwiki.job.DefaultJobStatus; import org.xwiki.job.JobManagerConfiguration; import org.xwiki.job.JobStatusStore; import org.xwiki.job.event.status.JobStatus; -import org.xwiki.logging.LogQueue; -import org.xwiki.logging.LoggerManager; import org.xwiki.logging.tail.LoggerTail; +import com.google.common.util.concurrent.Striped; + /** - * Default implementation of {@link JobStatusStorage}. + * Default implementation of {@link JobStatusStore} that handles caching and locking. The actual storage is delegated to + * {@link PersistentJobStatusStore}. * * @version $Id$ * @since 6.1M2 */ -// The job status store is too big and should be refactored. -@SuppressWarnings("checkstyle:ClassFanOutComplexity") @Component @Singleton public class DefaultJobStatusStore implements JobStatusStore, Initializable { - /** - * The current version of the store. Should be upgraded if any change is made. - */ - private static final int VERSION = 1; - - /** - * The name of the file where the job status is stored as XML. - */ - private static final String FILENAME_STATUS_XML = "status.xml"; - - /** - * The name of the file where the job status is ZIPPED. - */ - private static final String FILENAME_STATUS_ZIP = FILENAME_STATUS_XML + ".zip"; - - /** - * The name of the file where various information about the status store are stored (like the version of the store). - */ - private static final String INDEX_FILE = "store.properties"; - - /** - * The name of the property containing the version of the store. - */ - private static final String INDEX_FILE_VERSION = "version"; + private static final JobStatus NOSTATUS = new DefaultJobStatus<>(null, null, null, null, null); - private static final String STATUS_LOG_PREFIX = "log"; + @Inject + private Logger logger; - private static final JobStatus NOSTATUS = new DefaultJobStatus<>(null, null, null, null, null); + private ExecutorService executorService; - /** - * Used to get the storage directory. - */ @Inject private JobManagerConfiguration configuration; @Inject - private CacheManager cacheManager; + private PersistentJobStatusStore persistentJobStatusStore; @Inject - private LoggerManager loggerManager; + private CacheManager cacheManager; - @Inject - private JobStatusSerializer serializer; + private Cache cache; - @Inject - private List folderResolvers; + private final Striped locks = Striped.lock(16); /** - * The logger to log. + * Tracks in-flight writes (both sync and async) to prevent stale reads from disk during the window between cache + * eviction and save completion. */ - @Inject - private Logger logger; - - private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); - - private final ReadLock readLock = lock.readLock(); - - private final WriteLock writeLock = lock.writeLock(); - - private ExecutorService executorService; - - private Cache cache; + private final ConcurrentMap pendingWrites = new ConcurrentHashMap<>(); class JobStatusSerializerRunnable implements Runnable { @@ -154,30 +108,10 @@ public void run() } } + @Override public void initialize() throws InitializationException { - try { - // Check if the store need to be upgraded - File folder = this.configuration.getStorage(); - File file = new File(folder, INDEX_FILE); - - FileBasedConfigurationBuilder builder = - new FileBasedConfigurationBuilder<>(PropertiesConfiguration.class, null, true) - .configure(new Parameters().properties().setFile(file)); - PropertiesConfiguration properties = builder.getConfiguration(); - int version = properties.getInt(INDEX_FILE_VERSION, 0); - if (VERSION > version) { - repair(); - - // Update version - properties.setProperty(INDEX_FILE_VERSION, VERSION); - builder.save(); - } - } catch (Exception e) { - this.logger.error("Failed to load jobs", e); - } - BasicThreadFactory threadFactory = new BasicThreadFactory.Builder().namingPattern("Job status serializer") .daemon(true).priority(Thread.MIN_PRIORITY).build(); this.executorService = @@ -193,291 +127,116 @@ public void initialize() throws InitializationException } } - private String toUniqueString(List id) - { - return StringUtils.join(id, '/'); - } - - /** - * Load jobs from directory. - * - * @throws IOException when failing to load statuses - */ - private void repair() throws IOException + @Override + public JobStatus getJobStatus(List id) { - File folder = this.configuration.getStorage(); - - if (folder.exists()) { - if (!folder.isDirectory()) { - throw new IOException("Not a directory: " + folder); - } + String idString = toUniqueString(id); - repairFolder(folder); - } - } + JobStatus status = this.cache.get(idString); - /** - * @param folder the folder from where to load the jobs - */ - private void repairFolder(File folder) - { - for (File file : folder.listFiles()) { - if (file.isDirectory()) { - repairFolder(file); - } else if (file.getName().equals(FILENAME_STATUS_ZIP) || file.getName().equals(FILENAME_STATUS_XML)) { - try { - JobStatus status = loadStatus(folder); + if (status == null) { + Lock lock = this.locks.get(idString); + lock.lock(); + try { + // Check again to avoid reading the same status twice. All reading happens inside the lock. + status = this.cache.get(idString); + if (status == null) { + // Check pending writes before loading from disk to avoid reading stale data during the + // window between cache eviction and save completion. + status = this.pendingWrites.get(idString); if (status != null) { - File properFolder = getAndMoveJobFolder(status.getRequest().getId(), false); - - if (!folder.equals(properFolder)) { - moveJobStatus(folder, file, properFolder); - } - } - } catch (Exception e) { - this.logger.warn("Failed to load job status in folder [{}]", folder, e); - } - } - } - } - - private void moveJobStatus(File sourceDirectory, File statusFile, File targetDirectory) - { - try { - // Check if the target already exists. - File targetStatusZip = new File(targetDirectory, FILENAME_STATUS_ZIP); - File targetStatus = new File(targetDirectory, FILENAME_STATUS_XML); - - // Compare the last modified times of the source and the target status file. - long sourceLastModified = statusFile.lastModified(); - // The target last modified time will be 0 if the target status file does not exist. - long targetLastModified = Math.max(targetStatusZip.lastModified(), targetStatus.lastModified()); - - if (sourceLastModified > targetLastModified) { - // Source is newer (or target doesn't exist), overwrite the target. - if (targetDirectory.isDirectory()) { - deleteJobStatusFiles(targetDirectory); - } - - // Move the status in its right place - FileUtils.moveFileToDirectory(statusFile, targetDirectory, true); - // Move the job log, too. - for (File file : sourceDirectory.listFiles()) { - if (!file.isDirectory() && file.getName().startsWith(STATUS_LOG_PREFIX)) { - FileUtils.moveFileToDirectory(file, targetDirectory, true); - } - } - } else { - // Target is more recent, remove the source. - deleteJobStatusFiles(sourceDirectory); - } - - // Remove the source directory and its parents if they are empty now. - cleanEmptyDirectories(sourceDirectory); - } catch (IOException e) { - this.logger.error("Failed to move job status and log files, and cleaning up", e); - } - } - - private void cleanEmptyDirectories(File sourceDirectory) throws IOException - { - Path storagePath = this.configuration.getStorage().toPath(); - for (Path sourcePath = sourceDirectory.toPath(); - !Objects.equals(storagePath, sourcePath) && sourcePath != null && isDirEmpty(sourcePath); - sourcePath = sourcePath.getParent()) { - Files.delete(sourcePath); - } - } - - private static boolean isDirEmpty(Path directory) throws IOException - { - try (DirectoryStream dirStream = Files.newDirectoryStream(directory)) { - return !dirStream.iterator().hasNext(); - } - } - - /** - * @param folder the folder from where to load the job status - * @throws IOException when failing to load the status file - */ - private JobStatus loadStatus(File folder) throws IOException - { - this.readLock.lock(); - - try { - // First try as ZIP - File statusFile = getStatusFile(folder); - if (statusFile.exists()) { - JobStatus status = loadJobStatus(statusFile); - - // Check if there is a separated log available - for (File child : folder.listFiles()) { - if (!child.isDirectory() && child.getName().startsWith(STATUS_LOG_PREFIX)) { + this.cache.set(idString, status); + } else { try { - LoggerTail loggerTail = createLoggerTail(new File(folder, STATUS_LOG_PREFIX), true); - - if (status instanceof AbstractJobStatus) { - ((AbstractJobStatus) status).setLoggerTail(loggerTail); - } + status = this.persistentJobStatusStore.loadJobStatus(id); + this.cache.set(idString, computeCacheValue(status)); } catch (Exception e) { - this.logger.error("Failed to load the job status log in [{}]", folder, e); - } + this.logger.warn("Failed to load job status for id {}", id, e); - break; + this.cache.remove(idString); + } } } - - return status; + } finally { + lock.unlock(); } - } finally { - this.readLock.unlock(); } - return null; + return status == NOSTATUS ? null : status; } - private static File getStatusFile(File folder) + private static @NonNull JobStatus computeCacheValue(JobStatus status) { - File statusFile = new File(folder, FILENAME_STATUS_ZIP); - if (!statusFile.exists()) { - // Then try as XML - statusFile = new File(folder, FILENAME_STATUS_XML); - } - return statusFile; + return status != null ? status : NOSTATUS; } - /** - * @param statusFile the file containing job status to load - * @return the job status - * @throws IOException when failing to load the job status from the file - */ - private JobStatus loadJobStatus(File statusFile) throws IOException - { - return this.serializer.read(statusFile); - } - - // JobStatusStorage - - /** - * @param id the id of the job - * @param moveToCurrent if the job status should be moved from a previous to the current location - * @return the folder where to store the job related informations - */ - private File getAndMoveJobFolder(List id, boolean moveToCurrent) - { - JobStatusFolderResolver currentResolver = this.folderResolvers.get(0); - - File folder = currentResolver.getFolder(id); - - if (moveToCurrent && !getStatusFile(folder).exists()) { - File previousFolder = null; - File previousStatusFile = null; - for (JobStatusFolderResolver folderResolver - : this.folderResolvers.subList(1, this.folderResolvers.size())) { - File f = folderResolver.getFolder(id); - File s = getStatusFile(f); - if (s.exists()) { - previousFolder = f; - previousStatusFile = s; - break; - } - } - - if (previousFolder != null) { - moveJobStatus(previousFolder, previousStatusFile, folder); - } - } - - return folder; - } - - private File getJobLogBaseFile(List id) + @Override + public void store(JobStatus status) { - return new File(getAndMoveJobFolder(id, true), STATUS_LOG_PREFIX); + store(status, false); } - /** - * @param status the job status to save - */ - private void saveJobStatus(JobStatus status) + @Override + public void storeAsync(JobStatus status) { - try { - this.writeLock.lock(); - - try { - File statusFile = getAndMoveJobFolder(status.getRequest().getId(), true); - statusFile = new File(statusFile, FILENAME_STATUS_ZIP); - - this.logger.debug("Serializing status [{}] in [{}]", status.getRequest().getId(), statusFile); - - this.serializer.write(status, statusFile); - } finally { - this.writeLock.unlock(); - } - } catch (Exception e) { - this.logger.warn("Failed to save job status [{}]", status, e); - } + store(status, true); } @Override - public JobStatus getJobStatus(List id) + public void remove(List id) { String idString = toUniqueString(id); - JobStatus status = this.cache.get(idString); - - if (status == null) { - status = maybeLoadStatus(id, idString); - } - - return status == NOSTATUS ? null : status; - } - - private synchronized JobStatus maybeLoadStatus(List id, String idString) - { - JobStatus status = this.cache.get(idString); - - if (status == null) { - try { - status = loadStatus(getAndMoveJobFolder(id, true)); + Lock lock = this.locks.get(idString); + lock.lock(); - this.cache.set(idString, status); - } catch (Exception e) { - this.logger.warn("Failed to load job status for id {}", id, e); + try { + this.persistentJobStatusStore.removeJobStatus(id); - this.cache.remove(idString); - } + this.cache.remove(idString); + } catch (IOException e) { + this.logger.warn("Failed to remove job status for id {}", id, e); + } finally { + lock.unlock(); } - - return status; } @Override - public void store(JobStatus status) + public LoggerTail createLoggerTail(List jobId, boolean readonly) { - store(status, false); + return this.persistentJobStatusStore.createLoggerTail(jobId, readonly); } - @Override - public void storeAsync(JobStatus status) + private String toUniqueString(List id) { - store(status, true); + return StringUtils.join(id, '/'); } private void store(JobStatus status, boolean async) { if (status != null && status.getRequest() != null && status.getRequest().getId() != null) { - synchronized (this.cache) { - String id = toUniqueString(status.getRequest().getId()); + String id = toUniqueString(status.getRequest().getId()); + boolean serializable = JobUtils.isSerializable(status); + Lock lock = this.locks.get(id); + lock.lock(); + try { this.logger.debug("Store status [{}] in cache", id); this.cache.set(id, status); + + // Track pending writes for serializable statuses to prevent stale reads from disk during the + // window between cache eviction and save completion. + if (serializable) { + this.pendingWrites.put(id, status); + } + } finally { + lock.unlock(); } - // Only store Serializable job status on file system - if (JobUtils.isSerializable(status)) { + // Only store Serializable job status on the file system + if (serializable) { if (async) { this.executorService.execute(new JobStatusSerializerRunnable(status)); } else { @@ -487,68 +246,19 @@ private void store(JobStatus status, boolean async) } } - @Override - public void remove(List id) + protected void saveJobStatus(JobStatus status) { - this.writeLock.lock(); - + String id = toUniqueString(status.getRequest().getId()); + Lock lock = this.locks.get(id); + lock.lock(); try { - // Delete the job status from all possible locations to ensure that when loading it again, it indeed - // cannot be found anymore. - for (JobStatusFolderResolver folderResolver : this.folderResolvers) { - File jobFolder = folderResolver.getFolder(id); - - if (jobFolder.isDirectory()) { - try { - deleteJobStatusFiles(jobFolder); - cleanEmptyDirectories(jobFolder); - } catch (IOException e) { - this.logger.warn("Failed to delete job folder [{}]", jobFolder, e); - } - } - } - - this.cache.remove(toUniqueString(id)); + this.persistentJobStatusStore.saveJobStatus(status); + } catch (Exception e) { + this.logger.warn("Failed to save job status for id {}", status.getRequest().getId(), e); } finally { - this.writeLock.unlock(); + // Use two-arg remove to only clear our own entry, not one from a newer store() call. + this.pendingWrites.remove(id, status); + lock.unlock(); } } - - private static void deleteJobStatusFiles(File jobFolder) throws IOException - { - for (File file : jobFolder.listFiles()) { - if (!file.isDirectory() && (file.getName().startsWith(STATUS_LOG_PREFIX) - || file.getName().startsWith(FILENAME_STATUS_XML))) - { - Files.delete(file.toPath()); - } - } - } - - @Override - public LoggerTail createLoggerTail(List jobId, boolean readonly) - { - if (jobId != null) { - try { - return createLoggerTail(getJobLogBaseFile(jobId), readonly); - } catch (Exception e) { - this.logger.error("Failed to create a logger tail for job [{}]", jobId, e); - } - } - - return new LogQueue(); - } - - private LoggerTail createLoggerTail(File logBaseFile, boolean readonly) throws IOException - { - return this.loggerManager.createLoggerTail(logBaseFile.toPath(), readonly); - } - - /** - * Remove all elements from the cache. - */ - public void flushCache() - { - this.cache.removeAll(); - } } diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/DefaultPersistentJobStatusStore.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/DefaultPersistentJobStatusStore.java new file mode 100644 index 0000000000..c213e3d0a3 --- /dev/null +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/DefaultPersistentJobStatusStore.java @@ -0,0 +1,391 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.job.internal; + +import java.io.File; +import java.io.IOException; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Objects; + +import javax.inject.Inject; +import javax.inject.Singleton; + +import org.apache.commons.configuration2.PropertiesConfiguration; +import org.apache.commons.configuration2.builder.FileBasedConfigurationBuilder; +import org.apache.commons.configuration2.builder.fluent.Parameters; +import org.apache.commons.io.FileUtils; +import org.slf4j.Logger; +import org.xwiki.component.annotation.Component; +import org.xwiki.component.phase.Initializable; +import org.xwiki.component.phase.InitializationException; +import org.xwiki.job.AbstractJobStatus; +import org.xwiki.job.JobManagerConfiguration; +import org.xwiki.job.JobStatusStore; +import org.xwiki.job.event.status.JobStatus; +import org.xwiki.logging.LogQueue; +import org.xwiki.logging.LoggerManager; +import org.xwiki.logging.tail.LoggerTail; + +/** + * Default implementation of {@link JobStatusStore}. + * + * @version $Id$ + * @since 18.2.0RC1 + */ +@Component +@Singleton +public class DefaultPersistentJobStatusStore implements PersistentJobStatusStore, Initializable +{ + /** + * The current version of the store. Should be upgraded if any change is made. + */ + private static final int VERSION = 1; + + /** + * The name of the file where the job status is stored as XML. + */ + private static final String FILENAME_STATUS_XML = "status.xml"; + + /** + * The name of the file where the job status is ZIPPED. + */ + private static final String FILENAME_STATUS_ZIP = FILENAME_STATUS_XML + ".zip"; + + /** + * The name of the file where various information about the status store are stored (like the version of the store). + */ + private static final String INDEX_FILE = "store.properties"; + + /** + * The name of the property containing the version of the store. + */ + private static final String INDEX_FILE_VERSION = "version"; + + private static final String STATUS_LOG_PREFIX = "log"; + + @Inject + private LoggerManager loggerManager; + + @Inject + private JobStatusSerializer serializer; + + @Inject + private List folderResolvers; + + @Inject + private JobManagerConfiguration configuration; + + @Inject + private Logger logger; + + @Override + public void initialize() throws InitializationException + { + try { + // Check if the store need to be upgraded + File folder = this.configuration.getStorage(); + File file = new File(folder, INDEX_FILE); + + FileBasedConfigurationBuilder builder = + new FileBasedConfigurationBuilder<>(PropertiesConfiguration.class, null, true) + .configure(new Parameters().properties().setFile(file)); + PropertiesConfiguration properties = builder.getConfiguration(); + int version = properties.getInt(INDEX_FILE_VERSION, 0); + if (VERSION > version) { + repair(); + + // Update version + properties.setProperty(INDEX_FILE_VERSION, VERSION); + builder.save(); + } + } catch (Exception e) { + this.logger.error("Failed to load jobs", e); + } + } + + /** + * Load jobs from directory. + * + * @throws IOException when failing to load statuses + */ + private void repair() throws IOException + { + File folder = this.configuration.getStorage(); + + if (folder.exists()) { + if (!folder.isDirectory()) { + throw new IOException("Not a directory: " + folder); + } + + repairFolder(folder); + } + } + + /** + * @param folder the folder from where to load the jobs + */ + private void repairFolder(File folder) + { + for (File file : folder.listFiles()) { + if (file.isDirectory()) { + repairFolder(file); + } else if (file.getName().equals(FILENAME_STATUS_ZIP) || file.getName().equals(FILENAME_STATUS_XML)) { + try { + JobStatus status = loadStatus(folder); + + if (status != null) { + File properFolder = getAndMoveJobFolder(status.getRequest().getId(), false); + + if (!folder.equals(properFolder)) { + moveJobStatus(folder, file, properFolder); + } + } + } catch (Exception e) { + this.logger.warn("Failed to load job status in folder [{}]", folder, e); + } + } + } + } + + private void moveJobStatus(File sourceDirectory, File statusFile, File targetDirectory) + { + try { + // Check if the target already exists. + File targetStatusZip = new File(targetDirectory, FILENAME_STATUS_ZIP); + File targetStatus = new File(targetDirectory, FILENAME_STATUS_XML); + + // Compare the last modified times of the source and the target status file. + long sourceLastModified = statusFile.lastModified(); + // The target last modified time will be 0 if the target status file does not exist. + long targetLastModified = Math.max(targetStatusZip.lastModified(), targetStatus.lastModified()); + + if (sourceLastModified > targetLastModified) { + // Source is newer (or target doesn't exist), overwrite the target. + if (targetDirectory.isDirectory()) { + deleteJobStatusFiles(targetDirectory); + } + + // Move the status in its right place + FileUtils.moveFileToDirectory(statusFile, targetDirectory, true); + // Move the job log, too. + for (File file : sourceDirectory.listFiles()) { + if (!file.isDirectory() && file.getName().startsWith(STATUS_LOG_PREFIX)) { + FileUtils.moveFileToDirectory(file, targetDirectory, true); + } + } + } else { + // Target is more recent, remove the source. + deleteJobStatusFiles(sourceDirectory); + } + + // Remove the source directory and its parents if they are empty now. + cleanEmptyDirectories(sourceDirectory); + } catch (IOException e) { + this.logger.error("Failed to move job status and log files, and cleaning up", e); + } + } + + private void cleanEmptyDirectories(File sourceDirectory) throws IOException + { + Path storagePath = this.configuration.getStorage().toPath(); + for (Path sourcePath = sourceDirectory.toPath(); + !Objects.equals(storagePath, sourcePath) && sourcePath != null && isDirEmpty(sourcePath); + sourcePath = sourcePath.getParent()) { + Files.delete(sourcePath); + } + } + + private static boolean isDirEmpty(Path directory) throws IOException + { + try (DirectoryStream dirStream = Files.newDirectoryStream(directory)) { + return !dirStream.iterator().hasNext(); + } + } + + /** + * @param folder the folder from where to load the job status + * @throws IOException when failing to load the status file + */ + private JobStatus loadStatus(File folder) throws IOException + { + // First try as ZIP + File statusFile = getStatusFile(folder); + if (statusFile.exists()) { + JobStatus status = loadJobStatus(statusFile); + + // Check if there is a separated log available + for (File child : folder.listFiles()) { + if (!child.isDirectory() && child.getName().startsWith(STATUS_LOG_PREFIX)) { + try { + LoggerTail loggerTail = createLoggerTail(new File(folder, STATUS_LOG_PREFIX), true); + + if (status instanceof AbstractJobStatus) { + ((AbstractJobStatus) status).setLoggerTail(loggerTail); + } + } catch (Exception e) { + this.logger.error("Failed to load the job status log in [{}]", folder, e); + } + + break; + } + } + + return status; + } + + return null; + } + + private static File getStatusFile(File folder) + { + File statusFile = new File(folder, FILENAME_STATUS_ZIP); + if (!statusFile.exists()) { + // Then try as XML + statusFile = new File(folder, FILENAME_STATUS_XML); + } + return statusFile; + } + + /** + * @param statusFile the file containing job status to load + * @return the job status + * @throws IOException when failing to load the job status from the file + */ + private JobStatus loadJobStatus(File statusFile) throws IOException + { + return this.serializer.read(statusFile); + } + + // JobStatusStorage + + /** + * @param id the id of the job + * @param moveToCurrent if the job status should be moved from a previous to the current location + * @return the folder where to store the job related informations + */ + private synchronized File getAndMoveJobFolder(List id, boolean moveToCurrent) + { + JobStatusFolderResolver currentResolver = this.folderResolvers.get(0); + + File folder = currentResolver.getFolder(id); + + if (moveToCurrent && !getStatusFile(folder).exists()) { + File previousFolder = null; + File previousStatusFile = null; + for (JobStatusFolderResolver folderResolver + : this.folderResolvers.subList(1, this.folderResolvers.size())) { + File f = folderResolver.getFolder(id); + File s = getStatusFile(f); + if (s.exists()) { + previousFolder = f; + previousStatusFile = s; + break; + } + } + + if (previousFolder != null) { + moveJobStatus(previousFolder, previousStatusFile, folder); + } + } + + return folder; + } + + private File getJobLogBaseFile(List id) + { + return new File(getAndMoveJobFolder(id, true), STATUS_LOG_PREFIX); + } + + /** + * @param status the job status to save + */ + @Override + public void saveJobStatus(JobStatus status) throws IOException + { + if (!JobUtils.isSerializable(status)) { + return; + } + + File statusFile = getAndMoveJobFolder(status.getRequest().getId(), true); + statusFile = new File(statusFile, FILENAME_STATUS_ZIP); + + this.logger.debug("Serializing status [{}] in [{}]", status.getRequest().getId(), statusFile); + + this.serializer.write(status, statusFile); + } + + @Override + public JobStatus loadJobStatus(List id) throws IOException + { + return loadStatus(getAndMoveJobFolder(id, true)); + } + + @Override + public synchronized void removeJobStatus(List id) + { + // Delete the job status from all possible locations to ensure that when loading it again, it indeed + // cannot be found anymore. + for (JobStatusFolderResolver folderResolver : this.folderResolvers) { + File jobFolder = folderResolver.getFolder(id); + + if (jobFolder.isDirectory()) { + try { + deleteJobStatusFiles(jobFolder); + cleanEmptyDirectories(jobFolder); + } catch (IOException e) { + this.logger.warn("Failed to delete job folder [{}]", jobFolder, e); + } + } + } + } + + private static void deleteJobStatusFiles(File jobFolder) throws IOException + { + for (File file : jobFolder.listFiles()) { + if (!file.isDirectory() && (file.getName().startsWith(STATUS_LOG_PREFIX) + || file.getName().startsWith(FILENAME_STATUS_XML))) + { + Files.delete(file.toPath()); + } + } + } + + @Override + public LoggerTail createLoggerTail(List jobId, boolean readonly) + { + if (jobId != null) { + try { + return createLoggerTail(getJobLogBaseFile(jobId), readonly); + } catch (Exception e) { + this.logger.error("Failed to create a logger tail for job [{}]", jobId, e); + } + } + + return new LogQueue(); + } + + private LoggerTail createLoggerTail(File logBaseFile, boolean readonly) throws IOException + { + return this.loggerManager.createLoggerTail(logBaseFile.toPath(), readonly); + } +} diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/JobStatusFolderResolver.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/JobStatusFolderResolver.java index 9bca9029f9..7fb5cc41ce 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/JobStatusFolderResolver.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/JobStatusFolderResolver.java @@ -42,4 +42,12 @@ public interface JobStatusFolderResolver * @return the folder where the job status and log should be stored according to the resolver */ File getFolder(List jobID); + + /** + * @param jobID the ID of the job for which the segments of the folder path shall be retrieved + * @return the segments of the folder path for the job status and log according to the resolver, starting from + * the configured storage directory + * @since 18.2.0RC1 + */ + List getFolderSegments(List jobID); } diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/JobStatusSerializer.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/JobStatusSerializer.java index 58352d3552..d90df31c75 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/JobStatusSerializer.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/JobStatusSerializer.java @@ -22,6 +22,7 @@ import java.io.File; import java.io.FileInputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.nio.charset.Charset; @@ -33,7 +34,6 @@ import javax.inject.Inject; import javax.inject.Singleton; -import org.apache.commons.compress.archivers.ArchiveOutputStream; import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream; import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream; @@ -61,6 +61,8 @@ public class JobStatusSerializer private static final String ZIP_EXTENSION = "zip"; + private static final String STATUS_FILE_NAME = "status.xml"; + /** * Used to serialize and unserialize status. */ @@ -82,14 +84,14 @@ public void write(JobStatus status, File file) throws IOException File tempFile = File.createTempFile(file.getName(), ".tmp", parent); try (OutputStream stream = getOutputStream(tempFile, isZip(file))) { - if (stream instanceof ArchiveOutputStream) { - ((ArchiveOutputStream) stream).putArchiveEntry(new ZipArchiveEntry("status.xml")); + if (stream instanceof ZipArchiveOutputStream archiveOutputStream) { + archiveOutputStream.putArchiveEntry(new ZipArchiveEntry(STATUS_FILE_NAME)); } write(status, stream); - if (stream instanceof ArchiveOutputStream) { - ((ArchiveOutputStream) stream).closeArchiveEntry(); + if (stream instanceof ZipArchiveOutputStream archiveOutputStream) { + archiveOutputStream.closeArchiveEntry(); } } @@ -113,6 +115,52 @@ public void write(JobStatus status, File file) throws IOException } } + /** + * Serialize a job status to an output stream. + * + * @param status the status to serialize + * @param stream the stream to serialize the status to + * @param zip whether to serialize the status as a ZIP file + * @throws IOException when failing to serialize the status + * @since 18.2.0RC1 + */ + public void write(JobStatus status, OutputStream stream, boolean zip) throws IOException + { + if (zip) { + ZipArchiveOutputStream archiveOutputStream = new ZipArchiveOutputStream(stream); + archiveOutputStream.putArchiveEntry(new ZipArchiveEntry(STATUS_FILE_NAME)); + write(status, archiveOutputStream); + archiveOutputStream.closeArchiveEntry(); + archiveOutputStream.finish(); + } else { + write(status, stream); + } + } + + /** + * Reads a job status from the given stream. + * + * @param stream the stream to read from + * @param zip {@code true} if the stream contains a ZIP file, {@code false} otherwise + * @return the job status read from the stream + * @throws IOException when failing to read the status + * @since 18.2.0RC1 + */ + public JobStatus read(InputStream stream, boolean zip) throws IOException + { + if (zip) { + try (ZipArchiveInputStream archiveInputStream = new ZipArchiveInputStream(stream)) { + ZipArchiveEntry entry = archiveInputStream.getNextEntry(); + if (entry == null) { + throw new IOException("Missing entry in the status zip file"); + } + return (JobStatus) this.xstream.fromXML(archiveInputStream); + } + } else { + return (JobStatus) this.xstream.fromXML(stream); + } + } + private OutputStream getOutputStream(File tempFile, boolean zip) throws IOException { tempFile.mkdirs(); @@ -145,14 +193,7 @@ public void write(JobStatus status, OutputStream stream) throws IOException public JobStatus read(File file) throws IOException { if (isZip(file)) { - try (ZipArchiveInputStream stream = new ZipArchiveInputStream(new FileInputStream(file))) { - ZipArchiveEntry entry = stream.getNextZipEntry(); - if (entry == null) { - throw new IOException("Missing entry in the status zip file"); - } - - return (JobStatus) this.xstream.fromXML(stream); - } + return read(new FileInputStream(file), true); } else { return (JobStatus) this.xstream.fromXML(file); } diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/PersistentJobStatusStore.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/PersistentJobStatusStore.java new file mode 100644 index 0000000000..3fad443333 --- /dev/null +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/PersistentJobStatusStore.java @@ -0,0 +1,72 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.job.internal; + +import java.io.IOException; +import java.util.List; + +import org.xwiki.component.annotation.Role; +import org.xwiki.job.event.status.JobStatus; +import org.xwiki.logging.tail.LoggerTail; + +/** + * Persistent storage for job statuses. Internal role that is used by {@link DefaultPersistentJobStatusStore}. + * + * @version $Id$ + * @since 18.2.0RC1 + */ +@Role +public interface PersistentJobStatusStore +{ + /** + * Save the job status to the persistent storage. + * + * @param status the job status to save + * @throws IOException when failing to save the job status + */ + void saveJobStatus(JobStatus status) throws IOException; + + /** + * Load the job status from the persistent storage. + * + * @param id the id of the job + * @return the job status + * @throws IOException when failing to load the job status + */ + JobStatus loadJobStatus(List id) throws IOException; + + /** + * Remove the job status from the persistent storage. + * + * @param id the id of the job + * @throws IOException when failing to remove the job status + */ + void removeJobStatus(List id) throws IOException; + + /** + * Create a {@link LoggerTail} instance for the given job. If {@code readonly} is true, this allows accessing the + * existing job log, otherwise, a fresh log is created. + * + * @param jobId the identifier of the job + * @param readonly true of the log is readonly + * @return the {@link LoggerTail} instance + */ + LoggerTail createLoggerTail(List jobId, boolean readonly); +} diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/Version1JobStatusFolderResolver.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/Version1JobStatusFolderResolver.java index b03c3c397e..383dbc021d 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/Version1JobStatusFolderResolver.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/Version1JobStatusFolderResolver.java @@ -19,7 +19,7 @@ */ package org.xwiki.job.internal; -import java.io.File; +import java.util.List; import javax.annotation.Priority; import javax.inject.Named; @@ -41,8 +41,8 @@ public class Version1JobStatusFolderResolver extends AbstractJobStatusFolderResolver { @Override - protected File addIDElement(String idElement, File folder) + protected List encodeAndSplit(String idElement) { - return new File(folder, nullAwareURLEncode(idElement)); + return List.of(nullAwareURLEncode(idElement)); } } diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/Version2JobStatusFolderResolver.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/Version2JobStatusFolderResolver.java index 1fc4490717..5e513f5b75 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/Version2JobStatusFolderResolver.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/Version2JobStatusFolderResolver.java @@ -19,7 +19,8 @@ */ package org.xwiki.job.internal; -import java.io.File; +import java.util.ArrayList; +import java.util.List; import javax.annotation.Priority; import javax.inject.Named; @@ -42,9 +43,9 @@ public class Version2JobStatusFolderResolver extends AbstractJobStatusFolderResolver { @Override - protected File addIDElement(String idElement, File folder) + protected List encodeAndSplit(String idElement) { - File result = folder; + List result = new ArrayList<>(); // Cut each element if is it's bigger than 255 bytes (and not characters) since it's a very common // limit for a single element of the path among file systems // To be sure to deal with characters not taking more than 1 byte, we start by encoding it in base 64 @@ -52,11 +53,11 @@ protected File addIDElement(String idElement, File folder) : Base64.encodeBase64String(idElement.getBytes()); if (encodedIdElement != null && encodedIdElement.length() > 255) { do { - result = new File(result, nullAwareURLEncode(encodedIdElement.substring(0, 255))); + result.add(nullAwareURLEncode(encodedIdElement.substring(0, 255))); encodedIdElement = encodedIdElement.substring(255); } while (encodedIdElement.length() > 255); } else { - result = new File(result, nullAwareURLEncode(encodedIdElement)); + result.add(nullAwareURLEncode(encodedIdElement)); } return result; } diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/Version3JobStatusFolderResolver.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/Version3JobStatusFolderResolver.java index d2cf3aa56f..4fa0c93639 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/Version3JobStatusFolderResolver.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/Version3JobStatusFolderResolver.java @@ -19,7 +19,6 @@ */ package org.xwiki.job.internal; -import java.io.File; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -69,23 +68,13 @@ public class Version3JobStatusFolderResolver extends AbstractJobStatusFolderReso } @Override - protected File getBaseFolder() + public List getBaseFolderSegments() { - return new File(super.getBaseFolder(), "3"); + return List.of("3"); } - protected File addIDElement(String fullIdElement, File folder) - { - File result = folder; - - for (String encodedPart : encodeAndSplit(fullIdElement)) { - result = new File(result, encodedPart); - } - - return result; - } - - private List encodeAndSplit(String content) + @Override + protected List encodeAndSplit(String content) { if (content == null) { return List.of(FOLDER_NULL); diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/resources/META-INF/components.txt b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/resources/META-INF/components.txt index e8a6fe73fa..0e9bb34bae 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/resources/META-INF/components.txt +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/resources/META-INF/components.txt @@ -4,6 +4,7 @@ org.xwiki.job.internal.DefaultJobManagerConfiguration org.xwiki.job.internal.DefaultJobProgressManager org.xwiki.job.internal.DefaultJobStatusStorage org.xwiki.job.internal.DefaultJobStatusStore +org.xwiki.job.internal.DefaultPersistentJobStatusStore org.xwiki.job.internal.JobStatusSerializer org.xwiki.job.internal.script.safe.JobStatusScriptSafeProvider org.xwiki.job.internal.xstream.SerializableXStreamChecker diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/DefaultJobStatusStoreTest.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/DefaultJobStatusStoreTest.java index 07d2be5554..b8f5f33586 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/DefaultJobStatusStoreTest.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/DefaultJobStatusStoreTest.java @@ -19,60 +19,45 @@ */ package org.xwiki.job.internal; -import java.io.File; -import java.util.Arrays; +import java.io.IOException; import java.util.List; -import java.util.Objects; +import java.util.concurrent.ExecutorService; -import javax.inject.Provider; - -import org.apache.commons.io.FileUtils; -import org.apache.commons.lang3.StringUtils; import org.junit.jupiter.api.Test; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; -import org.slf4j.Logger; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; +import org.xwiki.cache.Cache; +import org.xwiki.cache.CacheException; import org.xwiki.cache.CacheManager; -import org.xwiki.cache.internal.MapCache; -import org.xwiki.component.manager.ComponentManager; import org.xwiki.component.util.ReflectionUtils; -import org.xwiki.job.AbstractJobStatus; import org.xwiki.job.DefaultJobStatus; import org.xwiki.job.DefaultRequest; import org.xwiki.job.JobManagerConfiguration; -import org.xwiki.job.Request; -import org.xwiki.job.annotation.Serializable; import org.xwiki.job.event.status.JobStatus; -import org.xwiki.job.internal.xstream.SerializableXStreamChecker; -import org.xwiki.job.test.SerializableStandaloneComponent; -import org.xwiki.job.test.StandaloneComponent; -import org.xwiki.job.test.UnserializableJobStatus; -import org.xwiki.logging.LoggerManager; -import org.xwiki.logging.event.LogEvent; -import org.xwiki.logging.internal.tail.XStreamFileLoggerTail; -import org.xwiki.logging.marker.TranslationMarker; import org.xwiki.logging.tail.LoggerTail; -import org.xwiki.test.TestEnvironment; -import org.xwiki.test.XWikiTempDirUtil; +import org.xwiki.test.LogLevel; import org.xwiki.test.annotation.BeforeComponent; -import org.xwiki.test.annotation.ComponentList; +import org.xwiki.test.junit5.LogCaptureExtension; import org.xwiki.test.junit5.mockito.ComponentTest; -import org.xwiki.test.junit5.mockito.InjectComponentManager; import org.xwiki.test.junit5.mockito.InjectMockComponents; import org.xwiki.test.junit5.mockito.MockComponent; -import org.xwiki.test.mockito.MockitoComponentManager; -import org.xwiki.xstream.internal.SafeXStream; -import org.xwiki.xstream.internal.XStreamUtils; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -82,752 +67,260 @@ * @version $Id$ */ @ComponentTest -// @formatter:off -@ComponentList({ - SafeXStream.class, - XStreamUtils.class, - SerializableXStreamChecker.class, - JobStatusSerializer.class, - XStreamFileLoggerTail.class, - TestEnvironment.class, - // Add all components to ensure that the priority works as expected. - Version1JobStatusFolderResolver.class, - Version2JobStatusFolderResolver.class, - Version3JobStatusFolderResolver.class -}) -// @formatter:on class DefaultJobStatusStoreTest { - private static final List ID = Arrays.asList("test"); - - private static final String STATUS_XML_ZIP = "status.xml.zip"; - - @Serializable - private static class SerializableCrossReferenceObject - { - public SerializableCrossReferenceObject field; - - public SerializableCrossReferenceObject() - { - this.field = this; - } - } - - @Serializable - private static class SerializableProvider implements Provider - { - @Override - public String get() - { - return null; - } - } - - private static class SerializableImplementationProvider implements Provider, java.io.Serializable - { - private static final long serialVersionUID = 1L; - - @Override - public String get() - { - return null; - } - } - - @Serializable - private static class SerializableObjectTest - { - public Object field; - - public SerializableObjectTest(Object field) - { - this.field = field; - } - } - - @Serializable - private static class CustomSerializableObject - { - public String field; - - public CustomSerializableObject(String field) - { - this.field = field; - } - - @Override - public boolean equals(Object obj) - { - return Objects.equals(((CustomSerializableObject) obj).field, this.field); - } - } - - @Serializable - private static class SerializableCustomObject - { - public String field; - - public SerializableCustomObject(String field) - { - this.field = field; - } - - @Override - public boolean equals(Object obj) - { - return Objects.equals(((SerializableCustomObject) obj).field, this.field); - } - } - - @Serializable(false) - private static class NotSerializableCustomObject - { - public String field; - - public NotSerializableCustomObject(String field) - { - this.field = field; - } + private static final List ID = List.of("id1", "id2"); - @Override - public boolean equals(Object obj) - { - return Objects.equals(((NotSerializableCustomObject) obj).field, this.field); - } - - @Override - public String toString() - { - return this.field; - } - } + private static final String ID_STRING = "id1/id2"; - @SuppressWarnings("serial") - private static class TestException extends Exception - { - private Object custom; + private static final String ERROR = "error"; - public TestException(String message, Throwable cause, Object custom) - { - super(message, cause); + private static final String TYPE = "type"; - this.custom = custom; - } - - public Object getCustom() - { - return this.custom; - } - } + @RegisterExtension + private final LogCaptureExtension logCapture = new LogCaptureExtension(LogLevel.WARN); @InjectMockComponents private DefaultJobStatusStore store; - @InjectComponentManager - private MockitoComponentManager componentManager; - - @MockComponent - private JobManagerConfiguration jobManagerConfiguration; - @MockComponent - private LoggerManager loggerManager; + private PersistentJobStatusStore persistentJobStatusStore; @MockComponent private CacheManager cacheManager; - private File storeDirectory; - - private File wrongDirectory; - - private File wrongDirectory2; - - private File correctDirectory; + @MockComponent + private JobManagerConfiguration configuration; - private File wrongStatusInCorrectDirectory; + private Cache cache; @BeforeComponent void before() throws Exception { - this.storeDirectory = XWikiTempDirUtil.createTemporaryDirectory(); - - FileUtils.copyDirectory(new File("src/test/resources/jobs/status/"), this.storeDirectory); - - this.wrongDirectory = new File(this.storeDirectory, "wrong/location"); - this.wrongDirectory2 = new File(this.storeDirectory, "wrong/location2"); - this.correctDirectory = new File(this.storeDirectory, "3/correct/location"); - assertTrue(this.wrongDirectory.isDirectory(), "Directory copied from resources doesn't exist."); - - File mostRecentFile = new File(this.wrongDirectory, STATUS_XML_ZIP); - File oldestFile = new File(this.wrongDirectory2, STATUS_XML_ZIP); - File middleFile = new File(this.correctDirectory, "status.xml"); - this.wrongStatusInCorrectDirectory = middleFile; - - assertTrue(mostRecentFile.exists()); - assertTrue(oldestFile.exists()); - assertTrue(middleFile.exists()); - - assertTrue(mostRecentFile.setLastModified(System.currentTimeMillis())); - assertTrue(oldestFile.setLastModified(System.currentTimeMillis() - 4000)); - assertTrue(middleFile.setLastModified(System.currentTimeMillis() - 2000)); - - when(this.jobManagerConfiguration.getStorage()).thenReturn(this.storeDirectory); - when(this.jobManagerConfiguration.getJobStatusCacheSize()).thenReturn(100); - - when(this.cacheManager.createNewCache(any())).thenReturn(new MapCache<>()); - - when(this.loggerManager.createLoggerTail(any(), anyBoolean())).then(new Answer() - { - @Override - public LoggerTail answer(InvocationOnMock invocation) throws Throwable - { - XStreamFileLoggerTail loggerTail = componentManager.getInstance(XStreamFileLoggerTail.class); - loggerTail.initialize(invocation.getArgument(0), invocation.getArgument(1)); - - return loggerTail; - } - }); - } - - private DefaultJobStatus createStatus() - { - return createStatus(true); - } - - private DefaultJobStatus createStatus(boolean logtail) - { - DefaultRequest request = new DefaultRequest(); - request.setId(ID); - - DefaultJobStatus status = new DefaultJobStatus<>("type", request, null, null, null); - - if (logtail) { - status.setLoggerTail(this.store.createLoggerTail(ID, false)); - } - - return status; - } - - private S getStatus() - { - return (S) this.store.getJobStatus(ID); - } - - private S storeGet(S status) throws Exception - { - if (status instanceof AbstractJobStatus) { - ((AbstractJobStatus) status).getLoggerTail().close(); - } - - this.store.store(status); - - this.store.flushCache(); - - return getStatus(); + when(this.configuration.getJobStatusCacheSize()).thenReturn(100); + this.cache = mock(); + doReturn(this.cache).when(this.cacheManager).createNewCache(any()); } @Test - void jobStatusAndLogMoved() + void initializeWhenCacheCreationFails() throws CacheException { - JobStatus status = this.store.getJobStatus(List.of("correct", "location")); - assertNotNull(status); - LogEvent lastLogEvent = status.getLogTail().getLastLogEvent(); - assertNotNull(lastLogEvent); - // Verify that we got the correct job. - assertEquals("Finished correct job of type [{}] with identifier [{}]", lastLogEvent.getMessage()); - - assertFalse(this.wrongDirectory.exists()); - assertFalse(this.wrongDirectory.getParentFile().exists()); - assertFalse(this.wrongDirectory2.exists()); - assertFalse(this.wrongStatusInCorrectDirectory.exists()); - assertTrue(new File(this.correctDirectory, STATUS_XML_ZIP).exists()); - assertTrue(new File(this.correctDirectory, "log.index").exists()); - assertTrue(new File(this.correctDirectory, "log.xml").exists()); - } + when(this.configuration.getJobStatusCacheSize()).thenReturn(100); + when(this.cacheManager.createNewCache(any())).thenThrow(new CacheException(ERROR)); - @Test - void getJobStatusWithNullId() - { - JobStatus jobStatus = this.store.getJobStatus(null); - - assertNotNull(jobStatus); - assertNull(jobStatus.getRequest().getId()); - assertEquals(JobStatus.State.FINISHED, jobStatus.getState()); - - assertSame(jobStatus, this.store.getJobStatus(null)); + assertThrows(Exception.class, () -> this.store.initialize()); } @Test - void getJobStatusWithMultipleId() + void getJobStatusWhenCacheHit() { - JobStatus jobStatus = this.store.getJobStatus(Arrays.asList("id1", "id2")); + JobStatus status = mock(); - assertNotNull(jobStatus); - assertEquals(Arrays.asList("id1", "id2"), jobStatus.getRequest().getId()); - assertEquals(JobStatus.State.FINISHED, jobStatus.getState()); + when(this.cache.get(ID_STRING)).thenReturn(status); - assertSame(jobStatus, this.store.getJobStatus(Arrays.asList("id1", "id2"))); - } + assertSame(status, this.store.getJobStatus(ID)); - @Test - void getJobStatusInOldPlace() - { - JobStatus jobStatus = this.store.getJobStatus(Arrays.asList("id1", "id2", "id3")); - - assertNotNull(jobStatus); - assertEquals(Arrays.asList("id1", "id2", "id3"), jobStatus.getRequest().getId()); - assertEquals(JobStatus.State.FINISHED, jobStatus.getState()); - } - - @Test - void getJobStatusInWrongPlaceAndWithInvalidLogArgument() - { - JobStatus jobStatus = this.store.getJobStatus(Arrays.asList("invalidlogargument")); - - assertNotNull(jobStatus); - assertEquals(3, jobStatus.getLog().size()); + verify(this.cache).get(ID_STRING); + verifyNoInteractions(this.persistentJobStatusStore); } @Test - void getJobStatusThatDoesNotExist() + void getJobStatusWhenCacheMissThenLoadedAndCached() throws Exception { - JobStatus jobStatus = this.store.getJobStatus(Arrays.asList("nostatus")); + JobStatus status = mock(); - assertNull(jobStatus); + when(this.cache.get(ID_STRING)).thenReturn(null); + when(this.persistentJobStatusStore.loadJobStatus(ID)).thenReturn(status); - JobStatusSerializer mockSerializer = mock(JobStatusSerializer.class); - ReflectionUtils.setFieldValue(this.store, "serializer", mockSerializer); + assertSame(status, this.store.getJobStatus(ID)); - jobStatus = this.store.getJobStatus(Arrays.asList("nostatus")); - assertNull(jobStatus); - - verifyNoMoreInteractions(mockSerializer); + verify(this.persistentJobStatusStore).loadJobStatus(ID); + verify(this.cache).set(ID_STRING, status); } @Test - void removeJobStatus() + void getJobStatusWhenCacheMissThenSecondCheckAvoidsRead() { - List id = null; - - JobStatus jobStatus = this.store.getJobStatus(id); - - assertNotNull(jobStatus); - assertNull(jobStatus.getRequest().getId()); - assertEquals(JobStatus.State.FINISHED, jobStatus.getState()); - - assertSame(jobStatus, this.store.getJobStatus(id)); + JobStatus status = mock(); - this.store.remove(id); + when(this.cache.get(ID_STRING)).thenReturn(null, status); - assertSame(null, this.store.getJobStatus(id)); - } - - @Test - void removeNotExistingJobStatus() - { - this.store.remove(Arrays.asList("notexist")); - } + assertSame(status, this.store.getJobStatus(ID)); - @Test - void storeNullJobStatus() - { - this.store.store(null); + verify(this.cache, times(2)).get(ID_STRING); + verifyNoInteractions(this.persistentJobStatusStore); } @Test - void storeJobStatusWithNullRequest() + void getJobStatusWhenLoadFailsRemovesCacheAndLogs() throws Exception { - JobStatus jobStatus = new DefaultJobStatus("type", null, null, null, null); + when(this.cache.get(ID_STRING)).thenReturn(null); + when(this.persistentJobStatusStore.loadJobStatus(ID)).thenThrow(new IOException(ERROR)); - this.store.store(jobStatus); - } + assertNull(this.store.getJobStatus(ID)); - @Test - void storeJobStatusWithNullId() - { - JobStatus jobStatus = new DefaultJobStatus("type", new DefaultRequest(), null, null, null); - - this.store.store(jobStatus); + verify(this.cache).remove(ID_STRING); + assertEquals("Failed to load job status for id [id1, id2]", logCapture.getMessage(0)); } @Test - void storeJobStatusWithBigId() + void getJobStatusWhenLoadReturnsNullCachesSentinel() throws Exception { - DefaultRequest request = new DefaultRequest(); - request.setId(StringUtils.repeat('a', 768)); + when(this.cache.get(ID_STRING)).thenReturn(null); + when(this.persistentJobStatusStore.loadJobStatus(ID)).thenReturn(null); - JobStatus jobStatus = new DefaultJobStatus<>("type", request, null, null, null); + assertNull(this.store.getJobStatus(ID)); - this.store.store(jobStatus); + verify(this.persistentJobStatusStore).loadJobStatus(ID); - String longAName = StringUtils.repeat('a', 250); - - assertTrue(new File(this.storeDirectory, - "3/%s/%s/%s/aaaaaaaaaaaaaaaaaa/status.xml.zip".formatted(longAName, longAName, longAName)).exists()); + // Verify a non-null sentinel was cached to avoid repeated disk reads for missing statuses. + ArgumentCaptor captor = ArgumentCaptor.captor(); + verify(this.cache).set(eq(ID_STRING), captor.capture()); + assertNotNull(captor.getValue()); } @Test - void storeJobStatusWithNullPartInId() + void getJobStatusWhenPendingWriteExists() throws Exception { - DefaultRequest request = new DefaultRequest(); - request.setId("first", null, "second"); + ExecutorService executor = mock(); + ReflectionUtils.setFieldValue(this.store, "executorService", executor); - JobStatus jobStatus = new DefaultJobStatus<>("type", request, null, null, null); - - this.store.store(jobStatus); - - assertTrue(new File(this.storeDirectory, "3/first/&null/second/status.xml.zip").exists()); - } - - @Test - void storeJobStatusWithProblematicCharacters() - { - DefaultRequest request = new DefaultRequest(); - request.setId("..", ".", " .*."); - - JobStatus jobStatus = new DefaultJobStatus<>("type", request, null, null, null); - - this.store.store(jobStatus); - - assertTrue(new File(this.storeDirectory, "3/%2E%2E/%2E/%20.%2A%2E/status.xml.zip").exists()); - } - - @Test - void storeUnserializableJobStatus() - { - List id = Arrays.asList("test"); DefaultRequest request = new DefaultRequest(); - request.setId(id); - JobStatus jobStatus = new UnserializableJobStatus("type", request, null, null, null); - - this.store.store(jobStatus); - - // Verify that the status hasn't been serialized, indirectly verifying that isSerializable() has been called and - // returned true. - assertFalse(new File(this.storeDirectory, "3/test/status.xml").exists()); - } - - @Test - void storeUnserializedJobStatus() - { - List id = Arrays.asList("test"); - DefaultRequest request = new DefaultRequest(); - request.setId(id); - request.setStatusSerialized(false); - JobStatus jobStatus = new DefaultJobStatus("type", request, null, null, null); - - this.store.store(jobStatus); - - // Verify that the status hasn't been serialized, indirectly verifying that isSerializable() has been called and - // returned true. - assertFalse(new File(this.storeDirectory, "3/test/status.xml").exists()); - } - - @Test - void storeJobStatusWhenSerializable() - { - List id = Arrays.asList("newstatus"); - - DefaultRequest request = new DefaultRequest(); - request.setId(id); - JobStatus jobStatus = new DefaultJobStatus("type", request, null, null, null); - - this.store.store(jobStatus); - - assertSame(jobStatus, this.store.getJobStatus(id)); - - // Verify that the status has been serialized, indirectly verifying that isSerializable() has been called and - // returned true. - assertTrue(new File(this.storeDirectory, "3/%s/status.xml.zip".formatted(id.get(0))).exists()); - } - - @Test - void serializeUnserializeWhenLogMessage() throws Exception - { - DefaultJobStatus status = createStatus(); - - status.getLoggerTail().error("error message"); - - status = storeGet(status); - - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - } - - @Test - void serializeUnserializeWhenLogMarker() throws Exception - { - DefaultJobStatus status = createStatus(); - - status.getLoggerTail().error(new TranslationMarker("translation.key"), "error message"); - - status = storeGet(status); - - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals(new TranslationMarker("translation.key"), status.getLog().peek().getMarker()); - } - - @Test - void serializeUnserializeWhenLogWithException() throws Exception - { - DefaultJobStatus status = createStatus(); - - status.getLoggerTail().error("error message", - new TestException("exception message", new Exception("cause"), "custom")); - - status = storeGet(status); - - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals("exception message", status.getLog().peek().getThrowable().getMessage()); - assertEquals("cause", status.getLog().peek().getThrowable().getCause().getMessage()); - assertNull(((TestException) status.getLog().peek().getThrowable()).getCustom(), "exception message"); - } - - @Test - void serializeUnserializeWhenLogWithArguments() throws Exception - { - DefaultJobStatus status = createStatus(); - - status.getLoggerTail().error("error message", "arg1", "arg2"); - - status = storeGet(status); - - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals("arg1", status.getLog().peek().getArgumentArray()[0]); - assertEquals("arg2", status.getLog().peek().getArgumentArray()[1]); - } - - @Test - void serializeUnserializeWhenLogWithNullArguments() throws Exception - { - DefaultJobStatus status = createStatus(); - - status.getLoggerTail().error("error message", "arg1", null, "arg3"); - - status = storeGet(status); - - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals("arg1", status.getLog().peek().getArgumentArray()[0]); - assertNull(status.getLog().peek().getArgumentArray()[1]); - assertEquals("arg3", status.getLog().peek().getArgumentArray()[2]); - } - - @Test - void serializeUnserializeWhenLogWithComponentArgument() throws Exception - { - DefaultJobStatus status = createStatus(); - - status.getLoggerTail().error("error message", new DefaultJobStatusStore()); - - status = storeGet(status); - - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals(String.class, status.getLog().peek().getArgumentArray()[0].getClass()); - } - - @Test - void serializeUnserializeWhenLogWithStandaloneComponentArgument() throws Exception - { - DefaultJobStatus status = createStatus(); - - status.getLoggerTail().error("error message", new StandaloneComponent()); - - status = storeGet(status); + request.setId(ID); + request.setStatusSerialized(true); - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals(String.class, status.getLog().peek().getArgumentArray()[0].getClass()); - } + JobStatus status = new DefaultJobStatus<>(TYPE, request, null, null, null); - @Test - void serializeUnserializeWhenLogWithSerializableStandaloneComponentArgument() throws Exception - { - DefaultJobStatus status = createStatus(); + // Store async to create a pending write (executor mock won't actually run the task). + this.store.storeAsync(status); - status.getLoggerTail().error("error message", new SerializableStandaloneComponent()); + // Simulate cache eviction by making get() return null. + when(this.cache.get(ID_STRING)).thenReturn(null); - status = storeGet(status); + // getJobStatus should find the pending write instead of loading from disk. + assertSame(status, this.store.getJobStatus(ID)); - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals(SerializableStandaloneComponent.class, status.getLog().peek().getArgumentArray()[0].getClass()); + verify(this.persistentJobStatusStore, never()).loadJobStatus(any()); } @Test - void serializeUnserializeWhenLogWithCrossReference() throws Exception + void removeDelegatesAndEvictsCache() throws Exception { - DefaultJobStatus status = createStatus(); + this.store.remove(ID); - status.getLoggerTail().error("message", new SerializableCrossReferenceObject()); - - status = storeGet(status); - - assertNotNull(status.getLog()); - SerializableCrossReferenceObject obj = - (SerializableCrossReferenceObject) status.getLog().peek().getArgumentArray()[0]; - assertSame(obj, obj.field); + verify(this.persistentJobStatusStore).removeJobStatus(ID); + verify(this.cache).remove(ID_STRING); } @Test - void serializeUnserializeWhenLogWithComponentField() throws Exception + void removeWhenPersistenceFailsKeepsCacheAndLogs() throws Exception { - DefaultJobStatus status = createStatus(); + doThrow(new IOException(ERROR)).when(this.persistentJobStatusStore).removeJobStatus(ID); - status.getLoggerTail().error("error message", new SerializableObjectTest(new DefaultJobStatusStore())); + this.store.remove(ID); - status = storeGet(status); - - assertNotNull(status.getLog()); - assertNull(((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field); + verify(this.cache, never()).remove(ID_STRING); + assertEquals("Failed to remove job status for id [id1, id2]", logCapture.getMessage(0)); } @Test - void serializeUnserializeWhenLogWithStandaloneComponentField() throws Exception + void storeWithNullStatusDoesNothing() { - DefaultJobStatus status = createStatus(); - - status.getLoggerTail().error("error message", new SerializableObjectTest(new StandaloneComponent())); - - status = storeGet(status); - - assertNotNull(status.getLog()); - assertNull(((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field); - } - - @Test - void serializeUnserializeWhenLogWithLoggerField() throws Exception - { - DefaultJobStatus status = createStatus(); - - status.getLoggerTail().error("error message", new SerializableObjectTest(mock(Logger.class))); - - status = storeGet(status); + this.store.store(null); - assertNotNull(status.getLog()); - assertNull(((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field); + verifyNoInteractions(this.cache); + verifyNoInteractions(this.persistentJobStatusStore); } @Test - void serializeUnserializeWhenLogWithProviderField() throws Exception + void storeWithNullRequestDoesNothing() { - DefaultJobStatus status = createStatus(); + JobStatus status = new DefaultJobStatus<>(TYPE, null, null, null, null); - status.getLoggerTail().error("error message", new SerializableObjectTest(mock(Provider.class))); - - status = storeGet(status); + this.store.store(status); - assertNotNull(status.getLog()); - assertNull(((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field); + verifyNoInteractions(this.cache); + verifyNoInteractions(this.persistentJobStatusStore); } @Test - void serializeUnserializeWhenLogWithComponentManagerField() throws Exception + void storeWithNullIdDoesNothing() { - DefaultJobStatus status = createStatus(); + JobStatus status = new DefaultJobStatus<>(TYPE, new DefaultRequest(), null, null, null); - status.getLoggerTail().error("error message", new SerializableObjectTest(mock(ComponentManager.class))); - - status = storeGet(status); + this.store.store(status); - assertNotNull(status.getLog()); - assertNull(((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field); + verifyNoInteractions(this.cache); + verifyNoInteractions(this.persistentJobStatusStore); } @Test - void serializeUnserializeWhenLogWithSerializableProviderField() throws Exception + void storeSerializableSavesSynchronously() throws Exception { - DefaultJobStatus status = createStatus(); + DefaultRequest request = new DefaultRequest(); + request.setId(ID); + request.setStatusSerialized(true); - status.getLoggerTail().error("error message", new SerializableObjectTest(new SerializableProvider())); + JobStatus status = new DefaultJobStatus<>(TYPE, request, null, null, null); - status = storeGet(status); + this.store.store(status); - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals(SerializableProvider.class, - ((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field.getClass()); + verify(this.cache).set(ID_STRING, status); + verify(this.persistentJobStatusStore).saveJobStatus(status); } @Test - void serializeUnserializeWhenLogWithSerializableImplementationProviderField() throws Exception + void storeAsyncSerializableSchedulesSave() throws Exception { - DefaultJobStatus status = createStatus(); + ExecutorService executor = mock(); + ReflectionUtils.setFieldValue(this.store, "executorService", executor); - status.getLoggerTail().error("error message", - new SerializableObjectTest(new SerializableImplementationProvider())); + DefaultRequest request = new DefaultRequest(); + request.setId(ID); + request.setStatusSerialized(true); - status = storeGet(status); + JobStatus status = new DefaultJobStatus<>(TYPE, request, null, null, null); - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals(SerializableImplementationProvider.class, - ((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field.getClass()); - } + this.store.storeAsync(status); - @Test - void serializeUnserializeWhenLogWithCustomObjectArgument() throws Exception - { - DefaultJobStatus status = createStatus(); + verify(this.cache).set(ID_STRING, status); - status.getLoggerTail().error("error message", new CustomSerializableObject("value")); + ArgumentCaptor runnableCaptor = ArgumentCaptor.captor(); + verify(executor).execute(runnableCaptor.capture()); - status = storeGet(status); + runnableCaptor.getValue().run(); - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals(new CustomSerializableObject("value"), status.getLog().peek().getArgumentArray()[0]); + verify(this.persistentJobStatusStore).saveJobStatus(status); } @Test - void serializeUnserializeWhenLogWithSerializableCustomObjectArgument() throws Exception + void storeSerializableWhenSaveFailsLogs() throws Exception { - DefaultJobStatus status = createStatus(); - - status.getLoggerTail().error("error message", new SerializableCustomObject("value")); - - status = storeGet(status); - - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals(new SerializableCustomObject("value"), status.getLog().peek().getArgumentArray()[0]); - } + DefaultRequest request = new DefaultRequest(); + request.setId(ID); + request.setStatusSerialized(true); - @Test - void serializeUnserializeWhenLogWithNotSerializableCustomObjectArgument() throws Exception - { - DefaultJobStatus status = createStatus(); + JobStatus status = new DefaultJobStatus<>(TYPE, request, null, null, null); - status.getLoggerTail().error("error message", new NotSerializableCustomObject("value")); + doThrow(new IOException(ERROR)).when(this.persistentJobStatusStore).saveJobStatus(status); - status = storeGet(status); + this.store.store(status); - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals("value", status.getLog().peek().getArgumentArray()[0]); + assertEquals("Failed to save job status for id [id1, id2]", logCapture.getMessage(0)); } - @Test - void serializeUnserializeWithLogQueue() throws Exception + @ParameterizedTest + @ValueSource(booleans = { true, false }) + void createLoggerTailDelegates(boolean readonly) throws Exception { - JobStatus status = createStatus(false); - - status.getLog().error("error message", "arg1", "arg2"); + LoggerTail loggerTail = mock(); + when(this.persistentJobStatusStore.createLoggerTail(ID, readonly)).thenReturn(loggerTail); - status = storeGet(status); - - assertNotNull(status.getLog()); - assertEquals("error message", status.getLog().peek().getMessage()); - assertEquals("arg1", status.getLog().peek().getArgumentArray()[0]); - assertEquals("arg2", status.getLog().peek().getArgumentArray()[1]); - } + try (LoggerTail actual = this.store.createLoggerTail(ID, readonly)) { + assertSame(loggerTail, actual); + } - @Test - void createLoggerTailWithNullId() - { - assertNotNull(this.store.createLoggerTail(null, true)); + verify(this.persistentJobStatusStore).createLoggerTail(ID, readonly); + verifyNoMoreInteractions(this.persistentJobStatusStore); } } diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/DefaultPersistentJobStatusStoreTest.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/DefaultPersistentJobStatusStoreTest.java new file mode 100644 index 0000000000..ffb5249589 --- /dev/null +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/DefaultPersistentJobStatusStoreTest.java @@ -0,0 +1,826 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.job.internal; + +import java.io.File; +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; + +import javax.inject.Provider; + +import org.apache.commons.io.FileUtils; +import org.apache.commons.lang3.StringUtils; +import org.junit.jupiter.api.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; +import org.slf4j.Logger; +import org.xwiki.component.manager.ComponentManager; +import org.xwiki.component.util.ReflectionUtils; +import org.xwiki.job.AbstractJobStatus; +import org.xwiki.job.DefaultJobStatus; +import org.xwiki.job.DefaultRequest; +import org.xwiki.job.JobManagerConfiguration; +import org.xwiki.job.Request; +import org.xwiki.job.annotation.Serializable; +import org.xwiki.job.event.status.JobStatus; +import org.xwiki.job.internal.xstream.SerializableXStreamChecker; +import org.xwiki.job.test.SerializableStandaloneComponent; +import org.xwiki.job.test.StandaloneComponent; +import org.xwiki.job.test.UnserializableJobStatus; +import org.xwiki.logging.LoggerManager; +import org.xwiki.logging.event.LogEvent; +import org.xwiki.logging.internal.tail.XStreamFileLoggerTail; +import org.xwiki.logging.marker.TranslationMarker; +import org.xwiki.logging.tail.LoggerTail; +import org.xwiki.test.TestEnvironment; +import org.xwiki.test.XWikiTempDirUtil; +import org.xwiki.test.annotation.BeforeComponent; +import org.xwiki.test.annotation.ComponentList; +import org.xwiki.test.junit5.mockito.ComponentTest; +import org.xwiki.test.junit5.mockito.InjectComponentManager; +import org.xwiki.test.junit5.mockito.InjectMockComponents; +import org.xwiki.test.junit5.mockito.MockComponent; +import org.xwiki.test.mockito.MockitoComponentManager; +import org.xwiki.xstream.internal.SafeXStream; +import org.xwiki.xstream.internal.XStreamUtils; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link DefaultPersistentJobStatusStore}. + * + * @version $Id$ + */ +@ComponentTest +// @formatter:off +@ComponentList({ + SafeXStream.class, + XStreamUtils.class, + SerializableXStreamChecker.class, + JobStatusSerializer.class, + XStreamFileLoggerTail.class, + TestEnvironment.class, + // Add all components to ensure that the priority works as expected. + Version1JobStatusFolderResolver.class, + Version2JobStatusFolderResolver.class, + Version3JobStatusFolderResolver.class +}) +// @formatter:on +class DefaultPersistentJobStatusStoreTest +{ + private static final List ID = Arrays.asList("test"); + + private static final String STATUS_XML_ZIP = "status.xml.zip"; + + @Serializable + private static class SerializableCrossReferenceObject + { + public SerializableCrossReferenceObject field; + + public SerializableCrossReferenceObject() + { + this.field = this; + } + } + + @Serializable + private static class SerializableProvider implements Provider + { + @Override + public String get() + { + return null; + } + } + + private static class SerializableImplementationProvider implements Provider, java.io.Serializable + { + private static final long serialVersionUID = 1L; + + @Override + public String get() + { + return null; + } + } + + @Serializable + private static class SerializableObjectTest + { + public Object field; + + public SerializableObjectTest(Object field) + { + this.field = field; + } + } + + @Serializable + private static class CustomSerializableObject + { + public String field; + + public CustomSerializableObject(String field) + { + this.field = field; + } + + @Override + public boolean equals(Object obj) + { + return Objects.equals(((CustomSerializableObject) obj).field, this.field); + } + } + + @Serializable + private static class SerializableCustomObject + { + public String field; + + public SerializableCustomObject(String field) + { + this.field = field; + } + + @Override + public boolean equals(Object obj) + { + return Objects.equals(((SerializableCustomObject) obj).field, this.field); + } + } + + @Serializable(false) + private static class NotSerializableCustomObject + { + public String field; + + public NotSerializableCustomObject(String field) + { + this.field = field; + } + + @Override + public boolean equals(Object obj) + { + return Objects.equals(((NotSerializableCustomObject) obj).field, this.field); + } + + @Override + public String toString() + { + return this.field; + } + } + + @SuppressWarnings("serial") + private static class TestException extends Exception + { + private Object custom; + + public TestException(String message, Throwable cause, Object custom) + { + super(message, cause); + + this.custom = custom; + } + + public Object getCustom() + { + return this.custom; + } + } + + @InjectMockComponents + private DefaultPersistentJobStatusStore store; + + @InjectComponentManager + private MockitoComponentManager componentManager; + + @MockComponent + private JobManagerConfiguration jobManagerConfiguration; + + @MockComponent + private LoggerManager loggerManager; + + private File storeDirectory; + + private File wrongDirectory; + + private File wrongDirectory2; + + private File correctDirectory; + + private File wrongStatusInCorrectDirectory; + + @BeforeComponent + void before() throws Exception + { + this.storeDirectory = XWikiTempDirUtil.createTemporaryDirectory(); + + FileUtils.copyDirectory(new File("src/test/resources/jobs/status/"), this.storeDirectory); + + this.wrongDirectory = new File(this.storeDirectory, "wrong/location"); + this.wrongDirectory2 = new File(this.storeDirectory, "wrong/location2"); + this.correctDirectory = new File(this.storeDirectory, "3/correct/location"); + assertTrue(this.wrongDirectory.isDirectory(), "Directory copied from resources doesn't exist."); + + File mostRecentFile = new File(this.wrongDirectory, STATUS_XML_ZIP); + File oldestFile = new File(this.wrongDirectory2, STATUS_XML_ZIP); + File middleFile = new File(this.correctDirectory, "status.xml"); + this.wrongStatusInCorrectDirectory = middleFile; + + assertTrue(mostRecentFile.exists()); + assertTrue(oldestFile.exists()); + assertTrue(middleFile.exists()); + + assertTrue(mostRecentFile.setLastModified(System.currentTimeMillis())); + assertTrue(oldestFile.setLastModified(System.currentTimeMillis() - 4000)); + assertTrue(middleFile.setLastModified(System.currentTimeMillis() - 2000)); + + when(this.jobManagerConfiguration.getStorage()).thenReturn(this.storeDirectory); + + when(this.loggerManager.createLoggerTail(any(), anyBoolean())).then(new Answer() + { + @Override + public LoggerTail answer(InvocationOnMock invocation) throws Throwable + { + XStreamFileLoggerTail loggerTail = componentManager.getInstance(XStreamFileLoggerTail.class); + loggerTail.initialize(invocation.getArgument(0), invocation.getArgument(1)); + + return loggerTail; + } + }); + } + + private DefaultJobStatus createStatus() + { + return createStatus(true); + } + + private DefaultJobStatus createStatus(boolean logtail) + { + DefaultRequest request = new DefaultRequest(); + request.setId(ID); + + DefaultJobStatus status = new DefaultJobStatus<>("type", request, null, null, null); + + if (logtail) { + status.setLoggerTail(this.store.createLoggerTail(ID, false)); + } + + return status; + } + + private S getStatus() throws IOException + { + return (S) this.store.loadJobStatus(ID); + } + + private S storeGet(S status) throws Exception + { + if (status instanceof AbstractJobStatus abstractStatus) { + abstractStatus.getLoggerTail().close(); + } + + this.store.saveJobStatus(status); + + return getStatus(); + } + + @Test + void jobStatusAndLogMoved() throws IOException + { + JobStatus status = this.store.loadJobStatus(List.of("correct", "location")); + assertNotNull(status); + LogEvent lastLogEvent = status.getLogTail().getLastLogEvent(); + assertNotNull(lastLogEvent); + // Verify that we got the correct job. + assertEquals("Finished correct job of type [{}] with identifier [{}]", lastLogEvent.getMessage()); + + assertFalse(this.wrongDirectory.exists()); + assertFalse(this.wrongDirectory.getParentFile().exists()); + assertFalse(this.wrongDirectory2.exists()); + assertFalse(this.wrongStatusInCorrectDirectory.exists()); + assertTrue(new File(this.correctDirectory, STATUS_XML_ZIP).exists()); + assertTrue(new File(this.correctDirectory, "log.index").exists()); + assertTrue(new File(this.correctDirectory, "log.xml").exists()); + } + + @Test + void getJobStatusWithNullId() throws IOException + { + JobStatus jobStatus = this.store.loadJobStatus(null); + + assertNotNull(jobStatus); + assertNull(jobStatus.getRequest().getId()); + assertEquals(JobStatus.State.FINISHED, jobStatus.getState()); + } + + @Test + void getJobStatusWithMultipleId() throws IOException + { + JobStatus jobStatus = this.store.loadJobStatus(Arrays.asList("id1", "id2")); + + assertNotNull(jobStatus); + assertEquals(Arrays.asList("id1", "id2"), jobStatus.getRequest().getId()); + assertEquals(JobStatus.State.FINISHED, jobStatus.getState()); + + JobStatus secondJobStatus = this.store.loadJobStatus(Arrays.asList("id1", "id2")); + assertEquals(jobStatus.getRequest(), secondJobStatus.getRequest()); + assertEquals(jobStatus.getState(), secondJobStatus.getState()); + assertEquals(jobStatus.getJobType(), secondJobStatus.getJobType()); + } + + @Test + void getJobStatusInOldPlace() throws IOException + { + JobStatus jobStatus = this.store.loadJobStatus(Arrays.asList("id1", "id2", "id3")); + + assertNotNull(jobStatus); + assertEquals(Arrays.asList("id1", "id2", "id3"), jobStatus.getRequest().getId()); + assertEquals(JobStatus.State.FINISHED, jobStatus.getState()); + } + + @Test + void getJobStatusInWrongPlaceAndWithInvalidLogArgument() throws IOException + { + JobStatus jobStatus = this.store.loadJobStatus(Arrays.asList("invalidlogargument")); + + assertNotNull(jobStatus); + assertEquals(3, jobStatus.getLog().size()); + } + + @Test + void getJobStatusThatDoesNotExist() throws IOException + { + JobStatus jobStatus = this.store.loadJobStatus(Arrays.asList("nostatus")); + + assertNull(jobStatus); + + JobStatusSerializer mockSerializer = mock(JobStatusSerializer.class); + ReflectionUtils.setFieldValue(this.store, "serializer", mockSerializer); + + jobStatus = this.store.loadJobStatus(Arrays.asList("nostatus")); + assertNull(jobStatus); + + verifyNoMoreInteractions(mockSerializer); + } + + @Test + void removeJobStatus() throws IOException + { + List id = null; + + JobStatus jobStatus = this.store.loadJobStatus(id); + + assertNotNull(jobStatus); + assertNull(jobStatus.getRequest().getId()); + assertEquals(JobStatus.State.FINISHED, jobStatus.getState()); + + this.store.removeJobStatus(id); + + assertNull(this.store.loadJobStatus(id)); + } + + @Test + void removeNotExistingJobStatus() throws IOException + { + this.store.removeJobStatus(Arrays.asList("notexist")); + } + + @Test + void storeNullJobStatus() throws IOException + { + this.store.saveJobStatus(null); + } + + @Test + void storeJobStatusWithNullRequest() throws IOException + { + JobStatus jobStatus = new DefaultJobStatus("type", null, null, null, null); + + this.store.saveJobStatus(jobStatus); + } + + @Test + void storeJobStatusWithNullId() throws IOException + { + JobStatus jobStatus = new DefaultJobStatus("type", new DefaultRequest(), null, null, null); + + this.store.saveJobStatus(jobStatus); + } + + @Test + void storeJobStatusWithBigId() throws IOException + { + DefaultRequest request = new DefaultRequest(); + request.setId(StringUtils.repeat('a', 768)); + + JobStatus jobStatus = new DefaultJobStatus<>("type", request, null, null, null); + + this.store.saveJobStatus(jobStatus); + + String longAName = StringUtils.repeat('a', 250); + + assertTrue(new File(this.storeDirectory, + "3/%s/%s/%s/aaaaaaaaaaaaaaaaaa/status.xml.zip".formatted(longAName, longAName, longAName)).exists()); + } + + @Test + void storeJobStatusWithNullPartInId() throws IOException + { + DefaultRequest request = new DefaultRequest(); + request.setId("first", null, "second"); + + JobStatus jobStatus = new DefaultJobStatus<>("type", request, null, null, null); + + this.store.saveJobStatus(jobStatus); + + assertTrue(new File(this.storeDirectory, "3/first/&null/second/status.xml.zip").exists()); + } + + @Test + void storeJobStatusWithProblematicCharacters() throws IOException + { + DefaultRequest request = new DefaultRequest(); + request.setId("..", ".", " .*."); + + JobStatus jobStatus = new DefaultJobStatus<>("type", request, null, null, null); + + this.store.saveJobStatus(jobStatus); + + assertTrue(new File(this.storeDirectory, "3/%2E%2E/%2E/%20.%2A%2E/status.xml.zip").exists()); + } + + @Test + void storeUnserializableJobStatus() throws IOException + { + List id = Arrays.asList("test"); + DefaultRequest request = new DefaultRequest(); + request.setId(id); + JobStatus jobStatus = new UnserializableJobStatus("type", request, null, null, null); + + this.store.saveJobStatus(jobStatus); + + // Verify that the status hasn't been serialized, indirectly verifying that isSerializable() has been called and + // returned true. + assertFalse(new File(this.storeDirectory, "3/test/status.xml").exists()); + } + + @Test + void storeUnserializedJobStatus() throws IOException + { + List id = Arrays.asList("test"); + DefaultRequest request = new DefaultRequest(); + request.setId(id); + request.setStatusSerialized(false); + JobStatus jobStatus = new DefaultJobStatus("type", request, null, null, null); + + this.store.saveJobStatus(jobStatus); + + // Verify that the status hasn't been serialized, indirectly verifying that isSerializable() has been called and + // returned true. + assertFalse(new File(this.storeDirectory, "3/test/status.xml").exists()); + } + + @Test + void storeJobStatusWhenSerializable() throws IOException + { + List id = Arrays.asList("newstatus"); + + DefaultRequest request = new DefaultRequest(); + request.setId(id); + JobStatus jobStatus = new DefaultJobStatus("type", request, null, null, null); + + this.store.saveJobStatus(jobStatus); + + JobStatus loadedJobStatus = this.store.loadJobStatus(id); + assertEquals(jobStatus.getRequest(), loadedJobStatus.getRequest()); + assertEquals(jobStatus.getState(), loadedJobStatus.getState()); + assertEquals(jobStatus.getJobType(), loadedJobStatus.getJobType()); + + // Verify that the status has been serialized, indirectly verifying that isSerializable() has been called and + // returned true. + assertTrue(new File(this.storeDirectory, "3/%s/status.xml.zip".formatted(id.get(0))).exists()); + } + + @Test + void serializeUnserializeWhenLogMessage() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message"); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + } + + @Test + void serializeUnserializeWhenLogMarker() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error(new TranslationMarker("translation.key"), "error message"); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals(new TranslationMarker("translation.key"), status.getLog().peek().getMarker()); + } + + @Test + void serializeUnserializeWhenLogWithException() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", + new TestException("exception message", new Exception("cause"), "custom")); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals("exception message", status.getLog().peek().getThrowable().getMessage()); + assertEquals("cause", status.getLog().peek().getThrowable().getCause().getMessage()); + assertNull(((TestException) status.getLog().peek().getThrowable()).getCustom(), "exception message"); + } + + @Test + void serializeUnserializeWhenLogWithArguments() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", "arg1", "arg2"); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals("arg1", status.getLog().peek().getArgumentArray()[0]); + assertEquals("arg2", status.getLog().peek().getArgumentArray()[1]); + } + + @Test + void serializeUnserializeWhenLogWithNullArguments() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", "arg1", null, "arg3"); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals("arg1", status.getLog().peek().getArgumentArray()[0]); + assertNull(status.getLog().peek().getArgumentArray()[1]); + assertEquals("arg3", status.getLog().peek().getArgumentArray()[2]); + } + + @Test + void serializeUnserializeWhenLogWithComponentArgument() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", new DefaultPersistentJobStatusStore()); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals(String.class, status.getLog().peek().getArgumentArray()[0].getClass()); + } + + @Test + void serializeUnserializeWhenLogWithStandaloneComponentArgument() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", new StandaloneComponent()); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals(String.class, status.getLog().peek().getArgumentArray()[0].getClass()); + } + + @Test + void serializeUnserializeWhenLogWithSerializableStandaloneComponentArgument() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", new SerializableStandaloneComponent()); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals(SerializableStandaloneComponent.class, status.getLog().peek().getArgumentArray()[0].getClass()); + } + + @Test + void serializeUnserializeWhenLogWithCrossReference() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("message", new SerializableCrossReferenceObject()); + + status = storeGet(status); + + assertNotNull(status.getLog()); + SerializableCrossReferenceObject obj = + (SerializableCrossReferenceObject) status.getLog().peek().getArgumentArray()[0]; + assertSame(obj, obj.field); + } + + @Test + void serializeUnserializeWhenLogWithComponentField() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", new SerializableObjectTest(new DefaultPersistentJobStatusStore())); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertNull(((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field); + } + + @Test + void serializeUnserializeWhenLogWithStandaloneComponentField() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", new SerializableObjectTest(new StandaloneComponent())); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertNull(((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field); + } + + @Test + void serializeUnserializeWhenLogWithLoggerField() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", new SerializableObjectTest(mock(Logger.class))); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertNull(((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field); + } + + @Test + void serializeUnserializeWhenLogWithProviderField() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", new SerializableObjectTest(mock(Provider.class))); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertNull(((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field); + } + + @Test + void serializeUnserializeWhenLogWithComponentManagerField() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", new SerializableObjectTest(mock(ComponentManager.class))); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertNull(((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field); + } + + @Test + void serializeUnserializeWhenLogWithSerializableProviderField() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", new SerializableObjectTest(new SerializableProvider())); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals(SerializableProvider.class, + ((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field.getClass()); + } + + @Test + void serializeUnserializeWhenLogWithSerializableImplementationProviderField() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", + new SerializableObjectTest(new SerializableImplementationProvider())); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals(SerializableImplementationProvider.class, + ((SerializableObjectTest) status.getLog().peek().getArgumentArray()[0]).field.getClass()); + } + + @Test + void serializeUnserializeWhenLogWithCustomObjectArgument() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", new CustomSerializableObject("value")); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals(new CustomSerializableObject("value"), status.getLog().peek().getArgumentArray()[0]); + } + + @Test + void serializeUnserializeWhenLogWithSerializableCustomObjectArgument() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", new SerializableCustomObject("value")); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals(new SerializableCustomObject("value"), status.getLog().peek().getArgumentArray()[0]); + } + + @Test + void serializeUnserializeWhenLogWithNotSerializableCustomObjectArgument() throws Exception + { + DefaultJobStatus status = createStatus(); + + status.getLoggerTail().error("error message", new NotSerializableCustomObject("value")); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals("value", status.getLog().peek().getArgumentArray()[0]); + } + + @Test + void serializeUnserializeWithLogQueue() throws Exception + { + JobStatus status = createStatus(false); + + status.getLog().error("error message", "arg1", "arg2"); + + status = storeGet(status); + + assertNotNull(status.getLog()); + assertEquals("error message", status.getLog().peek().getMessage()); + assertEquals("arg1", status.getLog().peek().getArgumentArray()[0]); + assertEquals("arg2", status.getLog().peek().getArgumentArray()[1]); + } + + @Test + void createLoggerTailWithNullId() + { + assertNotNull(this.store.createLoggerTail(null, true)); + } +} diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/JobStatusSerializerTest.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/JobStatusSerializerTest.java index 009ff219b0..8d24f5e2ef 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/JobStatusSerializerTest.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/JobStatusSerializerTest.java @@ -19,15 +19,20 @@ */ package org.xwiki.job.internal; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.xwiki.job.DefaultJobStatus; import org.xwiki.job.DefaultRequest; import org.xwiki.job.Request; import org.xwiki.job.event.status.JobStatus; import org.xwiki.test.annotation.ComponentList; +import org.xwiki.test.junit5.XWikiTempDir; import org.xwiki.test.junit5.mockito.ComponentTest; import org.xwiki.test.junit5.mockito.InjectMockComponents; import org.xwiki.xstream.internal.SafeXStream; @@ -48,13 +53,16 @@ class JobStatusSerializerTest @InjectMockComponents private JobStatusSerializer serializer; - private File testFile = new File("target/test/status.xml"); + @XWikiTempDir + private File tempDir; private JobStatus writeRead(JobStatus status) throws IOException { - this.serializer.write(status, this.testFile); + File testFile = new File(this.tempDir, "status.xml"); - return this.serializer.read(this.testFile); + this.serializer.write(status, testFile); + + return this.serializer.read(testFile); } @Test @@ -67,6 +75,19 @@ void serializeUnserialize() throws IOException assertEquals("type", status.getJobType()); } + @ParameterizedTest + @ValueSource(booleans = { false, true }) + void serializeUnserializeStream(boolean zip) throws IOException + { + JobStatus status = new DefaultJobStatus("type", new DefaultRequest(), null, null, null); + + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + this.serializer.write(status, outputStream, zip); + JobStatus readStatus = this.serializer.read(new ByteArrayInputStream(outputStream.toByteArray()), zip); + + assertEquals("type", readStatus.getJobType()); + } + @Test void serializeUnserializeProgress() throws IOException { diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/Version1JobStatusFolderResolverTest.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/Version1JobStatusFolderResolverTest.java index 3ebb4b5ed4..ae52e64884 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/Version1JobStatusFolderResolverTest.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/Version1JobStatusFolderResolverTest.java @@ -21,7 +21,6 @@ import java.io.File; import java.util.Arrays; -import java.util.Collections; import java.util.List; import org.junit.jupiter.api.BeforeEach; @@ -61,28 +60,36 @@ void setup() void getFolderWithNullId() { assertEquals(this.storageDir, this.resolver.getFolder(null)); + + assertEquals(List.of(), this.resolver.getFolderSegments(null)); } @Test void getFolderWithEmptyId() { - assertEquals(this.storageDir, this.resolver.getFolder(Collections.emptyList())); + assertEquals(this.storageDir, this.resolver.getFolder(List.of())); + + assertEquals(List.of(), this.resolver.getFolderSegments(List.of())); } @Test void getFolderWithSingleElementId() { - List id = Collections.singletonList("element"); + List id = List.of("element"); File expected = new File(this.storageDir, "element"); assertEquals(expected, this.resolver.getFolder(id)); + + assertEquals(id, this.resolver.getFolderSegments(id)); } @Test void getFolderWithMultipleElementId() { - List id = Arrays.asList("first", "second", "third"); + List id = List.of("first", "second", "third"); File expected = new File(new File(new File(this.storageDir, "first"), "second"), "third"); assertEquals(expected, this.resolver.getFolder(id)); + + assertEquals(id, this.resolver.getFolderSegments(id)); } @Test @@ -91,21 +98,27 @@ void getFolderWithNullElementInId() List id = Arrays.asList("first", null, "third"); File expected = new File(new File(new File(this.storageDir, "first"), "&null"), "third"); assertEquals(expected, this.resolver.getFolder(id)); + + assertEquals(List.of("first", "&null", "third"), this.resolver.getFolderSegments(id)); } @Test void getFolderWithSpecialCharactersInId() { - List id = Arrays.asList("a/b", "c?d", "e&f"); + List id = List.of("a/b", "c?d", "e&f"); File expected = new File(new File(new File(this.storageDir, "a%2Fb"), "c%3Fd"), "e%26f"); assertEquals(expected, this.resolver.getFolder(id)); + + assertEquals(List.of("a%2Fb", "c%3Fd", "e%26f"), this.resolver.getFolderSegments(id)); } @Test void getFolderWithSpacesInId() { - List id = Collections.singletonList("element with spaces"); + List id = List.of("element with spaces"); File expected = new File(this.storageDir, "element+with+spaces"); assertEquals(expected, this.resolver.getFolder(id)); + + assertEquals(List.of("element+with+spaces"), this.resolver.getFolderSegments(id)); } } diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/Version2JobStatusFolderResolverTest.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/Version2JobStatusFolderResolverTest.java index 1c73276675..32bb3f42e8 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/Version2JobStatusFolderResolverTest.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/Version2JobStatusFolderResolverTest.java @@ -84,12 +84,16 @@ void setup() void getFolderWithNullId() { assertEquals(this.storageDir, this.resolver.getFolder(null)); + + assertEquals(List.of(), this.resolver.getFolderSegments(null)); } @Test void getFolderWithEmptyId() { assertEquals(this.storageDir, this.resolver.getFolder(Collections.emptyList())); + + assertEquals(List.of(), this.resolver.getFolderSegments(Collections.emptyList())); } @Test @@ -112,6 +116,8 @@ void getFolderWithNullElementInId() String encodedThird = encodeComponent(THIRD); File expected = new File(new File(new File(this.storageDir, encodedFirst), "&null"), encodedThird); assertEquals(expected, this.resolver.getFolder(id)); + + assertEquals(List.of(encodedFirst, "&null", encodedThird), this.resolver.getFolderSegments(id)); } @Test @@ -129,6 +135,8 @@ void getFolderWithLongIdElement() List id = List.of(element); assertEquals(expected, this.resolver.getFolder(id)); + + assertEquals(encodedElements, this.resolver.getFolderSegments(id)); } @Test @@ -155,6 +163,11 @@ private void assertFolderPath(List idElements) expected = new File(expected, encodeComponent(element)); } assertEquals(expected, this.resolver.getFolder(idElements)); + + List encodedElements = idElements.stream() + .map(Version2JobStatusFolderResolverTest::encodeComponent) + .toList(); + assertEquals(encodedElements, this.resolver.getFolderSegments(idElements)); } private static String encodeComponent(String component) diff --git a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/Version3JobStatusFolderResolverTest.java b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/Version3JobStatusFolderResolverTest.java index 60a52c1192..82e463c96f 100644 --- a/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/Version3JobStatusFolderResolverTest.java +++ b/xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/Version3JobStatusFolderResolverTest.java @@ -76,12 +76,16 @@ void setup() void getFolderWithNullId() { assertEquals(this.baseDir, this.resolver.getFolder(null)); + + assertEquals(List.of("3"), this.resolver.getFolderSegments(null)); } @Test void getFolderWithEmptyId() { - assertEquals(this.baseDir, this.resolver.getFolder(Collections.emptyList())); + assertEquals(this.baseDir, this.resolver.getFolder(List.of())); + + assertEquals(List.of("3"), this.resolver.getFolderSegments(List.of())); } @Test @@ -90,6 +94,8 @@ void getFolderWithSingleElementId() List idElements = Collections.singletonList(ELEMENT_1); File expected = getExpectedFolder(idElements); assertEquals(expected, this.resolver.getFolder(idElements)); + + assertEquals(List.of("3", ELEMENT_1), this.resolver.getFolderSegments(idElements)); } @Test @@ -98,6 +104,8 @@ void getFolderWithMultipleElementId() List idElements = Arrays.asList(FIRST, SECOND, THIRD); File expected = getExpectedFolder(idElements); assertEquals(expected, this.resolver.getFolder(idElements)); + + assertEquals(List.of("3", FIRST, SECOND, THIRD), this.resolver.getFolderSegments(idElements)); } @Test @@ -106,6 +114,8 @@ void getFolderWithNullElementInId() List id = Arrays.asList(FIRST, null, THIRD); File expected = getExpectedFolder(List.of(FIRST, "&null", THIRD)); assertEquals(expected, this.resolver.getFolder(id)); + + assertEquals(List.of("3", FIRST, "&null", THIRD), this.resolver.getFolderSegments(id)); } @Test @@ -117,6 +127,9 @@ void getFolderWithLongIdElement() File expected = getExpectedFolder(List.of(element.substring(0, 250), element.substring(250))); assertEquals(expected, this.resolver.getFolder(id)); + + assertEquals(List.of("3", element.substring(0, 250), element.substring(250)), + this.resolver.getFolderSegments(id)); } @ParameterizedTest @@ -136,6 +149,8 @@ void getFolderWithSpecialCharactersInId(String idElement, String expectedEncoded List id = Collections.singletonList(idElement); File expected = new File(this.baseDir, expectedEncodedElement); assertEquals(expected, this.resolver.getFolder(id)); + + assertEquals(List.of("3", expectedEncodedElement), this.resolver.getFolderSegments(id)); } @Test @@ -159,6 +174,10 @@ void getFolderWithLongIdElementContainingSpecialChars() assertFalse(StringUtils.contains(encodedElement, "*")); assertTrue(encodedElement.length() <= 255); } + + // We collected the folder segments in reverse order, so we need to reverse them before comparing. + encodedElements.add("3"); + assertEquals(encodedElements.reversed(), this.resolver.getFolderSegments(id)); } private File getExpectedFolder(List idElements)