diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ba070a6f2..b3788cea82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com) ## Unreleased ([details][unreleased changes details]) +- #3225 - PackageGarbageCollector leaves temp files behind - #3187 - Remove warning during build on Java 11 or higher when DialogProviderAnnotationProcessor is invoked - #3242 - Actually update lodash to 4.17.21 (was mistakenly updated to 4.17.15 instead of 4.17.21) diff --git a/bundle/src/main/java/com/adobe/acs/commons/packagegarbagecollector/PackageGarbageCollectionJob.java b/bundle/src/main/java/com/adobe/acs/commons/packagegarbagecollector/PackageGarbageCollectionJob.java index 87f1e0a32e..d38c865827 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/packagegarbagecollector/PackageGarbageCollectionJob.java +++ b/bundle/src/main/java/com/adobe/acs/commons/packagegarbagecollector/PackageGarbageCollectionJob.java @@ -19,7 +19,26 @@ */ package com.adobe.acs.commons.packagegarbagecollector; -import org.apache.jackrabbit.JcrConstants; +import static com.adobe.acs.commons.packagegarbagecollector.PackageGarbageCollectionScheduler.GROUP_NAME; +import static com.adobe.acs.commons.packagegarbagecollector.PackageGarbageCollectionScheduler.MAX_AGE_IN_DAYS; +import static com.adobe.acs.commons.packagegarbagecollector.PackageGarbageCollectionScheduler.REMOVE_NOT_INSTALLED_PACKAGES; + +import java.io.IOException; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.Period; +import java.time.format.DateTimeFormatter; +import java.time.format.FormatStyle; +import java.util.Calendar; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Optional; +import java.util.stream.Stream; + +import javax.jcr.RepositoryException; +import javax.jcr.Session; + import org.apache.jackrabbit.vault.packaging.JcrPackage; import org.apache.jackrabbit.vault.packaging.JcrPackageDefinition; import org.apache.jackrabbit.vault.packaging.JcrPackageManager; @@ -35,24 +54,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.annotation.Nonnull; -import javax.jcr.Node; -import javax.jcr.RepositoryException; -import javax.jcr.Session; -import java.io.IOException; -import java.time.LocalDate; -import java.time.LocalDateTime; -import java.time.Period; -import java.time.format.DateTimeFormatter; -import java.time.format.FormatStyle; -import java.util.Calendar; -import java.util.Collections; -import java.util.Comparator; -import java.util.List; -import java.util.Optional; - -import static com.adobe.acs.commons.packagegarbagecollector.PackageGarbageCollectionScheduler.*; - @Component( service = JobConsumer.class, immediate = true, @@ -87,24 +88,30 @@ public JobResult process(Job job) { JcrPackageManager packageManager = packaging.getPackageManager(session); List packages = packageManager.listPackages(groupName, false); - for (JcrPackage jcrPackage : packages) { - String packageDescription = getPackageDescription(jcrPackage); - LOG.info("Processing package {}", packageDescription); - - if (isPackageOldEnough(jcrPackage, maxAgeInDays)) { - if (removeNotInstalledPackages && !isInstalled(jcrPackage)) { - packageManager.remove(jcrPackage); - packagesRemoved++; - LOG.info("Deleted not-installed package {}", packageDescription); - } else if (isInstalled(jcrPackage) && !isLatestInstalled(jcrPackage, packageManager.listPackages(groupName, false))) { - packageManager.remove(jcrPackage); - packagesRemoved++; - LOG.info("Deleted installed package {} since it is not the latest installed version.", packageDescription); + for (JcrPackage tmpPackage : packages) { + try (JcrPackage jcrPackage = tmpPackage) { + JcrPackageDefinition definition = jcrPackage.getDefinition(); + if (definition == null) { + LOG.warn("Skipping package without definition: {}", jcrPackage.getNode().getPath()); + } + String packageDescription = getPackageDescription(definition); + LOG.info("Processing package {}", packageDescription); + + if (isPackageOldEnough(definition, maxAgeInDays)) { + if (removeNotInstalledPackages && !isInstalled(definition)) { + packageManager.remove(jcrPackage); + packagesRemoved++; + LOG.info("Deleted not-installed package {}", packageDescription); + } else if (isInstalled(definition) && !isLatestInstalled(definition, packageManager.listPackages(groupName, false).stream())) { + packageManager.remove(jcrPackage); + packagesRemoved++; + LOG.info("Deleted installed package {} since it is not the latest installed version.", packageDescription); + } else { + LOG.info("Not removing package because it's the current installed one {}", packageDescription); + } } else { - LOG.info("Not removing package because it's the current installed one {}", packageDescription); + LOG.debug("Not removing package because it's not old enough {}", packageDescription); } - } else { - LOG.debug("Not removing package because it's not old enough {}", packageDescription); } } } catch (LoginException | RepositoryException | IOException e) { @@ -118,120 +125,84 @@ public JobResult process(Job job) { return JobResult.OK; } - private boolean isInstalled(JcrPackage jcrPackage) { - PackageDefinition definition = new PackageDefinition(jcrPackage); - return definition.getLastUnpacked() != null; + private boolean isInstalled(JcrPackageDefinition pkgDefinition) { + return pkgDefinition.getLastUnpacked() != null; } - private boolean isLatestInstalled(JcrPackage jcrPackage, List installedPackages) { - Optional lastInstalledPackageOptional = installedPackages.stream().filter(installedPackage -> { - PackageDefinition definition = new PackageDefinition(installedPackage); - return definition.isSameNameAndGroup(jcrPackage); - }) - .filter(pkg -> new PackageDefinition(pkg).getLastUnpacked() != null) - .max(Comparator.comparing(pkg -> new PackageDefinition(pkg).getLastUnpacked())); - - if (lastInstalledPackageOptional.isPresent()) { - JcrPackage lastInstalledPackage = lastInstalledPackageOptional.get(); - PackageDefinition lastInstalledPackageDefinition = new PackageDefinition(lastInstalledPackage); - PackageDefinition thisPackageDefinition = new PackageDefinition(jcrPackage); + private static final class UncheckedRepositoryException extends RuntimeException { + private static final long serialVersionUID = 8851421623772855854L; - // If it's not actually installed yet. - if (lastInstalledPackageDefinition.getLastUnpacked() == null) { - // This should never be here since this check is guarded by isInstalled() above. - return false; - } + protected UncheckedRepositoryException(RepositoryException e) { + super(e); + } - return lastInstalledPackageDefinition.hasSamePid(thisPackageDefinition); + /** + * Returns the cause of this exception. + * + * @return the {@code RepositoryException} which is the cause of this exception. + */ + @Override + public RepositoryException getCause() { + return (RepositoryException) super.getCause(); } - return false; } - static class PackageDefinition { - JcrPackage jcrPackage; - - public PackageDefinition(@Nonnull JcrPackage jcrPackage) { - this.jcrPackage = jcrPackage; - } - - public Calendar getLastUnpacked() { - try { - JcrPackageDefinition definition = jcrPackage.getDefinition(); - if (definition != null) { - return definition.getLastUnpacked(); - } - return null; - } catch (RepositoryException ex) { - return null; - } - } + private boolean isLatestInstalled(JcrPackageDefinition referencePkgDefinition, Stream installedPackages) throws RepositoryException { + try { + Optional lastInstalledPckDefinitionOptional = installedPackages + .map(p -> { + try { + return p.getDefinition(); + } catch (RepositoryException e) { + String pckPath; + try { + pckPath = p.getNode().getPath(); + } catch (RepositoryException nestedException) { + pckPath = "Unknown"; + } + throw new UncheckedRepositoryException(new RepositoryException("Cannot read package definition of package " + pckPath, e)); + } + }) + .filter(def -> isSameNameAndGroup(referencePkgDefinition.getId(), def.getId())) + .filter(def -> def.getLastUnpacked() != null) + .max(Comparator.comparing(def -> def.getLastUnpacked())); - public boolean isSameNameAndGroup(JcrPackage otherPackage) { - Optional otherPackageId = getPid(otherPackage); - Optional thisPackageId = getPid(jcrPackage); - if (otherPackageId.isPresent() && thisPackageId.isPresent()) { - return otherPackageId.get().getGroup().equals(thisPackageId.get().getGroup()) - && otherPackageId.get().getName().equals(thisPackageId.get().getName()); + if (lastInstalledPckDefinitionOptional.isPresent()) { + return lastInstalledPckDefinitionOptional.get().getId().equals(referencePkgDefinition.getId()); } return false; + } catch (UncheckedRepositoryException e) { + throw e.getCause(); } + } - public PackageId getId() { - try { - JcrPackageDefinition definition = jcrPackage.getDefinition(); - if (definition != null) { - return definition.getId(); - } - return null; - } catch (RepositoryException ex) { - return null; - } - } - - private Optional getPid(JcrPackage jcrPkg) { - try { - return Optional.ofNullable(jcrPkg.getDefinition()).map(JcrPackageDefinition::getId); - } catch (RepositoryException ex) { - return Optional.empty(); - } - } - - public boolean hasSamePid(PackageDefinition jcrPkg) { - try { - Optional pkgId = Optional.ofNullable(jcrPkg.getId()); - return pkgId.map(packageId -> packageId.equals(getId())).orElse(false); - } catch (NullPointerException ex) { - return false; - } - } + public static boolean isSameNameAndGroup(PackageId thisPackageId, PackageId otherPackageId){ + return otherPackageId.getGroup().equals(thisPackageId.getGroup()) + && otherPackageId.getName().equals(thisPackageId.getName()); } - private boolean isPackageOldEnough(JcrPackage jcrPackage, Integer maxAgeInDays) throws RepositoryException, IOException { + private boolean isPackageOldEnough(JcrPackageDefinition pkgDefinition, Integer maxAgeInDays) throws RepositoryException, IOException { Period maxAge = Period.ofDays(maxAgeInDays); LocalDate oldestAge = LocalDate.now().minus(maxAge); - Calendar packageCreatedAtCalendar = jcrPackage.getPackage().getCreated(); + final Calendar packageCreatedAtCalendar; try { + packageCreatedAtCalendar = pkgDefinition.getCreated(); if (packageCreatedAtCalendar == null) { - // Try getting the created at directly from the JCR node that represents the package. - packageCreatedAtCalendar = jcrPackage.getDefinition().getNode().getProperty(JcrConstants.JCR_CREATED).getValue().getDate(); - - if (packageCreatedAtCalendar == null) { - // This should not happen, but if it does, we don't want to delete the package. - LOG.warn("Package [ {} ] has no created date, assuming it's NOT old enough", jcrPackage.getNode().getPath()); - return false; - } + // This should not happen, but if it does, we don't want to delete the package. + LOG.warn("Package [ {} ] has no created date, assuming it's NOT old enough", pkgDefinition.getNode().getPath()); + return false; } } catch (RepositoryException e) { - LOG.error("Unable to get created date for package [ {} ]", jcrPackage.getNode().getPath(), e); + LOG.error("Unable to get created date for package [ {} ]", pkgDefinition.getNode().getPath(), e); return false; } LocalDate packageCreatedAt = LocalDateTime.ofInstant( packageCreatedAtCalendar.toInstant(), packageCreatedAtCalendar.getTimeZone().toZoneId()).toLocalDate(); - String packageDescription = getPackageDescription(jcrPackage); + String packageDescription = getPackageDescription(pkgDefinition); if (LOG.isDebugEnabled()) { LOG.debug("Checking if package is old enough: Name: {}, Created At: {}, Oldest Age: {}", @@ -240,12 +211,7 @@ private boolean isPackageOldEnough(JcrPackage jcrPackage, Integer maxAgeInDays) return !packageCreatedAt.isAfter(oldestAge); } - private String getPackageDescription(JcrPackage jcrPackage) throws RepositoryException { - JcrPackageDefinition definition = jcrPackage.getDefinition(); - Node packageNode = jcrPackage.getNode(); - if (definition != null && packageNode != null) { - return String.format("%s:%s:v%s [%s]", definition.getId().getName(), definition.getId().getGroup(), definition.getId().getVersionString(), packageNode.getPath()); - } - return "Unknown package"; + private String getPackageDescription(JcrPackageDefinition definition) throws RepositoryException { + return String.format("%s:%s:v%s [%s]", definition.getId().getName(), definition.getId().getGroup(), definition.getId().getVersionString(), definition.getNode().getPath()); } } diff --git a/bundle/src/test/java/com/adobe/acs/commons/packagegarbagecollector/PackageGarbageCollectionJobTest.java b/bundle/src/test/java/com/adobe/acs/commons/packagegarbagecollector/PackageGarbageCollectionJobTest.java index 70240c41cd..0829e249c4 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/packagegarbagecollector/PackageGarbageCollectionJobTest.java +++ b/bundle/src/test/java/com/adobe/acs/commons/packagegarbagecollector/PackageGarbageCollectionJobTest.java @@ -26,7 +26,6 @@ import org.apache.jackrabbit.vault.packaging.JcrPackageManager; import org.apache.jackrabbit.vault.packaging.PackageId; import org.apache.jackrabbit.vault.packaging.Packaging; -import org.apache.jackrabbit.vault.packaging.VaultPackage; import org.apache.sling.event.jobs.Job; import org.apache.sling.event.jobs.consumer.JobConsumer; import org.apache.sling.testing.mock.sling.ResourceResolverType; @@ -130,14 +129,12 @@ void mockPackageManager(JcrPackage... jcrPackage) throws RepositoryException { JcrPackage mockPackage(Integer daysAgo, Integer lastUnpackedDaysAgo, String packageName, String name, String version) throws RepositoryException, IOException { JcrPackage jcrPackage = mock(JcrPackage.class); - VaultPackage vaultPackage = mock(VaultPackage.class); - when(jcrPackage.getPackage()).thenReturn(vaultPackage); - when(vaultPackage.getCreated()).thenReturn(getDate(daysAgo)); Node packageNode = mock(Node.class); when(packageNode.getPath()).thenReturn("/etc/packages/" + packageName); - when(jcrPackage.getNode()).thenReturn(packageNode); JcrPackageDefinition definition = mock(JcrPackageDefinition.class); when(definition.getLastUnpacked()).thenReturn(getDate(lastUnpackedDaysAgo)); + when(definition.getCreated()).thenReturn(getDate(daysAgo)); + when(definition.getNode()).thenReturn(packageNode); PackageId pid = mock(PackageId.class); when(pid.getName()).thenReturn(name); when(pid.getGroup()).thenReturn("com.acs");