diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d31d683ee..adac360512 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com) ## Unreleased ([details][unreleased changes details]) +### Fix + +- #3246 - PackageGarbageCollector is not cleaning up all packages since v6.3.4 (##3225) + ## 6.3.4 - 2023-01-17 - #3223 - Project with class extending WCMUsePojo leads to build error: cannot access aQute.bnd.annotation.ConsumerType @@ -22,7 +26,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com) - #3162 - Renovator MCP: ensure old source path is removed - #3205 - HttpClientFactory: Expose a method to customize the underlying HttpClient - #3209 - WARN org.apache.sling.models.impl.ModelAdapterFactory - Cannot provide default for java.util.List -- #3197 - Encrypt user credentials in ACS Content Sync +- #3197 - Encrypt user credentials in ACS Content Sync - #3196 - Content Sync: prevent exception when creating parent nodes - #3194 - Redirect Manager: Ignore Case value is not persisting @@ -43,18 +47,20 @@ The format is based on [Keep a Changelog](http://keepachangelog.com) ## Removed - #3183 - Removed .wrap package including JackrabbitSessionIWrap and related classes which is no longer supported in Cloud Manager pipelines. - + ## 6.1.0 - 2023-09-08 ## Added - #3159 - Add PageProperty annotation for Sling Models - #3170 - Added a new MCP tool to bulk tag AEM content pages via an Excel file input. + ## Fixed - #3147 - Fixed setting initial content-type when importing CFs from a spreadsheet - #3040 - Fixed bug where namespaced multi-fields would have the namespace 2 times -- #3140 - Fixed issue where malformed MCP process nodes can cause a NPE that breaks the entire MPC reporting UI. Now displays more friendly values in UI to help remove the invalid nodes. +- #3140 - Fixed issue where malformed MCP process nodes can cause a NPE that breaks the entire MPC reporting UI. Now + displays more friendly values in UI to help remove the invalid nodes. - #3150 - Support for case-insensitive redirect rules ( [NC] flag equivalent of apache) - #3138 - Re-arrange action removes data from redirect node diff --git a/bundle/src/main/java/com/adobe/acs/commons/packagegarbagecollector/PackageGarbageCollectionConfig.java b/bundle/src/main/java/com/adobe/acs/commons/packagegarbagecollector/PackageGarbageCollectionConfig.java index 41c1b76263..ffd6bb1198 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/packagegarbagecollector/PackageGarbageCollectionConfig.java +++ b/bundle/src/main/java/com/adobe/acs/commons/packagegarbagecollector/PackageGarbageCollectionConfig.java @@ -22,20 +22,20 @@ import org.osgi.service.metatype.annotations.AttributeDefinition; import org.osgi.service.metatype.annotations.ObjectClassDefinition; -@ObjectClassDefinition(name = "ACS Commons - Package Garbage Collection Configuration", description = "Used to config a package garbage collection job") +@ObjectClassDefinition(name = "ACS Commons - Package Garbage Collection Configuration", description = "Used to config a package garbage collection job.") public @interface PackageGarbageCollectionConfig { - @AttributeDefinition(name = "Schedule", description = "Cron expression detailing when the garbage collection is run. Default runs at 02:30 every day") + @AttributeDefinition(name = "Schedule", description = "Cron expression detailing when the garbage collection is run. Default runs at 02:30 every day.") String scheduler() default "0 30 2 ? * * *"; - @AttributeDefinition(name = "Package Group Name", description = "The group name of the packages to remove") + @AttributeDefinition(name = "Package group name", description = "The group name of the packages to remove.") String groupName() default ""; - @AttributeDefinition(name = "Max age of package", description = "Packages older than this (in days) will be removed. Default is 60 days") + @AttributeDefinition(name = "Max upload age of package", description = "Packages uploaded more than the given amount of days ago will be removed. Default is 60 days.") int maxAgeInDays() default 60; - @AttributeDefinition(name = "Remove not installed packages", description = "Remove packages that are not installed (or have been uninstalled) and the created date is older than maxAgeInDays. Default is false.") + @AttributeDefinition(name = "Remove not installed packages", description = "Remove packages that are not installed (or have been uninstalled) and the upload date is older than maxAgeInDays. Default is false.") boolean removeNotInstalledPackages() default false; @AttributeDefinition(name = "webconsole.configurationFactory.nameHint") - String webconsole_configurationFactory_nameHint() default "Package Garbage Collection - Clear packages in {groupName} older than {maxAgeInDays} days using the schedule [{scheduler}]"; + String webconsole_configurationFactory_nameHint() default "Package Garbage Collection - Clear packages in {groupName} uploaded older than {maxAgeInDays} days using the schedule [{scheduler}]"; } 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 d38c865827..0a3710537c 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 @@ -96,7 +96,7 @@ public JobResult process(Job job) { } String packageDescription = getPackageDescription(definition); LOG.info("Processing package {}", packageDescription); - + if (isPackageOldEnough(definition, maxAgeInDays)) { if (removeNotInstalledPackages && !isInstalled(definition)) { packageManager.remove(jcrPackage); @@ -126,6 +126,7 @@ public JobResult process(Job job) { } private boolean isInstalled(JcrPackageDefinition pkgDefinition) { + // lastUnpacked is when the package was installed (aka unpacked) to this AEM/JCR. return pkgDefinition.getLastUnpacked() != null; } @@ -139,7 +140,7 @@ protected UncheckedRepositoryException(RepositoryException e) { /** * Returns the cause of this exception. * - * @return the {@code RepositoryException} which is the cause of this exception. + * @return the {@code RepositoryException} which is the cause of this exception. */ @Override public RepositoryException getCause() { @@ -151,22 +152,22 @@ public RepositoryException getCause() { private boolean isLatestInstalled(JcrPackageDefinition referencePkgDefinition, Stream installedPackages) throws RepositoryException { try { Optional lastInstalledPckDefinitionOptional = installedPackages - .map(p -> { - try { - return p.getDefinition(); - } catch (RepositoryException e) { - String pckPath; + .map(p -> { try { - pckPath = p.getNode().getPath(); - } catch (RepositoryException nestedException) { - pckPath = "Unknown"; + 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)); } - 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())); + }) + .filter(def -> isSameNameAndGroup(referencePkgDefinition.getId(), def.getId())) + .filter(def -> def.getLastUnpacked() != null) + .max(Comparator.comparing(def -> def.getLastUnpacked())); if (lastInstalledPckDefinitionOptional.isPresent()) { return lastInstalledPckDefinitionOptional.get().getId().equals(referencePkgDefinition.getId()); @@ -177,38 +178,43 @@ private boolean isLatestInstalled(JcrPackageDefinition referencePkgDefinition, S } } - public static boolean isSameNameAndGroup(PackageId thisPackageId, PackageId otherPackageId){ - return otherPackageId.getGroup().equals(thisPackageId.getGroup()) - && otherPackageId.getName().equals(thisPackageId.getName()); + public static boolean isSameNameAndGroup(PackageId thisPackageId, PackageId otherPackageId) { + return otherPackageId.getGroup().equals(thisPackageId.getGroup()) + && otherPackageId.getName().equals(thisPackageId.getName()); } + private boolean isPackageOldEnough(JcrPackageDefinition pkgDefinition, Integer maxAgeInDays) throws RepositoryException, IOException { Period maxAge = Period.ofDays(maxAgeInDays); LocalDate oldestAge = LocalDate.now().minus(maxAge); - final Calendar packageCreatedAtCalendar; + // lastUnwrapped is when the package was UPLOADED to this AEM/JCR, lastUnpacked is what it was last INSTALLED! + Calendar packageUploadedDate; try { - packageCreatedAtCalendar = pkgDefinition.getCreated(); - if (packageCreatedAtCalendar == null) { + // getLastUnwrapped() is when the package was introduced (aka uploaded) to this AEM/JCR. + // getCreated() is when the package was created (aka built) by the package manager; which could be years ago. + packageUploadedDate = pkgDefinition.getLastUnwrapped(); + + if (packageUploadedDate == 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", pkgDefinition.getNode().getPath()); + LOG.warn("Package [ {} ] has no lastUnwrapped (uploaded) date, assuming it's NOT old enough", pkgDefinition.getNode().getPath()); return false; } } catch (RepositoryException e) { - LOG.error("Unable to get created date for package [ {} ]", pkgDefinition.getNode().getPath(), e); + LOG.error("Unable to get lastUnwrapped (uploaded) date for package [ {} ]", pkgDefinition.getNode().getPath(), e); return false; } - LocalDate packageCreatedAt = LocalDateTime.ofInstant( - packageCreatedAtCalendar.toInstant(), - packageCreatedAtCalendar.getTimeZone().toZoneId()).toLocalDate(); + LocalDate packageUploadedAt = LocalDateTime.ofInstant( + packageUploadedDate.toInstant(), + packageUploadedDate.getTimeZone().toZoneId()).toLocalDate(); String packageDescription = getPackageDescription(pkgDefinition); if (LOG.isDebugEnabled()) { - LOG.debug("Checking if package is old enough: Name: {}, Created At: {}, Oldest Age: {}", - packageDescription, packageCreatedAt.format(LOCALIZED_DATE_FORMATTER), oldestAge.format(LOCALIZED_DATE_FORMATTER)); + LOG.debug("Checking if package is old enough: Name: {}, Uploaded at: {}, Oldest age: {}", + packageDescription, packageUploadedAt.format(LOCALIZED_DATE_FORMATTER), oldestAge.format(LOCALIZED_DATE_FORMATTER)); } - return !packageCreatedAt.isAfter(oldestAge); + return !packageUploadedAt.isAfter(oldestAge); } private String getPackageDescription(JcrPackageDefinition definition) throws RepositoryException { 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 0829e249c4..d51aec5ddc 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 @@ -133,7 +133,8 @@ JcrPackage mockPackage(Integer daysAgo, Integer lastUnpackedDaysAgo, String pack when(packageNode.getPath()).thenReturn("/etc/packages/" + packageName); JcrPackageDefinition definition = mock(JcrPackageDefinition.class); when(definition.getLastUnpacked()).thenReturn(getDate(lastUnpackedDaysAgo)); - when(definition.getCreated()).thenReturn(getDate(daysAgo)); + when(definition.getLastUnwrapped()).thenReturn(getDate(daysAgo)); + when(definition.getCreated()).thenReturn(null); when(definition.getNode()).thenReturn(packageNode); PackageId pid = mock(PackageId.class); when(pid.getName()).thenReturn(name);