From c7796234a0cba030d454b8118ee3ddd09687a2b1 Mon Sep 17 00:00:00 2001 From: Ian Allen Date: Mon, 14 Oct 2024 08:39:56 -0300 Subject: [PATCH] Add better logging when resources are deleted to make it clear what metadata record the resource was deleted from. Also renamed "MetadataResource" in the message to be "Metadata resource" --- .../records/attachments/FilesystemStore.java | 22 ++++++++++++----- .../api/records/attachments/CMISStore.java | 12 ++++------ .../api/records/attachments/JCloudStore.java | 24 +++++++++---------- .../api/records/attachments/S3Store.java | 16 +++++++++---- 4 files changed, 43 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/org/fao/geonet/api/records/attachments/FilesystemStore.java b/core/src/main/java/org/fao/geonet/api/records/attachments/FilesystemStore.java index fb0577bc8bde..b721b0572d0d 100644 --- a/core/src/main/java/org/fao/geonet/api/records/attachments/FilesystemStore.java +++ b/core/src/main/java/org/fao/geonet/api/records/attachments/FilesystemStore.java @@ -234,20 +234,26 @@ public String delResources(ServiceContext context, int metadataId) throws Except try { Log.info(Geonet.RESOURCES, String.format("Deleting all files from metadataId '%d'", metadataId)); IO.deleteFileOrDirectory(metadataDir, true); - return String.format("Metadata '%s' directory removed.", metadataId); + Log.warning(Geonet.RESOURCES, + String.format("Metadata '%d' directory removed.", metadataId)); + return String.format("Metadata '%d' directory removed.", metadataId); } catch (Exception e) { - return String.format("Unable to remove metadata '%s' directory.", metadataId); + return String.format("Unable to remove metadata '%d' directory.", metadataId); } } @Override public String delResource(ServiceContext context, String metadataUuid, String resourceId, Boolean approved) throws Exception { - canEdit(context, metadataUuid, approved); + int metadataId = canEdit(context, metadataUuid, approved); try (ResourceHolder filePath = getResource(context, metadataUuid, resourceId, approved)) { Files.deleteIfExists(filePath.getPath()); - return String.format("MetadataResource '%s' removed.", resourceId); + Log.info(Geonet.RESOURCES, + String.format("Resource '%s' removed for metadata %d (%s).", resourceId, metadataId, metadataUuid)); + return String.format("Metadata resource '%s' removed.", resourceId); } catch (IOException e) { + Log.info(Geonet.RESOURCES, + String.format("Unable to remove resource '%s' for metadata %d (%s). %s", resourceId, metadataId, metadataUuid), e.getMessage()); return String.format("Unable to remove resource '%s'.", resourceId); } } @@ -255,12 +261,16 @@ public String delResource(ServiceContext context, String metadataUuid, String re @Override public String delResource(final ServiceContext context, final String metadataUuid, final MetadataResourceVisibility visibility, final String resourceId, Boolean approved) throws Exception { - canEdit(context, metadataUuid, approved); + int metadataId = canEdit(context, metadataUuid, approved); try (ResourceHolder filePath = getResource(context, metadataUuid, visibility, resourceId, approved)) { Files.deleteIfExists(filePath.getPath()); - return String.format("MetadataResource '%s' removed.", resourceId); + Log.info(Geonet.RESOURCES, + String.format("Resource '%s' removed for metadata %d (%s).", resourceId, metadataId, metadataUuid)); + return String.format("Metadata resource '%s' removed.", resourceId); } catch (IOException e) { + Log.info(Geonet.RESOURCES, + String.format("Unable to remove resource '%s' for metadata %d (%s). %s", resourceId, metadataId, metadataUuid), e.getMessage()); return String.format("Unable to remove resource '%s'.", resourceId); } } diff --git a/datastorages/cmis/src/main/java/org/fao/geonet/api/records/attachments/CMISStore.java b/datastorages/cmis/src/main/java/org/fao/geonet/api/records/attachments/CMISStore.java index 37fe85018994..50c6e5360029 100644 --- a/datastorages/cmis/src/main/java/org/fao/geonet/api/records/attachments/CMISStore.java +++ b/datastorages/cmis/src/main/java/org/fao/geonet/api/records/attachments/CMISStore.java @@ -424,13 +424,9 @@ public String delResource(final ServiceContext context, final String metadataUui for (MetadataResourceVisibility visibility : MetadataResourceVisibility.values()) { if (tryDelResource(context, metadataUuid, metadataId, visibility, resourceId)) { - Log.info(Geonet.RESOURCES, - String.format("MetadataResource '%s' removed.", resourceId)); return String.format("MetadataResource '%s' removed.", resourceId); } } - Log.info(Geonet.RESOURCES, - String.format("Unable to remove resource '%s'.", resourceId)); return String.format("Unable to remove resource '%s'.", resourceId); } @@ -439,12 +435,8 @@ public String delResource(final ServiceContext context, final String metadataUui final String resourceId, Boolean approved) throws Exception { int metadataId = canEdit(context, metadataUuid, approved); if (tryDelResource(context, metadataUuid, metadataId, visibility, resourceId)) { - Log.info(Geonet.RESOURCES, - String.format("MetadataResource '%s' removed.", resourceId)); return String.format("MetadataResource '%s' removed.", resourceId); } - Log.info(Geonet.RESOURCES, - String.format("Unable to remove resource '%s'.", resourceId)); return String.format("Unable to remove resource '%s'.", resourceId); } @@ -459,6 +451,8 @@ protected boolean tryDelResource(final ServiceContext context, final String meta try { final CmisObject object = cmisConfiguration.getClient().getObjectByPath(key, oc); object.delete(); + Log.info(Geonet.RESOURCES, + String.format("Resource '%s' removed for metadata %d (%s).", resourceId, metadataId, metadataUuid)); if (object instanceof Folder) { cmisUtils.invalidateFolderCacheItem(key); } @@ -467,6 +461,8 @@ protected boolean tryDelResource(final ServiceContext context, final String meta //CmisPermissionDeniedException when user does not have permissions. //CmisConstraintException when there is a lock on the file from a checkout. } catch (CmisObjectNotFoundException | CmisPermissionDeniedException | CmisConstraintException e) { + Log.info(Geonet.RESOURCES, + String.format("Unable to remove resource '%s' for metadata %d (%s). Not Found", resourceId, metadataId, metadataUuid)); return false; } } diff --git a/datastorages/jcloud/src/main/java/org/fao/geonet/api/records/attachments/JCloudStore.java b/datastorages/jcloud/src/main/java/org/fao/geonet/api/records/attachments/JCloudStore.java index 835dc3051c47..0c02ceb67418 100644 --- a/datastorages/jcloud/src/main/java/org/fao/geonet/api/records/attachments/JCloudStore.java +++ b/datastorages/jcloud/src/main/java/org/fao/geonet/api/records/attachments/JCloudStore.java @@ -356,11 +356,13 @@ public String delResources(final ServiceContext context, final int metadataId) t } marker = page.getNextMarker(); } while (marker != null); - return String.format("Metadata '%s' directory removed.", metadataId); + Log.warning(Geonet.RESOURCES, + String.format("Metadata '%d' directory removed.", metadataId)); + return String.format("Metadata '%d' directory removed.", metadataId); } catch (ContainerNotFoundException e) { Log.warning(Geonet.RESOURCES, - String.format("Unable to located metadata '%s' directory to be removed.", metadataId)); - return String.format("Unable to located metadata '%s' directory to be removed.", metadataId); + String.format("Unable to located metadata '%d' directory to be removed.", metadataId)); + return String.format("Unable to located metadata '%d' directory to be removed.", metadataId); } } @@ -371,13 +373,9 @@ public String delResource(final ServiceContext context, final String metadataUui for (MetadataResourceVisibility visibility : MetadataResourceVisibility.values()) { if (tryDelResource(context, metadataUuid, metadataId, visibility, resourceId)) { - Log.info(Geonet.RESOURCES, - String.format("MetadataResource '%s' removed.", resourceId)); - return String.format("MetadataResource '%s' removed.", resourceId); + return String.format("Metadata resource '%s' removed.", resourceId); } } - Log.info(Geonet.RESOURCES, - String.format("Unable to remove resource '%s'.", resourceId)); return String.format("Unable to remove resource '%s'.", resourceId); } @@ -386,12 +384,8 @@ public String delResource(final ServiceContext context, final String metadataUui final String resourceId, Boolean approved) throws Exception { int metadataId = canEdit(context, metadataUuid, approved); if (tryDelResource(context, metadataUuid, metadataId, visibility, resourceId)) { - Log.info(Geonet.RESOURCES, - String.format("MetadataResource '%s' removed.", resourceId)); - return String.format("MetadataResource '%s' removed.", resourceId); + return String.format("Metadata resource '%s' removed.", resourceId); } - Log.info(Geonet.RESOURCES, - String.format("Unable to remove resource '%s'.", resourceId)); return String.format("Unable to remove resource '%s'.", resourceId); } @@ -401,8 +395,12 @@ protected boolean tryDelResource(final ServiceContext context, final String meta if (jCloudConfiguration.getClient().getBlobStore().blobExists(jCloudConfiguration.getContainerName(), key)) { jCloudConfiguration.getClient().getBlobStore().removeBlob(jCloudConfiguration.getContainerName(), key); + Log.info(Geonet.RESOURCES, + String.format("Resource '%s' removed for metadata %d (%s).", resourceId, metadataId, metadataUuid)); return true; } + Log.info(Geonet.RESOURCES, + String.format("Unable to remove resource '%s' for metadata %d (%s).", resourceId, metadataId, metadataUuid)); return false; } diff --git a/datastorages/s3/src/main/java/org/fao/geonet/api/records/attachments/S3Store.java b/datastorages/s3/src/main/java/org/fao/geonet/api/records/attachments/S3Store.java index 27df07a45325..9ff2fb8a37be 100644 --- a/datastorages/s3/src/main/java/org/fao/geonet/api/records/attachments/S3Store.java +++ b/datastorages/s3/src/main/java/org/fao/geonet/api/records/attachments/S3Store.java @@ -193,9 +193,13 @@ public String delResources(final ServiceContext context, final int metadataId) t for (S3ObjectSummary object: objects.getObjectSummaries()) { s3.getClient().deleteObject(s3.getBucket(), object.getKey()); } - return String.format("Metadata '%s' directory removed.", metadataId); + Log.warning(Geonet.RESOURCES, + String.format("Metadata '%d' directory removed.", metadataId)); + return String.format("Metadata '%d' directory removed.", metadataId); } catch (AmazonServiceException e) { - return String.format("Unable to remove metadata '%s' directory.", metadataId); + Log.warning(Geonet.RESOURCES, + String.format("Unable to remove metadata '%d' directory. %s", metadataId, e.getMessage())); + return String.format("Unable to remove metadata '%d' directory.", metadataId); } } @@ -206,7 +210,7 @@ public String delResource(final ServiceContext context, final String metadataUui for (MetadataResourceVisibility visibility: MetadataResourceVisibility.values()) { if (tryDelResource(metadataUuid, metadataId, visibility, resourceId)) { - return String.format("MetadataResource '%s' removed.", resourceId); + return String.format("Metadata resource '%s' removed.", resourceId); } } return String.format("Unable to remove resource '%s'.", resourceId); @@ -217,7 +221,7 @@ public String delResource(final ServiceContext context, final String metadataUui final String resourceId, Boolean approved) throws Exception { int metadataId = canEdit(context, metadataUuid, approved); if (tryDelResource(metadataUuid, metadataId, visibility, resourceId)) { - return String.format("MetadataResource '%s' removed.", resourceId); + return String.format("Metadata resource '%s' removed.", resourceId); } return String.format("Unable to remove resource '%s'.", resourceId); } @@ -227,8 +231,12 @@ private boolean tryDelResource(final String metadataUuid, final int metadataId, final String key = getKey(metadataUuid, metadataId, visibility, resourceId); if (s3.getClient().doesObjectExist(s3.getBucket(), key)) { s3.getClient().deleteObject(s3.getBucket(), key); + Log.info(Geonet.RESOURCES, + String.format("Resource '%s' removed for metadata %d (%s).", resourceId, metadataId, metadataUuid)); return true; } + Log.info(Geonet.RESOURCES, + String.format("Unable to remove resource '%s' for metadata %d (%s).", resourceId, metadataId, metadataUuid)); return false; }