From 2fb5bf85a5ae036f995a723f0e01a472b4534cb3 Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 11:07:40 +0530 Subject: [PATCH 01/16] fix copy attachment with invalid properties --- .../cds/sdm/constants/SDMErrorMessages.java | 4 + .../sap/cds/sdm/constants/SDMUIErrorKeys.java | 4 + .../cds/sdm/model/CopyAttachmentsResult.java | 14 ++ .../handler/SDMCustomServiceHandler.java | 144 +++++++++++++++--- 4 files changed, 147 insertions(+), 19 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java index 14cae539f..189b0496c 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java +++ b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java @@ -142,6 +142,10 @@ private SDMErrorMessages() { "Invalid secondary properties detected: "; public static final String INVALID_SECONDARY_PROPERTIES_FOR_MOVE_SUFFIX = ". Attachment rolled back to source."; + public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX = + "Invalid secondary properties detected: "; + public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX = + ". Attachment not copied."; public static final String SDM_MOVE_OPERATION_FAILED = "SDM move operation failed"; public static final String FAILED_TO_ACCESS_ERROR_KEY_FIELDS = "Failed to access SDM error key fields"; diff --git a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java index 8eead9791..f3796b803 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java +++ b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java @@ -87,6 +87,10 @@ private SDMUIErrorKeys() {} "SDM.invalidSecondaryPropertiesForMovePrefix"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_MOVE_SUFFIX_KEY = "SDM.invalidSecondaryPropertiesForMoveSuffix"; + public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX_KEY = + "SDM.invalidSecondaryPropertiesForCopyPrefix"; + public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX_KEY = + "SDM.invalidSecondaryPropertiesForCopySuffix"; public static final String MAX_COUNT_ERROR_MESSAGE_KEY = "SDM.maxCountErrorMessage"; public static Map getAllUIErrorKeys() { diff --git a/sdm/src/main/java/com/sap/cds/sdm/model/CopyAttachmentsResult.java b/sdm/src/main/java/com/sap/cds/sdm/model/CopyAttachmentsResult.java index 93cf50bfd..f27f834f5 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/model/CopyAttachmentsResult.java +++ b/sdm/src/main/java/com/sap/cds/sdm/model/CopyAttachmentsResult.java @@ -1,5 +1,6 @@ package com.sap.cds.sdm.model; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -7,11 +8,20 @@ public class CopyAttachmentsResult { private final List> attachmentsMetadata; private final List populatedDocuments; + private final List> failedAttachments; public CopyAttachmentsResult( List> attachmentsMetadata, List populatedDocuments) { + this(attachmentsMetadata, populatedDocuments, new ArrayList<>()); + } + + public CopyAttachmentsResult( + List> attachmentsMetadata, + List populatedDocuments, + List> failedAttachments) { this.attachmentsMetadata = attachmentsMetadata; this.populatedDocuments = populatedDocuments; + this.failedAttachments = failedAttachments; } public List> getAttachmentsMetadata() { @@ -21,4 +31,8 @@ public List> getAttachmentsMetadata() { public List getPopulatedDocuments() { return populatedDocuments; } + + public List> getFailedAttachments() { + return failedAttachments; + } } diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index b393a30a9..5abade461 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -150,25 +150,55 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti List> attachmentsMetadata = copyResult.getAttachmentsMetadata(); List populatedDocuments = copyResult.getPopulatedDocuments(); + List> copyFailures = copyResult.getFailedAttachments(); - String upIdKey = resolveUpIdKey(context, parentEntity, compositionName); + // Show warning if there are failures + if (!copyFailures.isEmpty()) { + StringBuilder warningMessage = + new StringBuilder("Failed to copy the following attachments:\n"); + for (Map failure : copyFailures) { + warningMessage + .append(" - ObjectId: ") + .append(failure.get(OBJECT_ID_KEY)) + .append(", Reason: ") + .append(failure.get(FAILURE_REASON_KEY)) + .append("\n"); + } + context.getMessages().warn(warningMessage.toString()); + } - CreateDraftEntriesRequest draftRequest = - CreateDraftEntriesRequest.builder() - .attachmentsMetadata(attachmentsMetadata) - .populatedDocuments(populatedDocuments) - .parentEntity(parentEntity) - .compositionName(compositionName) - .upID(upID) - .upIdKey(upIdKey) - .repositoryId(repositoryId) - .folderId(folderId) - .customPropertyValues(null) - .build(); + String upIdKey = resolveUpIdKey(context, parentEntity, compositionName); - // Pass the entity for type conversion - CdsEntity targetEntity = entity.isPresent() ? entity.get() : null; - createDraftEntries(draftRequest, customPropertyDefinitions, targetEntity); + // Create draft entries if there are successful copies + if (!attachmentsMetadata.isEmpty()) { + CreateDraftEntriesRequest draftRequest = + CreateDraftEntriesRequest.builder() + .attachmentsMetadata(attachmentsMetadata) + .populatedDocuments(populatedDocuments) + .parentEntity(parentEntity) + .compositionName(compositionName) + .upID(upID) + .upIdKey(upIdKey) + .repositoryId(repositoryId) + .folderId(folderId) + .customPropertyValues(null) + .build(); + + // Pass the entity for type conversion + CdsEntity targetEntity = entity.isPresent() ? entity.get() : null; + createDraftEntries(draftRequest, customPropertyDefinitions, targetEntity); + } else if (!folderExists) { + // All copies failed but folder was newly created - delete the empty folder + logger.info( + "All attachments failed to copy. Deleting newly created empty folder: {}", folderId); + try { + sdmService.deleteDocument( + "deleteTree", folderId, context.getUserInfo().getName(), isSystemUser); + logger.info("Deleted empty folder: {}", folderId); + } catch (Exception e) { + logger.error("Failed to delete empty folder {}: {}", folderId, e.getMessage()); + } + } logger.info( "Copy attachments completed - {} attachments copied for upID: {}", @@ -570,6 +600,33 @@ private CopyAttachmentsResult copyAttachmentsToSDM( logger.debug("START: Copy {} attachments to SDM", request.getObjectIds().size()); List> attachmentsMetadata = new ArrayList<>(); List populatedDocuments = new ArrayList<>(); + List> failedAttachments = new ArrayList<>(); + + // Fetch valid secondary properties for validation + List validSecondaryProperties = new ArrayList<>(); + boolean shouldValidateSecondaryProperties = false; + if (!customPropertiesInSDM.isEmpty()) { + try { + List secondaryTypes = + sdmService.getSecondaryTypes( + request.getRepositoryId(), request.getSdmCredentials(), request.getIsSystemUser()); + validSecondaryProperties = + sdmService.getValidSecondaryProperties( + secondaryTypes, + request.getSdmCredentials(), + request.getRepositoryId(), + request.getIsSystemUser()); + shouldValidateSecondaryProperties = true; + logger.debug( + "Fetched {} valid secondary properties for copy validation", + validSecondaryProperties.size()); + } catch (Exception e) { + logger.warn( + "Failed to fetch valid secondary properties for copy validation: {}. " + + "Proceeding without validation.", + e.getMessage()); + } + } for (String objectId : request.getObjectIds()) { logger.debug("Processing copy for objectId: {}", objectId); @@ -584,7 +641,6 @@ private CopyAttachmentsResult copyAttachmentsToSDM( populatedDocument.setType(cmisDocument.getType()); populatedDocument.setUrl(cmisDocument.getUrl()); populatedDocument.setUploadStatus(cmisDocument.getUploadStatus()); - populatedDocuments.add(populatedDocument); try { Map attachmentData = @@ -594,6 +650,53 @@ private CopyAttachmentsResult copyAttachmentsToSDM( request.getIsSystemUser(), customPropertiesInSDM); + // Validate secondary properties after copy (similar to move validation) + // Only validate if we successfully fetched valid secondary properties + List invalidProperties = new ArrayList<>(); + if (shouldValidateSecondaryProperties) { + for (String customProp : customPropertiesInSDM) { + if (attachmentData.containsKey(customProp) + && !validSecondaryProperties.contains(customProp)) { + invalidProperties.add(customProp); + } + } + } + + if (!invalidProperties.isEmpty()) { + // Validation failed - delete the copied attachment and record failure + String copiedObjectId = attachmentData.get("cmis:objectId"); + logger.error( + "Attachment {} validation FAILED - Found {} invalid properties: {}. " + + "Deleting copied attachment...", + objectId, + invalidProperties.size(), + invalidProperties); + try { + sdmService.deleteDocument( + "delete", + copiedObjectId, + request.getContext().getUserInfo().getName(), + request.getIsSystemUser()); + logger.info("Deleted invalid copied attachment: {}", copiedObjectId); + } catch (Exception deleteEx) { + logger.error( + "Failed to delete invalid copied attachment {}: {}", + copiedObjectId, + deleteEx.getMessage()); + } + // Add to failed attachments list instead of throwing exception + Map failure = new HashMap<>(); + failure.put(OBJECT_ID_KEY, objectId); + failure.put( + FAILURE_REASON_KEY, + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") + + String.join(", ", invalidProperties) + + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); + failedAttachments.add(failure); + continue; + } + + populatedDocuments.add(populatedDocument); attachmentsMetadata.add(attachmentData); logger.debug("Successfully copied attachment: {}", objectId); } catch (ServiceException e) { @@ -607,8 +710,11 @@ private CopyAttachmentsResult copyAttachmentsToSDM( } } - logger.debug("END: Copy attachments to SDM - {} successful", attachmentsMetadata.size()); - return new CopyAttachmentsResult(attachmentsMetadata, populatedDocuments); + logger.debug( + "END: Copy attachments to SDM - {} successful, {} failed", + attachmentsMetadata.size(), + failedAttachments.size()); + return new CopyAttachmentsResult(attachmentsMetadata, populatedDocuments, failedAttachments); } /** From b95da2d06d741bcf46c11c975724f0cede95563b Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 11:31:01 +0530 Subject: [PATCH 02/16] sonar fix --- .../handler/SDMCustomServiceHandler.java | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index 5abade461..2ab005dbd 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -671,19 +671,10 @@ private CopyAttachmentsResult copyAttachmentsToSDM( objectId, invalidProperties.size(), invalidProperties); - try { - sdmService.deleteDocument( - "delete", - copiedObjectId, - request.getContext().getUserInfo().getName(), - request.getIsSystemUser()); - logger.info("Deleted invalid copied attachment: {}", copiedObjectId); - } catch (Exception deleteEx) { - logger.error( - "Failed to delete invalid copied attachment {}: {}", - copiedObjectId, - deleteEx.getMessage()); - } + deleteInvalidCopiedAttachment( + copiedObjectId, + request.getContext().getUserInfo().getName(), + request.getIsSystemUser()); // Add to failed attachments list instead of throwing exception Map failure = new HashMap<>(); failure.put(OBJECT_ID_KEY, objectId); @@ -1650,6 +1641,19 @@ private void handleCopyFailure( throw new ServiceException(e.getMessage()); } + private void deleteInvalidCopiedAttachment( + String copiedObjectId, String userName, Boolean isSystemUser) { + try { + sdmService.deleteDocument("delete", copiedObjectId, userName, isSystemUser); + logger.info("Deleted invalid copied attachment: {}", copiedObjectId); + } catch (Exception deleteEx) { + logger.error( + "Failed to delete invalid copied attachment {}: {}", + copiedObjectId, + deleteEx.getMessage()); + } + } + private String resolveUpIdKey(EventContext context, String parentEntity, String compositionName) { logger.debug( "Resolving upIdKey for parentEntity: {}, compositionName: {}", From c567210f607522816a27f9bd651eac17dc72ba14 Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 15:08:09 +0530 Subject: [PATCH 03/16] changes --- .../handler/SDMCustomServiceHandler.java | 105 ++++++++++-------- 1 file changed, 59 insertions(+), 46 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index 2ab005dbd..46ecbe80e 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -630,6 +630,34 @@ private CopyAttachmentsResult copyAttachmentsToSDM( for (String objectId : request.getObjectIds()) { logger.debug("Processing copy for objectId: {}", objectId); + + // Validate secondary properties BEFORE copying using source attachment metadata + if (shouldValidateSecondaryProperties) { + List invalidProperties = + validateSourceAttachmentProperties( + objectId, + customPropertiesInSDM, + validSecondaryProperties, + request.getSdmCredentials(), + request.getIsSystemUser()); + + if (!invalidProperties.isEmpty()) { + logger.warn( + "Attachment {} has invalid secondary properties: {}. Skipping copy.", + objectId, + invalidProperties); + Map failure = new HashMap<>(); + failure.put(OBJECT_ID_KEY, objectId); + failure.put( + FAILURE_REASON_KEY, + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") + + String.join(", ", invalidProperties) + + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); + failedAttachments.add(failure); + continue; + } + } + CmisDocument cmisDocument = dbQuery.getAttachmentForObjectID(persistenceService, objectId, request.getContext()); cmisDocument.setObjectId(objectId); @@ -650,43 +678,6 @@ private CopyAttachmentsResult copyAttachmentsToSDM( request.getIsSystemUser(), customPropertiesInSDM); - // Validate secondary properties after copy (similar to move validation) - // Only validate if we successfully fetched valid secondary properties - List invalidProperties = new ArrayList<>(); - if (shouldValidateSecondaryProperties) { - for (String customProp : customPropertiesInSDM) { - if (attachmentData.containsKey(customProp) - && !validSecondaryProperties.contains(customProp)) { - invalidProperties.add(customProp); - } - } - } - - if (!invalidProperties.isEmpty()) { - // Validation failed - delete the copied attachment and record failure - String copiedObjectId = attachmentData.get("cmis:objectId"); - logger.error( - "Attachment {} validation FAILED - Found {} invalid properties: {}. " - + "Deleting copied attachment...", - objectId, - invalidProperties.size(), - invalidProperties); - deleteInvalidCopiedAttachment( - copiedObjectId, - request.getContext().getUserInfo().getName(), - request.getIsSystemUser()); - // Add to failed attachments list instead of throwing exception - Map failure = new HashMap<>(); - failure.put(OBJECT_ID_KEY, objectId); - failure.put( - FAILURE_REASON_KEY, - SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") - + String.join(", ", invalidProperties) - + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); - failedAttachments.add(failure); - continue; - } - populatedDocuments.add(populatedDocument); attachmentsMetadata.add(attachmentData); logger.debug("Successfully copied attachment: {}", objectId); @@ -1641,17 +1632,39 @@ private void handleCopyFailure( throw new ServiceException(e.getMessage()); } - private void deleteInvalidCopiedAttachment( - String copiedObjectId, String userName, Boolean isSystemUser) { + /** + * Validates source attachment's secondary properties before copying. Fetches attachment metadata + * from SDM and checks if it has properties not supported by the target entity. + * + * @return list of invalid property names, empty if all properties are valid + */ + private List validateSourceAttachmentProperties( + String objectId, + Set customPropertiesInSDM, + List validSecondaryProperties, + SDMCredentials sdmCredentials, + Boolean isSystemUser) { + List invalidProperties = new ArrayList<>(); try { - sdmService.deleteDocument("delete", copiedObjectId, userName, isSystemUser); - logger.info("Deleted invalid copied attachment: {}", copiedObjectId); - } catch (Exception deleteEx) { - logger.error( - "Failed to delete invalid copied attachment {}: {}", - copiedObjectId, - deleteEx.getMessage()); + JSONObject sdmMetadata = sdmService.getObject(objectId, sdmCredentials, isSystemUser); + if (sdmMetadata != null) { + JSONObject succinctProperties = sdmMetadata.optJSONObject("succinctProperties"); + if (succinctProperties != null) { + for (String customProp : customPropertiesInSDM) { + if (succinctProperties.has(customProp) + && !validSecondaryProperties.contains(customProp)) { + invalidProperties.add(customProp); + } + } + } + } + } catch (Exception e) { + logger.warn( + "Failed to fetch source attachment {} for validation: {}. Proceeding with copy.", + objectId, + e.getMessage()); } + return invalidProperties; } private String resolveUpIdKey(EventContext context, String parentEntity, String compositionName) { From 9ff552f8385d15ef10eca845e4cc8d1b97db7be0 Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 15:22:40 +0530 Subject: [PATCH 04/16] removed delete logic --- .../sdm/service/handler/SDMCustomServiceHandler.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index 46ecbe80e..f43a3363b 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -187,17 +187,6 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti // Pass the entity for type conversion CdsEntity targetEntity = entity.isPresent() ? entity.get() : null; createDraftEntries(draftRequest, customPropertyDefinitions, targetEntity); - } else if (!folderExists) { - // All copies failed but folder was newly created - delete the empty folder - logger.info( - "All attachments failed to copy. Deleting newly created empty folder: {}", folderId); - try { - sdmService.deleteDocument( - "deleteTree", folderId, context.getUserInfo().getName(), isSystemUser); - logger.info("Deleted empty folder: {}", folderId); - } catch (Exception e) { - logger.error("Failed to delete empty folder {}: {}", folderId, e.getMessage()); - } } logger.info( From f2c1622f2996964a922416742fe22bcf0def4ef1 Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 16:01:19 +0530 Subject: [PATCH 05/16] fix --- .../cds/sdm/constants/SDMErrorMessages.java | 2 + .../sap/cds/sdm/constants/SDMUIErrorKeys.java | 2 + .../handler/SDMCustomServiceHandler.java | 243 ++++++++---------- .../handler/SDMCustomServiceHandlerTest.java | 164 ++++++++++++ 4 files changed, 279 insertions(+), 132 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java index 189b0496c..a86172b54 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java +++ b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java @@ -142,6 +142,8 @@ private SDMErrorMessages() { "Invalid secondary properties detected: "; public static final String INVALID_SECONDARY_PROPERTIES_FOR_MOVE_SUFFIX = ". Attachment rolled back to source."; + public static final String FAILED_TO_COPY_ATTACHMENTS_PREFIX = + "Failed to copy the following attachments:\n"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX = "Invalid secondary properties detected: "; public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX = diff --git a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java index f3796b803..c98e89c7e 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java +++ b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java @@ -87,6 +87,8 @@ private SDMUIErrorKeys() {} "SDM.invalidSecondaryPropertiesForMovePrefix"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_MOVE_SUFFIX_KEY = "SDM.invalidSecondaryPropertiesForMoveSuffix"; + public static final String FAILED_TO_COPY_ATTACHMENTS_PREFIX_KEY = + "SDM.failedToCopyAttachmentsPrefix"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX_KEY = "SDM.invalidSecondaryPropertiesForCopyPrefix"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX_KEY = diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index f43a3363b..8de802ec6 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -135,30 +135,81 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti List objectIds = context.getObjectIds(); - CopyAttachmentsRequest request = - CopyAttachmentsRequest.builder() - .context(context) - .objectIds(objectIds) - .folderId(folderId) - .repositoryId(repositoryId) - .sdmCredentials(sdmCredentials) - .isSystemUser(isSystemUser) - .folderExists(folderExists) - .build(); + // Pre-copy validation: Check for invalid secondary properties before copying + List validObjectIds = new ArrayList<>(objectIds); + List> copyFailures = new ArrayList<>(); - CopyAttachmentsResult copyResult = copyAttachmentsToSDM(request, customPropertiesInSDM); + if (!customPropertiesInSDM.isEmpty()) { + // Fetch valid secondary properties from SDM + List secondaryTypes = + sdmService.getSecondaryTypes(repositoryId, sdmCredentials, isSystemUser); + List validSecondaryProperties = + sdmService.getValidSecondaryProperties( + secondaryTypes, sdmCredentials, repositoryId, isSystemUser); - List> attachmentsMetadata = copyResult.getAttachmentsMetadata(); - List populatedDocuments = copyResult.getPopulatedDocuments(); - List> copyFailures = copyResult.getFailedAttachments(); + logger.debug( + "Pre-copy validation - checking {} attachments against {} valid secondary properties", + objectIds.size(), + validSecondaryProperties.size()); - // Show warning if there are failures + validObjectIds = new ArrayList<>(); + for (String objectId : objectIds) { + try { + JSONObject sdmMetadata = sdmService.getObject(objectId, sdmCredentials, isSystemUser); + if (sdmMetadata != null && sdmMetadata.has("succinctProperties")) { + JSONObject succinctProperties = sdmMetadata.getJSONObject("succinctProperties"); + Set sdmResponseProperties = new HashSet<>(succinctProperties.keySet()); + + List invalidProperties = new ArrayList<>(); + for (String targetSdmProperty : customPropertiesInSDM) { + if (sdmResponseProperties.contains(targetSdmProperty) + && !validSecondaryProperties.contains(targetSdmProperty)) { + invalidProperties.add(targetSdmProperty); + logger.warn( + "Pre-copy validation - Attachment {} has invalid secondary property '{}'" + + " (present in SDM response but not in valid secondary properties list)", + objectId, + targetSdmProperty); + } + } + + if (!invalidProperties.isEmpty()) { + Map failure = new HashMap<>(); + failure.put(OBJECT_ID_KEY, objectId); + failure.put( + FAILURE_REASON_KEY, + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") + + String.join(", ", invalidProperties) + + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); + copyFailures.add(failure); + logger.warn( + "Pre-copy validation - Skipping attachment {} due to {} invalid secondary properties: {}", + objectId, + invalidProperties.size(), + invalidProperties); + } else { + validObjectIds.add(objectId); + } + } else { + validObjectIds.add(objectId); + } + } catch (IOException e) { + logger.error( + "Pre-copy validation - Failed to fetch metadata for attachment {}: {}", + objectId, + e.getMessage()); + validObjectIds.add(objectId); + } + } + } + + // Show warning if there are attachments with invalid secondary properties if (!copyFailures.isEmpty()) { StringBuilder warningMessage = - new StringBuilder("Failed to copy the following attachments:\n"); + new StringBuilder(SDMUtils.getErrorMessage("FAILED_TO_COPY_ATTACHMENTS_PREFIX")); for (Map failure : copyFailures) { warningMessage - .append(" - ObjectId: ") + .append("- ObjectId: ") .append(failure.get(OBJECT_ID_KEY)) .append(", Reason: ") .append(failure.get(FAILURE_REASON_KEY)) @@ -167,27 +218,48 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti context.getMessages().warn(warningMessage.toString()); } + // If no valid attachments remain after validation, complete early + if (validObjectIds.isEmpty()) { + logger.info("No valid attachments to copy after pre-copy validation for upID: {}", upID); + context.setCompleted(); + logger.debug("END: Copy attachments event"); + return; + } + + CopyAttachmentsRequest request = + CopyAttachmentsRequest.builder() + .context(context) + .objectIds(validObjectIds) + .folderId(folderId) + .repositoryId(repositoryId) + .sdmCredentials(sdmCredentials) + .isSystemUser(isSystemUser) + .folderExists(folderExists) + .build(); + + CopyAttachmentsResult copyResult = copyAttachmentsToSDM(request, customPropertiesInSDM); + + List> attachmentsMetadata = copyResult.getAttachmentsMetadata(); + List populatedDocuments = copyResult.getPopulatedDocuments(); + String upIdKey = resolveUpIdKey(context, parentEntity, compositionName); - // Create draft entries if there are successful copies - if (!attachmentsMetadata.isEmpty()) { - CreateDraftEntriesRequest draftRequest = - CreateDraftEntriesRequest.builder() - .attachmentsMetadata(attachmentsMetadata) - .populatedDocuments(populatedDocuments) - .parentEntity(parentEntity) - .compositionName(compositionName) - .upID(upID) - .upIdKey(upIdKey) - .repositoryId(repositoryId) - .folderId(folderId) - .customPropertyValues(null) - .build(); - - // Pass the entity for type conversion - CdsEntity targetEntity = entity.isPresent() ? entity.get() : null; - createDraftEntries(draftRequest, customPropertyDefinitions, targetEntity); - } + CreateDraftEntriesRequest draftRequest = + CreateDraftEntriesRequest.builder() + .attachmentsMetadata(attachmentsMetadata) + .populatedDocuments(populatedDocuments) + .parentEntity(parentEntity) + .compositionName(compositionName) + .upID(upID) + .upIdKey(upIdKey) + .repositoryId(repositoryId) + .folderId(folderId) + .customPropertyValues(null) + .build(); + + // Pass the entity for type conversion + CdsEntity targetEntity = entity.isPresent() ? entity.get() : null; + createDraftEntries(draftRequest, customPropertyDefinitions, targetEntity); logger.info( "Copy attachments completed - {} attachments copied for upID: {}", @@ -589,64 +661,9 @@ private CopyAttachmentsResult copyAttachmentsToSDM( logger.debug("START: Copy {} attachments to SDM", request.getObjectIds().size()); List> attachmentsMetadata = new ArrayList<>(); List populatedDocuments = new ArrayList<>(); - List> failedAttachments = new ArrayList<>(); - - // Fetch valid secondary properties for validation - List validSecondaryProperties = new ArrayList<>(); - boolean shouldValidateSecondaryProperties = false; - if (!customPropertiesInSDM.isEmpty()) { - try { - List secondaryTypes = - sdmService.getSecondaryTypes( - request.getRepositoryId(), request.getSdmCredentials(), request.getIsSystemUser()); - validSecondaryProperties = - sdmService.getValidSecondaryProperties( - secondaryTypes, - request.getSdmCredentials(), - request.getRepositoryId(), - request.getIsSystemUser()); - shouldValidateSecondaryProperties = true; - logger.debug( - "Fetched {} valid secondary properties for copy validation", - validSecondaryProperties.size()); - } catch (Exception e) { - logger.warn( - "Failed to fetch valid secondary properties for copy validation: {}. " - + "Proceeding without validation.", - e.getMessage()); - } - } for (String objectId : request.getObjectIds()) { logger.debug("Processing copy for objectId: {}", objectId); - - // Validate secondary properties BEFORE copying using source attachment metadata - if (shouldValidateSecondaryProperties) { - List invalidProperties = - validateSourceAttachmentProperties( - objectId, - customPropertiesInSDM, - validSecondaryProperties, - request.getSdmCredentials(), - request.getIsSystemUser()); - - if (!invalidProperties.isEmpty()) { - logger.warn( - "Attachment {} has invalid secondary properties: {}. Skipping copy.", - objectId, - invalidProperties); - Map failure = new HashMap<>(); - failure.put(OBJECT_ID_KEY, objectId); - failure.put( - FAILURE_REASON_KEY, - SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") - + String.join(", ", invalidProperties) - + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); - failedAttachments.add(failure); - continue; - } - } - CmisDocument cmisDocument = dbQuery.getAttachmentForObjectID(persistenceService, objectId, request.getContext()); cmisDocument.setObjectId(objectId); @@ -658,6 +675,7 @@ private CopyAttachmentsResult copyAttachmentsToSDM( populatedDocument.setType(cmisDocument.getType()); populatedDocument.setUrl(cmisDocument.getUrl()); populatedDocument.setUploadStatus(cmisDocument.getUploadStatus()); + populatedDocuments.add(populatedDocument); try { Map attachmentData = @@ -667,7 +685,6 @@ private CopyAttachmentsResult copyAttachmentsToSDM( request.getIsSystemUser(), customPropertiesInSDM); - populatedDocuments.add(populatedDocument); attachmentsMetadata.add(attachmentData); logger.debug("Successfully copied attachment: {}", objectId); } catch (ServiceException e) { @@ -681,11 +698,8 @@ private CopyAttachmentsResult copyAttachmentsToSDM( } } - logger.debug( - "END: Copy attachments to SDM - {} successful, {} failed", - attachmentsMetadata.size(), - failedAttachments.size()); - return new CopyAttachmentsResult(attachmentsMetadata, populatedDocuments, failedAttachments); + logger.debug("END: Copy attachments to SDM - {} successful", attachmentsMetadata.size()); + return new CopyAttachmentsResult(attachmentsMetadata, populatedDocuments); } /** @@ -1621,41 +1635,6 @@ private void handleCopyFailure( throw new ServiceException(e.getMessage()); } - /** - * Validates source attachment's secondary properties before copying. Fetches attachment metadata - * from SDM and checks if it has properties not supported by the target entity. - * - * @return list of invalid property names, empty if all properties are valid - */ - private List validateSourceAttachmentProperties( - String objectId, - Set customPropertiesInSDM, - List validSecondaryProperties, - SDMCredentials sdmCredentials, - Boolean isSystemUser) { - List invalidProperties = new ArrayList<>(); - try { - JSONObject sdmMetadata = sdmService.getObject(objectId, sdmCredentials, isSystemUser); - if (sdmMetadata != null) { - JSONObject succinctProperties = sdmMetadata.optJSONObject("succinctProperties"); - if (succinctProperties != null) { - for (String customProp : customPropertiesInSDM) { - if (succinctProperties.has(customProp) - && !validSecondaryProperties.contains(customProp)) { - invalidProperties.add(customProp); - } - } - } - } - } catch (Exception e) { - logger.warn( - "Failed to fetch source attachment {} for validation: {}. Proceeding with copy.", - objectId, - e.getMessage()); - } - return invalidProperties; - } - private String resolveUpIdKey(EventContext context, String parentEntity, String compositionName) { logger.debug( "Resolving upIdKey for parentEntity: {}, compositionName: {}", diff --git a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java index 437410755..caf9a5874 100644 --- a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java +++ b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java @@ -325,6 +325,170 @@ void testCopyAttachments_AttachmentCopyFails_FolderExists_AttachmentsDeleted() assertTrue(ex.getMessage().contains("Copy failed")); } + @Test + void testCopyAttachments_InvalidSecondaryProperty_SkipsAttachment() throws IOException { + // Mock SDMCredentials + SDMCredentials sdmCredentials = mock(SDMCredentials.class); + when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + + // Mock folder id retrieval + when(sdmService.getFolderIdByPath(any(), any(), any(), anyBoolean())).thenReturn(FOLDER_ID); + + // Create mock context with custom property annotations on the entity + AttachmentCopyEventContext context = createMockContextWithCustomProperties(); + when(context.getObjectIds()).thenReturn(List.of(OBJECT_ID)); + + // Mock secondary types and valid secondary properties from SDM + when(sdmService.getSecondaryTypes(any(), any(), anyBoolean())) + .thenReturn(List.of("secondaryType1")); + when(sdmService.getValidSecondaryProperties(any(), any(), any(), anyBoolean())) + .thenReturn(List.of("validProp1", "validProp2")); + + // Mock getObject: SDM response contains "invalidCustomProp" which is NOT in valid list + JSONObject sdmMetadata = new JSONObject(); + JSONObject succinctProperties = new JSONObject(); + succinctProperties.put("cmis:name", "test.pdf"); + succinctProperties.put("invalidCustomProp", "someValue"); + sdmMetadata.put("succinctProperties", succinctProperties); + when(sdmService.getObject(eq(OBJECT_ID), any(), anyBoolean())).thenReturn(sdmMetadata); + + // Act + sdmCustomServiceHandler.copyAttachments(context); + + // Assert: attachment should be skipped, no copy operation performed + verify(sdmService, never()) + .copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any()); + // Warning message should be issued + verify(context.getMessages(), times(1)).warn(any(String.class)); + verify(context, times(1)).setCompleted(); + } + + @Test + void testCopyAttachments_MixedValidAndInvalidSecondaryProps_CopiesOnlyValid() throws IOException { + // Mock SDMCredentials + SDMCredentials sdmCredentials = mock(SDMCredentials.class); + when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + + // Mock folder id retrieval + when(sdmService.getFolderIdByPath(any(), any(), any(), anyBoolean())).thenReturn(FOLDER_ID); + + String validObjectId = "validObjectId"; + String invalidObjectId = "invalidObjectId"; + + // Create mock context with custom property annotations + AttachmentCopyEventContext context = createMockContextWithCustomProperties(); + when(context.getObjectIds()).thenReturn(List.of(validObjectId, invalidObjectId)); + + // Mock secondary types and valid secondary properties + when(sdmService.getSecondaryTypes(any(), any(), anyBoolean())) + .thenReturn(List.of("secondaryType1")); + // "invalidCustomProp" is NOT in the valid list - only "validProp1" and "validProp2" are valid + when(sdmService.getValidSecondaryProperties(any(), any(), any(), anyBoolean())) + .thenReturn(List.of("validProp1", "validProp2")); + + // Mock getObject for valid attachment (does NOT have "invalidCustomProp" in response) + JSONObject validMetadata = new JSONObject(); + JSONObject validProps = new JSONObject(); + validProps.put("cmis:name", "valid.pdf"); + validProps.put("validProp1", "someValue"); // only valid properties present + validMetadata.put("succinctProperties", validProps); + when(sdmService.getObject(eq(validObjectId), any(), anyBoolean())).thenReturn(validMetadata); + + // Mock getObject for invalid attachment (HAS "invalidCustomProp" in response, which is NOT in + // valid list) + JSONObject invalidMetadata = new JSONObject(); + JSONObject invalidProps = new JSONObject(); + invalidProps.put("cmis:name", "invalid.pdf"); + invalidProps.put("invalidCustomProp", "someValue"); // present in response but NOT in valid list + invalidMetadata.put("succinctProperties", invalidProps); + when(sdmService.getObject(eq(invalidObjectId), any(), anyBoolean())) + .thenReturn(invalidMetadata); + + // Mock attachment copy for valid attachment + Map attachmentData = new HashMap<>(); + attachmentData.put("cmis:name", "valid.pdf"); + attachmentData.put("cmis:contentStreamMimeType", "application/pdf"); + attachmentData.put("cmis:objectId", validObjectId); + when(sdmService.copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any())) + .thenReturn(attachmentData); + CmisDocument cmisDocument = new CmisDocument(); + cmisDocument.setType("sap-icon://document"); + when(dbQuery.getAttachmentForObjectID(any(), any(), any(AttachmentCopyEventContext.class))) + .thenReturn(cmisDocument); + + // Act + sdmCustomServiceHandler.copyAttachments(context); + + // Assert: only valid attachment should be copied + verify(sdmService, times(1)) + .copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any()); + verify(context, times(1)).setCompleted(); + } + + private AttachmentCopyEventContext createMockContextWithCustomProperties() { + AttachmentCopyEventContext context = mock(AttachmentCopyEventContext.class); + CdsElement mockAssociationElement = mock(CdsElement.class); + CdsAssociationType mockAssociationType = mock(CdsAssociationType.class); + CqnElementRef mockCqnElementRef = mock(CqnElementRef.class); + + when(context.getParentEntity()).thenReturn("prefix.someIdentifier." + FACET); + when(context.getCompositionName()).thenReturn(FACET); + when(context.getUpId()).thenReturn(UP_ID); + when(context.getSystemUser()).thenReturn(true); + when(context.getObjectIds()).thenReturn(List.of(OBJECT_ID)); + + // Mock CdsModel and relevant entities and associations + CdsModel model = mock(CdsModel.class); + CdsEntity parentEntity = mock(CdsEntity.class); + CdsEntity draftEntity = mock(CdsEntity.class); + CdsEntity targetEntity = mock(CdsEntity.class); + CdsEntity compositionEntity = mock(CdsEntity.class); + + // Mock composition element and its type + CdsElement compositionElement = mock(CdsElement.class); + CdsAssociationType compositionType = mock(CdsAssociationType.class); + + // Setup expected behavior for model and parent entity + when(context.getModel()).thenReturn(model); + when(model.findEntity("prefix.someIdentifier." + FACET)).thenReturn(Optional.of(parentEntity)); + when(model.findEntity("prefix.someIdentifier." + FACET + "." + FACET)) + .thenReturn(Optional.of(compositionEntity)); + when(model.findEntity(endsWith("_drafts"))).thenReturn(Optional.of(draftEntity)); + + // Mock the composition entity with a custom property annotation + CdsElement customPropertyElement = mock(CdsElement.class); + com.sap.cds.reflect.CdsAnnotation nameAnnotation = + mock(com.sap.cds.reflect.CdsAnnotation.class); + com.sap.cds.reflect.CdsType elementType = mock(com.sap.cds.reflect.CdsType.class); + when(elementType.isAssociation()).thenReturn(false); + when(customPropertyElement.getType()).thenReturn(elementType); + when(customPropertyElement.getName()).thenReturn("customField"); + when(customPropertyElement.findAnnotation("SDM.Attachments.AdditionalProperty.name")) + .thenReturn(Optional.of(nameAnnotation)); + when(nameAnnotation.getValue()).thenReturn("invalidCustomProp"); + when(compositionEntity.elements()).thenReturn(Stream.of(customPropertyElement)); + + // Mock the composition element in parent entity + when(parentEntity.findElement(FACET)).thenReturn(Optional.of(compositionElement)); + when(compositionElement.getType()).thenReturn(compositionType); + when(compositionType.isAssociation()).thenReturn(true); + when(compositionType.getTarget()).thenReturn(targetEntity); + when(targetEntity.getQualifiedName()).thenReturn("target.entity.name"); + + // Mock the draft entity's up_ association + when(draftEntity.findAssociation("up_")).thenReturn(Optional.of(mockAssociationElement)); + when(mockAssociationElement.getType()).thenReturn(mockAssociationType); + when(mockAssociationType.refs()).thenReturn(Stream.of(mockCqnElementRef)); + when(mockCqnElementRef.path()).thenReturn("ID"); + + // Mock messages + com.sap.cds.services.messages.Messages messages = + mock(com.sap.cds.services.messages.Messages.class); + when(context.getMessages()).thenReturn(messages); + + return context; + } + private AttachmentCopyEventContext createMockContext() { AttachmentCopyEventContext context = mock(AttachmentCopyEventContext.class); CdsElement mockAssociationElement = mock(CdsElement.class); From c732f5f5d3a239674aa77b259468dcb393aa9a80 Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 16:03:02 +0530 Subject: [PATCH 06/16] Revert "fix" This reverts commit f2c1622f2996964a922416742fe22bcf0def4ef1. --- .../cds/sdm/constants/SDMErrorMessages.java | 2 - .../sap/cds/sdm/constants/SDMUIErrorKeys.java | 2 - .../handler/SDMCustomServiceHandler.java | 243 ++++++++++-------- .../handler/SDMCustomServiceHandlerTest.java | 164 ------------ 4 files changed, 132 insertions(+), 279 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java index a86172b54..189b0496c 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java +++ b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java @@ -142,8 +142,6 @@ private SDMErrorMessages() { "Invalid secondary properties detected: "; public static final String INVALID_SECONDARY_PROPERTIES_FOR_MOVE_SUFFIX = ". Attachment rolled back to source."; - public static final String FAILED_TO_COPY_ATTACHMENTS_PREFIX = - "Failed to copy the following attachments:\n"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX = "Invalid secondary properties detected: "; public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX = diff --git a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java index c98e89c7e..f3796b803 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java +++ b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java @@ -87,8 +87,6 @@ private SDMUIErrorKeys() {} "SDM.invalidSecondaryPropertiesForMovePrefix"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_MOVE_SUFFIX_KEY = "SDM.invalidSecondaryPropertiesForMoveSuffix"; - public static final String FAILED_TO_COPY_ATTACHMENTS_PREFIX_KEY = - "SDM.failedToCopyAttachmentsPrefix"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX_KEY = "SDM.invalidSecondaryPropertiesForCopyPrefix"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX_KEY = diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index 8de802ec6..f43a3363b 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -135,101 +135,10 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti List objectIds = context.getObjectIds(); - // Pre-copy validation: Check for invalid secondary properties before copying - List validObjectIds = new ArrayList<>(objectIds); - List> copyFailures = new ArrayList<>(); - - if (!customPropertiesInSDM.isEmpty()) { - // Fetch valid secondary properties from SDM - List secondaryTypes = - sdmService.getSecondaryTypes(repositoryId, sdmCredentials, isSystemUser); - List validSecondaryProperties = - sdmService.getValidSecondaryProperties( - secondaryTypes, sdmCredentials, repositoryId, isSystemUser); - - logger.debug( - "Pre-copy validation - checking {} attachments against {} valid secondary properties", - objectIds.size(), - validSecondaryProperties.size()); - - validObjectIds = new ArrayList<>(); - for (String objectId : objectIds) { - try { - JSONObject sdmMetadata = sdmService.getObject(objectId, sdmCredentials, isSystemUser); - if (sdmMetadata != null && sdmMetadata.has("succinctProperties")) { - JSONObject succinctProperties = sdmMetadata.getJSONObject("succinctProperties"); - Set sdmResponseProperties = new HashSet<>(succinctProperties.keySet()); - - List invalidProperties = new ArrayList<>(); - for (String targetSdmProperty : customPropertiesInSDM) { - if (sdmResponseProperties.contains(targetSdmProperty) - && !validSecondaryProperties.contains(targetSdmProperty)) { - invalidProperties.add(targetSdmProperty); - logger.warn( - "Pre-copy validation - Attachment {} has invalid secondary property '{}'" - + " (present in SDM response but not in valid secondary properties list)", - objectId, - targetSdmProperty); - } - } - - if (!invalidProperties.isEmpty()) { - Map failure = new HashMap<>(); - failure.put(OBJECT_ID_KEY, objectId); - failure.put( - FAILURE_REASON_KEY, - SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") - + String.join(", ", invalidProperties) - + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); - copyFailures.add(failure); - logger.warn( - "Pre-copy validation - Skipping attachment {} due to {} invalid secondary properties: {}", - objectId, - invalidProperties.size(), - invalidProperties); - } else { - validObjectIds.add(objectId); - } - } else { - validObjectIds.add(objectId); - } - } catch (IOException e) { - logger.error( - "Pre-copy validation - Failed to fetch metadata for attachment {}: {}", - objectId, - e.getMessage()); - validObjectIds.add(objectId); - } - } - } - - // Show warning if there are attachments with invalid secondary properties - if (!copyFailures.isEmpty()) { - StringBuilder warningMessage = - new StringBuilder(SDMUtils.getErrorMessage("FAILED_TO_COPY_ATTACHMENTS_PREFIX")); - for (Map failure : copyFailures) { - warningMessage - .append("- ObjectId: ") - .append(failure.get(OBJECT_ID_KEY)) - .append(", Reason: ") - .append(failure.get(FAILURE_REASON_KEY)) - .append("\n"); - } - context.getMessages().warn(warningMessage.toString()); - } - - // If no valid attachments remain after validation, complete early - if (validObjectIds.isEmpty()) { - logger.info("No valid attachments to copy after pre-copy validation for upID: {}", upID); - context.setCompleted(); - logger.debug("END: Copy attachments event"); - return; - } - CopyAttachmentsRequest request = CopyAttachmentsRequest.builder() .context(context) - .objectIds(validObjectIds) + .objectIds(objectIds) .folderId(folderId) .repositoryId(repositoryId) .sdmCredentials(sdmCredentials) @@ -241,25 +150,44 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti List> attachmentsMetadata = copyResult.getAttachmentsMetadata(); List populatedDocuments = copyResult.getPopulatedDocuments(); + List> copyFailures = copyResult.getFailedAttachments(); - String upIdKey = resolveUpIdKey(context, parentEntity, compositionName); + // Show warning if there are failures + if (!copyFailures.isEmpty()) { + StringBuilder warningMessage = + new StringBuilder("Failed to copy the following attachments:\n"); + for (Map failure : copyFailures) { + warningMessage + .append(" - ObjectId: ") + .append(failure.get(OBJECT_ID_KEY)) + .append(", Reason: ") + .append(failure.get(FAILURE_REASON_KEY)) + .append("\n"); + } + context.getMessages().warn(warningMessage.toString()); + } - CreateDraftEntriesRequest draftRequest = - CreateDraftEntriesRequest.builder() - .attachmentsMetadata(attachmentsMetadata) - .populatedDocuments(populatedDocuments) - .parentEntity(parentEntity) - .compositionName(compositionName) - .upID(upID) - .upIdKey(upIdKey) - .repositoryId(repositoryId) - .folderId(folderId) - .customPropertyValues(null) - .build(); + String upIdKey = resolveUpIdKey(context, parentEntity, compositionName); - // Pass the entity for type conversion - CdsEntity targetEntity = entity.isPresent() ? entity.get() : null; - createDraftEntries(draftRequest, customPropertyDefinitions, targetEntity); + // Create draft entries if there are successful copies + if (!attachmentsMetadata.isEmpty()) { + CreateDraftEntriesRequest draftRequest = + CreateDraftEntriesRequest.builder() + .attachmentsMetadata(attachmentsMetadata) + .populatedDocuments(populatedDocuments) + .parentEntity(parentEntity) + .compositionName(compositionName) + .upID(upID) + .upIdKey(upIdKey) + .repositoryId(repositoryId) + .folderId(folderId) + .customPropertyValues(null) + .build(); + + // Pass the entity for type conversion + CdsEntity targetEntity = entity.isPresent() ? entity.get() : null; + createDraftEntries(draftRequest, customPropertyDefinitions, targetEntity); + } logger.info( "Copy attachments completed - {} attachments copied for upID: {}", @@ -661,9 +589,64 @@ private CopyAttachmentsResult copyAttachmentsToSDM( logger.debug("START: Copy {} attachments to SDM", request.getObjectIds().size()); List> attachmentsMetadata = new ArrayList<>(); List populatedDocuments = new ArrayList<>(); + List> failedAttachments = new ArrayList<>(); + + // Fetch valid secondary properties for validation + List validSecondaryProperties = new ArrayList<>(); + boolean shouldValidateSecondaryProperties = false; + if (!customPropertiesInSDM.isEmpty()) { + try { + List secondaryTypes = + sdmService.getSecondaryTypes( + request.getRepositoryId(), request.getSdmCredentials(), request.getIsSystemUser()); + validSecondaryProperties = + sdmService.getValidSecondaryProperties( + secondaryTypes, + request.getSdmCredentials(), + request.getRepositoryId(), + request.getIsSystemUser()); + shouldValidateSecondaryProperties = true; + logger.debug( + "Fetched {} valid secondary properties for copy validation", + validSecondaryProperties.size()); + } catch (Exception e) { + logger.warn( + "Failed to fetch valid secondary properties for copy validation: {}. " + + "Proceeding without validation.", + e.getMessage()); + } + } for (String objectId : request.getObjectIds()) { logger.debug("Processing copy for objectId: {}", objectId); + + // Validate secondary properties BEFORE copying using source attachment metadata + if (shouldValidateSecondaryProperties) { + List invalidProperties = + validateSourceAttachmentProperties( + objectId, + customPropertiesInSDM, + validSecondaryProperties, + request.getSdmCredentials(), + request.getIsSystemUser()); + + if (!invalidProperties.isEmpty()) { + logger.warn( + "Attachment {} has invalid secondary properties: {}. Skipping copy.", + objectId, + invalidProperties); + Map failure = new HashMap<>(); + failure.put(OBJECT_ID_KEY, objectId); + failure.put( + FAILURE_REASON_KEY, + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") + + String.join(", ", invalidProperties) + + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); + failedAttachments.add(failure); + continue; + } + } + CmisDocument cmisDocument = dbQuery.getAttachmentForObjectID(persistenceService, objectId, request.getContext()); cmisDocument.setObjectId(objectId); @@ -675,7 +658,6 @@ private CopyAttachmentsResult copyAttachmentsToSDM( populatedDocument.setType(cmisDocument.getType()); populatedDocument.setUrl(cmisDocument.getUrl()); populatedDocument.setUploadStatus(cmisDocument.getUploadStatus()); - populatedDocuments.add(populatedDocument); try { Map attachmentData = @@ -685,6 +667,7 @@ private CopyAttachmentsResult copyAttachmentsToSDM( request.getIsSystemUser(), customPropertiesInSDM); + populatedDocuments.add(populatedDocument); attachmentsMetadata.add(attachmentData); logger.debug("Successfully copied attachment: {}", objectId); } catch (ServiceException e) { @@ -698,8 +681,11 @@ private CopyAttachmentsResult copyAttachmentsToSDM( } } - logger.debug("END: Copy attachments to SDM - {} successful", attachmentsMetadata.size()); - return new CopyAttachmentsResult(attachmentsMetadata, populatedDocuments); + logger.debug( + "END: Copy attachments to SDM - {} successful, {} failed", + attachmentsMetadata.size(), + failedAttachments.size()); + return new CopyAttachmentsResult(attachmentsMetadata, populatedDocuments, failedAttachments); } /** @@ -1635,6 +1621,41 @@ private void handleCopyFailure( throw new ServiceException(e.getMessage()); } + /** + * Validates source attachment's secondary properties before copying. Fetches attachment metadata + * from SDM and checks if it has properties not supported by the target entity. + * + * @return list of invalid property names, empty if all properties are valid + */ + private List validateSourceAttachmentProperties( + String objectId, + Set customPropertiesInSDM, + List validSecondaryProperties, + SDMCredentials sdmCredentials, + Boolean isSystemUser) { + List invalidProperties = new ArrayList<>(); + try { + JSONObject sdmMetadata = sdmService.getObject(objectId, sdmCredentials, isSystemUser); + if (sdmMetadata != null) { + JSONObject succinctProperties = sdmMetadata.optJSONObject("succinctProperties"); + if (succinctProperties != null) { + for (String customProp : customPropertiesInSDM) { + if (succinctProperties.has(customProp) + && !validSecondaryProperties.contains(customProp)) { + invalidProperties.add(customProp); + } + } + } + } + } catch (Exception e) { + logger.warn( + "Failed to fetch source attachment {} for validation: {}. Proceeding with copy.", + objectId, + e.getMessage()); + } + return invalidProperties; + } + private String resolveUpIdKey(EventContext context, String parentEntity, String compositionName) { logger.debug( "Resolving upIdKey for parentEntity: {}, compositionName: {}", diff --git a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java index caf9a5874..437410755 100644 --- a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java +++ b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java @@ -325,170 +325,6 @@ void testCopyAttachments_AttachmentCopyFails_FolderExists_AttachmentsDeleted() assertTrue(ex.getMessage().contains("Copy failed")); } - @Test - void testCopyAttachments_InvalidSecondaryProperty_SkipsAttachment() throws IOException { - // Mock SDMCredentials - SDMCredentials sdmCredentials = mock(SDMCredentials.class); - when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); - - // Mock folder id retrieval - when(sdmService.getFolderIdByPath(any(), any(), any(), anyBoolean())).thenReturn(FOLDER_ID); - - // Create mock context with custom property annotations on the entity - AttachmentCopyEventContext context = createMockContextWithCustomProperties(); - when(context.getObjectIds()).thenReturn(List.of(OBJECT_ID)); - - // Mock secondary types and valid secondary properties from SDM - when(sdmService.getSecondaryTypes(any(), any(), anyBoolean())) - .thenReturn(List.of("secondaryType1")); - when(sdmService.getValidSecondaryProperties(any(), any(), any(), anyBoolean())) - .thenReturn(List.of("validProp1", "validProp2")); - - // Mock getObject: SDM response contains "invalidCustomProp" which is NOT in valid list - JSONObject sdmMetadata = new JSONObject(); - JSONObject succinctProperties = new JSONObject(); - succinctProperties.put("cmis:name", "test.pdf"); - succinctProperties.put("invalidCustomProp", "someValue"); - sdmMetadata.put("succinctProperties", succinctProperties); - when(sdmService.getObject(eq(OBJECT_ID), any(), anyBoolean())).thenReturn(sdmMetadata); - - // Act - sdmCustomServiceHandler.copyAttachments(context); - - // Assert: attachment should be skipped, no copy operation performed - verify(sdmService, never()) - .copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any()); - // Warning message should be issued - verify(context.getMessages(), times(1)).warn(any(String.class)); - verify(context, times(1)).setCompleted(); - } - - @Test - void testCopyAttachments_MixedValidAndInvalidSecondaryProps_CopiesOnlyValid() throws IOException { - // Mock SDMCredentials - SDMCredentials sdmCredentials = mock(SDMCredentials.class); - when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); - - // Mock folder id retrieval - when(sdmService.getFolderIdByPath(any(), any(), any(), anyBoolean())).thenReturn(FOLDER_ID); - - String validObjectId = "validObjectId"; - String invalidObjectId = "invalidObjectId"; - - // Create mock context with custom property annotations - AttachmentCopyEventContext context = createMockContextWithCustomProperties(); - when(context.getObjectIds()).thenReturn(List.of(validObjectId, invalidObjectId)); - - // Mock secondary types and valid secondary properties - when(sdmService.getSecondaryTypes(any(), any(), anyBoolean())) - .thenReturn(List.of("secondaryType1")); - // "invalidCustomProp" is NOT in the valid list - only "validProp1" and "validProp2" are valid - when(sdmService.getValidSecondaryProperties(any(), any(), any(), anyBoolean())) - .thenReturn(List.of("validProp1", "validProp2")); - - // Mock getObject for valid attachment (does NOT have "invalidCustomProp" in response) - JSONObject validMetadata = new JSONObject(); - JSONObject validProps = new JSONObject(); - validProps.put("cmis:name", "valid.pdf"); - validProps.put("validProp1", "someValue"); // only valid properties present - validMetadata.put("succinctProperties", validProps); - when(sdmService.getObject(eq(validObjectId), any(), anyBoolean())).thenReturn(validMetadata); - - // Mock getObject for invalid attachment (HAS "invalidCustomProp" in response, which is NOT in - // valid list) - JSONObject invalidMetadata = new JSONObject(); - JSONObject invalidProps = new JSONObject(); - invalidProps.put("cmis:name", "invalid.pdf"); - invalidProps.put("invalidCustomProp", "someValue"); // present in response but NOT in valid list - invalidMetadata.put("succinctProperties", invalidProps); - when(sdmService.getObject(eq(invalidObjectId), any(), anyBoolean())) - .thenReturn(invalidMetadata); - - // Mock attachment copy for valid attachment - Map attachmentData = new HashMap<>(); - attachmentData.put("cmis:name", "valid.pdf"); - attachmentData.put("cmis:contentStreamMimeType", "application/pdf"); - attachmentData.put("cmis:objectId", validObjectId); - when(sdmService.copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any())) - .thenReturn(attachmentData); - CmisDocument cmisDocument = new CmisDocument(); - cmisDocument.setType("sap-icon://document"); - when(dbQuery.getAttachmentForObjectID(any(), any(), any(AttachmentCopyEventContext.class))) - .thenReturn(cmisDocument); - - // Act - sdmCustomServiceHandler.copyAttachments(context); - - // Assert: only valid attachment should be copied - verify(sdmService, times(1)) - .copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any()); - verify(context, times(1)).setCompleted(); - } - - private AttachmentCopyEventContext createMockContextWithCustomProperties() { - AttachmentCopyEventContext context = mock(AttachmentCopyEventContext.class); - CdsElement mockAssociationElement = mock(CdsElement.class); - CdsAssociationType mockAssociationType = mock(CdsAssociationType.class); - CqnElementRef mockCqnElementRef = mock(CqnElementRef.class); - - when(context.getParentEntity()).thenReturn("prefix.someIdentifier." + FACET); - when(context.getCompositionName()).thenReturn(FACET); - when(context.getUpId()).thenReturn(UP_ID); - when(context.getSystemUser()).thenReturn(true); - when(context.getObjectIds()).thenReturn(List.of(OBJECT_ID)); - - // Mock CdsModel and relevant entities and associations - CdsModel model = mock(CdsModel.class); - CdsEntity parentEntity = mock(CdsEntity.class); - CdsEntity draftEntity = mock(CdsEntity.class); - CdsEntity targetEntity = mock(CdsEntity.class); - CdsEntity compositionEntity = mock(CdsEntity.class); - - // Mock composition element and its type - CdsElement compositionElement = mock(CdsElement.class); - CdsAssociationType compositionType = mock(CdsAssociationType.class); - - // Setup expected behavior for model and parent entity - when(context.getModel()).thenReturn(model); - when(model.findEntity("prefix.someIdentifier." + FACET)).thenReturn(Optional.of(parentEntity)); - when(model.findEntity("prefix.someIdentifier." + FACET + "." + FACET)) - .thenReturn(Optional.of(compositionEntity)); - when(model.findEntity(endsWith("_drafts"))).thenReturn(Optional.of(draftEntity)); - - // Mock the composition entity with a custom property annotation - CdsElement customPropertyElement = mock(CdsElement.class); - com.sap.cds.reflect.CdsAnnotation nameAnnotation = - mock(com.sap.cds.reflect.CdsAnnotation.class); - com.sap.cds.reflect.CdsType elementType = mock(com.sap.cds.reflect.CdsType.class); - when(elementType.isAssociation()).thenReturn(false); - when(customPropertyElement.getType()).thenReturn(elementType); - when(customPropertyElement.getName()).thenReturn("customField"); - when(customPropertyElement.findAnnotation("SDM.Attachments.AdditionalProperty.name")) - .thenReturn(Optional.of(nameAnnotation)); - when(nameAnnotation.getValue()).thenReturn("invalidCustomProp"); - when(compositionEntity.elements()).thenReturn(Stream.of(customPropertyElement)); - - // Mock the composition element in parent entity - when(parentEntity.findElement(FACET)).thenReturn(Optional.of(compositionElement)); - when(compositionElement.getType()).thenReturn(compositionType); - when(compositionType.isAssociation()).thenReturn(true); - when(compositionType.getTarget()).thenReturn(targetEntity); - when(targetEntity.getQualifiedName()).thenReturn("target.entity.name"); - - // Mock the draft entity's up_ association - when(draftEntity.findAssociation("up_")).thenReturn(Optional.of(mockAssociationElement)); - when(mockAssociationElement.getType()).thenReturn(mockAssociationType); - when(mockAssociationType.refs()).thenReturn(Stream.of(mockCqnElementRef)); - when(mockCqnElementRef.path()).thenReturn("ID"); - - // Mock messages - com.sap.cds.services.messages.Messages messages = - mock(com.sap.cds.services.messages.Messages.class); - when(context.getMessages()).thenReturn(messages); - - return context; - } - private AttachmentCopyEventContext createMockContext() { AttachmentCopyEventContext context = mock(AttachmentCopyEventContext.class); CdsElement mockAssociationElement = mock(CdsElement.class); From 914ed29efcc1ece793fd98ac131578edaece9710 Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 16:03:10 +0530 Subject: [PATCH 07/16] Revert "removed delete logic" This reverts commit 9ff552f8385d15ef10eca845e4cc8d1b97db7be0. --- .../sdm/service/handler/SDMCustomServiceHandler.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index f43a3363b..46ecbe80e 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -187,6 +187,17 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti // Pass the entity for type conversion CdsEntity targetEntity = entity.isPresent() ? entity.get() : null; createDraftEntries(draftRequest, customPropertyDefinitions, targetEntity); + } else if (!folderExists) { + // All copies failed but folder was newly created - delete the empty folder + logger.info( + "All attachments failed to copy. Deleting newly created empty folder: {}", folderId); + try { + sdmService.deleteDocument( + "deleteTree", folderId, context.getUserInfo().getName(), isSystemUser); + logger.info("Deleted empty folder: {}", folderId); + } catch (Exception e) { + logger.error("Failed to delete empty folder {}: {}", folderId, e.getMessage()); + } } logger.info( From 3300b886a6ac6f1210266a38b4a5268073f6b90a Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 16:03:18 +0530 Subject: [PATCH 08/16] Revert "changes" This reverts commit c567210f607522816a27f9bd651eac17dc72ba14. --- .../handler/SDMCustomServiceHandler.java | 105 ++++++++---------- 1 file changed, 46 insertions(+), 59 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index 46ecbe80e..2ab005dbd 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -630,34 +630,6 @@ private CopyAttachmentsResult copyAttachmentsToSDM( for (String objectId : request.getObjectIds()) { logger.debug("Processing copy for objectId: {}", objectId); - - // Validate secondary properties BEFORE copying using source attachment metadata - if (shouldValidateSecondaryProperties) { - List invalidProperties = - validateSourceAttachmentProperties( - objectId, - customPropertiesInSDM, - validSecondaryProperties, - request.getSdmCredentials(), - request.getIsSystemUser()); - - if (!invalidProperties.isEmpty()) { - logger.warn( - "Attachment {} has invalid secondary properties: {}. Skipping copy.", - objectId, - invalidProperties); - Map failure = new HashMap<>(); - failure.put(OBJECT_ID_KEY, objectId); - failure.put( - FAILURE_REASON_KEY, - SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") - + String.join(", ", invalidProperties) - + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); - failedAttachments.add(failure); - continue; - } - } - CmisDocument cmisDocument = dbQuery.getAttachmentForObjectID(persistenceService, objectId, request.getContext()); cmisDocument.setObjectId(objectId); @@ -678,6 +650,43 @@ private CopyAttachmentsResult copyAttachmentsToSDM( request.getIsSystemUser(), customPropertiesInSDM); + // Validate secondary properties after copy (similar to move validation) + // Only validate if we successfully fetched valid secondary properties + List invalidProperties = new ArrayList<>(); + if (shouldValidateSecondaryProperties) { + for (String customProp : customPropertiesInSDM) { + if (attachmentData.containsKey(customProp) + && !validSecondaryProperties.contains(customProp)) { + invalidProperties.add(customProp); + } + } + } + + if (!invalidProperties.isEmpty()) { + // Validation failed - delete the copied attachment and record failure + String copiedObjectId = attachmentData.get("cmis:objectId"); + logger.error( + "Attachment {} validation FAILED - Found {} invalid properties: {}. " + + "Deleting copied attachment...", + objectId, + invalidProperties.size(), + invalidProperties); + deleteInvalidCopiedAttachment( + copiedObjectId, + request.getContext().getUserInfo().getName(), + request.getIsSystemUser()); + // Add to failed attachments list instead of throwing exception + Map failure = new HashMap<>(); + failure.put(OBJECT_ID_KEY, objectId); + failure.put( + FAILURE_REASON_KEY, + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") + + String.join(", ", invalidProperties) + + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); + failedAttachments.add(failure); + continue; + } + populatedDocuments.add(populatedDocument); attachmentsMetadata.add(attachmentData); logger.debug("Successfully copied attachment: {}", objectId); @@ -1632,39 +1641,17 @@ private void handleCopyFailure( throw new ServiceException(e.getMessage()); } - /** - * Validates source attachment's secondary properties before copying. Fetches attachment metadata - * from SDM and checks if it has properties not supported by the target entity. - * - * @return list of invalid property names, empty if all properties are valid - */ - private List validateSourceAttachmentProperties( - String objectId, - Set customPropertiesInSDM, - List validSecondaryProperties, - SDMCredentials sdmCredentials, - Boolean isSystemUser) { - List invalidProperties = new ArrayList<>(); + private void deleteInvalidCopiedAttachment( + String copiedObjectId, String userName, Boolean isSystemUser) { try { - JSONObject sdmMetadata = sdmService.getObject(objectId, sdmCredentials, isSystemUser); - if (sdmMetadata != null) { - JSONObject succinctProperties = sdmMetadata.optJSONObject("succinctProperties"); - if (succinctProperties != null) { - for (String customProp : customPropertiesInSDM) { - if (succinctProperties.has(customProp) - && !validSecondaryProperties.contains(customProp)) { - invalidProperties.add(customProp); - } - } - } - } - } catch (Exception e) { - logger.warn( - "Failed to fetch source attachment {} for validation: {}. Proceeding with copy.", - objectId, - e.getMessage()); + sdmService.deleteDocument("delete", copiedObjectId, userName, isSystemUser); + logger.info("Deleted invalid copied attachment: {}", copiedObjectId); + } catch (Exception deleteEx) { + logger.error( + "Failed to delete invalid copied attachment {}: {}", + copiedObjectId, + deleteEx.getMessage()); } - return invalidProperties; } private String resolveUpIdKey(EventContext context, String parentEntity, String compositionName) { From 2adbccc738f03ca8ca673c956db3cfa3d145d025 Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 16:03:28 +0530 Subject: [PATCH 09/16] Revert "sonar fix" This reverts commit b95da2d06d741bcf46c11c975724f0cede95563b. --- .../handler/SDMCustomServiceHandler.java | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index 2ab005dbd..5abade461 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -671,10 +671,19 @@ private CopyAttachmentsResult copyAttachmentsToSDM( objectId, invalidProperties.size(), invalidProperties); - deleteInvalidCopiedAttachment( - copiedObjectId, - request.getContext().getUserInfo().getName(), - request.getIsSystemUser()); + try { + sdmService.deleteDocument( + "delete", + copiedObjectId, + request.getContext().getUserInfo().getName(), + request.getIsSystemUser()); + logger.info("Deleted invalid copied attachment: {}", copiedObjectId); + } catch (Exception deleteEx) { + logger.error( + "Failed to delete invalid copied attachment {}: {}", + copiedObjectId, + deleteEx.getMessage()); + } // Add to failed attachments list instead of throwing exception Map failure = new HashMap<>(); failure.put(OBJECT_ID_KEY, objectId); @@ -1641,19 +1650,6 @@ private void handleCopyFailure( throw new ServiceException(e.getMessage()); } - private void deleteInvalidCopiedAttachment( - String copiedObjectId, String userName, Boolean isSystemUser) { - try { - sdmService.deleteDocument("delete", copiedObjectId, userName, isSystemUser); - logger.info("Deleted invalid copied attachment: {}", copiedObjectId); - } catch (Exception deleteEx) { - logger.error( - "Failed to delete invalid copied attachment {}: {}", - copiedObjectId, - deleteEx.getMessage()); - } - } - private String resolveUpIdKey(EventContext context, String parentEntity, String compositionName) { logger.debug( "Resolving upIdKey for parentEntity: {}, compositionName: {}", From f76b95f15a6e14c4da3194c2bab54d110beb5ffc Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 16:03:44 +0530 Subject: [PATCH 10/16] Revert "fix copy attachment with invalid properties" This reverts commit 2fb5bf85a5ae036f995a723f0e01a472b4534cb3. --- .../cds/sdm/constants/SDMErrorMessages.java | 4 - .../sap/cds/sdm/constants/SDMUIErrorKeys.java | 4 - .../cds/sdm/model/CopyAttachmentsResult.java | 14 -- .../handler/SDMCustomServiceHandler.java | 144 +++--------------- 4 files changed, 19 insertions(+), 147 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java index 189b0496c..14cae539f 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java +++ b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java @@ -142,10 +142,6 @@ private SDMErrorMessages() { "Invalid secondary properties detected: "; public static final String INVALID_SECONDARY_PROPERTIES_FOR_MOVE_SUFFIX = ". Attachment rolled back to source."; - public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX = - "Invalid secondary properties detected: "; - public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX = - ". Attachment not copied."; public static final String SDM_MOVE_OPERATION_FAILED = "SDM move operation failed"; public static final String FAILED_TO_ACCESS_ERROR_KEY_FIELDS = "Failed to access SDM error key fields"; diff --git a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java index f3796b803..8eead9791 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java +++ b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java @@ -87,10 +87,6 @@ private SDMUIErrorKeys() {} "SDM.invalidSecondaryPropertiesForMovePrefix"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_MOVE_SUFFIX_KEY = "SDM.invalidSecondaryPropertiesForMoveSuffix"; - public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX_KEY = - "SDM.invalidSecondaryPropertiesForCopyPrefix"; - public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX_KEY = - "SDM.invalidSecondaryPropertiesForCopySuffix"; public static final String MAX_COUNT_ERROR_MESSAGE_KEY = "SDM.maxCountErrorMessage"; public static Map getAllUIErrorKeys() { diff --git a/sdm/src/main/java/com/sap/cds/sdm/model/CopyAttachmentsResult.java b/sdm/src/main/java/com/sap/cds/sdm/model/CopyAttachmentsResult.java index f27f834f5..93cf50bfd 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/model/CopyAttachmentsResult.java +++ b/sdm/src/main/java/com/sap/cds/sdm/model/CopyAttachmentsResult.java @@ -1,6 +1,5 @@ package com.sap.cds.sdm.model; -import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -8,20 +7,11 @@ public class CopyAttachmentsResult { private final List> attachmentsMetadata; private final List populatedDocuments; - private final List> failedAttachments; public CopyAttachmentsResult( List> attachmentsMetadata, List populatedDocuments) { - this(attachmentsMetadata, populatedDocuments, new ArrayList<>()); - } - - public CopyAttachmentsResult( - List> attachmentsMetadata, - List populatedDocuments, - List> failedAttachments) { this.attachmentsMetadata = attachmentsMetadata; this.populatedDocuments = populatedDocuments; - this.failedAttachments = failedAttachments; } public List> getAttachmentsMetadata() { @@ -31,8 +21,4 @@ public List> getAttachmentsMetadata() { public List getPopulatedDocuments() { return populatedDocuments; } - - public List> getFailedAttachments() { - return failedAttachments; - } } diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index 5abade461..b393a30a9 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -150,55 +150,25 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti List> attachmentsMetadata = copyResult.getAttachmentsMetadata(); List populatedDocuments = copyResult.getPopulatedDocuments(); - List> copyFailures = copyResult.getFailedAttachments(); - - // Show warning if there are failures - if (!copyFailures.isEmpty()) { - StringBuilder warningMessage = - new StringBuilder("Failed to copy the following attachments:\n"); - for (Map failure : copyFailures) { - warningMessage - .append(" - ObjectId: ") - .append(failure.get(OBJECT_ID_KEY)) - .append(", Reason: ") - .append(failure.get(FAILURE_REASON_KEY)) - .append("\n"); - } - context.getMessages().warn(warningMessage.toString()); - } String upIdKey = resolveUpIdKey(context, parentEntity, compositionName); - // Create draft entries if there are successful copies - if (!attachmentsMetadata.isEmpty()) { - CreateDraftEntriesRequest draftRequest = - CreateDraftEntriesRequest.builder() - .attachmentsMetadata(attachmentsMetadata) - .populatedDocuments(populatedDocuments) - .parentEntity(parentEntity) - .compositionName(compositionName) - .upID(upID) - .upIdKey(upIdKey) - .repositoryId(repositoryId) - .folderId(folderId) - .customPropertyValues(null) - .build(); - - // Pass the entity for type conversion - CdsEntity targetEntity = entity.isPresent() ? entity.get() : null; - createDraftEntries(draftRequest, customPropertyDefinitions, targetEntity); - } else if (!folderExists) { - // All copies failed but folder was newly created - delete the empty folder - logger.info( - "All attachments failed to copy. Deleting newly created empty folder: {}", folderId); - try { - sdmService.deleteDocument( - "deleteTree", folderId, context.getUserInfo().getName(), isSystemUser); - logger.info("Deleted empty folder: {}", folderId); - } catch (Exception e) { - logger.error("Failed to delete empty folder {}: {}", folderId, e.getMessage()); - } - } + CreateDraftEntriesRequest draftRequest = + CreateDraftEntriesRequest.builder() + .attachmentsMetadata(attachmentsMetadata) + .populatedDocuments(populatedDocuments) + .parentEntity(parentEntity) + .compositionName(compositionName) + .upID(upID) + .upIdKey(upIdKey) + .repositoryId(repositoryId) + .folderId(folderId) + .customPropertyValues(null) + .build(); + + // Pass the entity for type conversion + CdsEntity targetEntity = entity.isPresent() ? entity.get() : null; + createDraftEntries(draftRequest, customPropertyDefinitions, targetEntity); logger.info( "Copy attachments completed - {} attachments copied for upID: {}", @@ -600,33 +570,6 @@ private CopyAttachmentsResult copyAttachmentsToSDM( logger.debug("START: Copy {} attachments to SDM", request.getObjectIds().size()); List> attachmentsMetadata = new ArrayList<>(); List populatedDocuments = new ArrayList<>(); - List> failedAttachments = new ArrayList<>(); - - // Fetch valid secondary properties for validation - List validSecondaryProperties = new ArrayList<>(); - boolean shouldValidateSecondaryProperties = false; - if (!customPropertiesInSDM.isEmpty()) { - try { - List secondaryTypes = - sdmService.getSecondaryTypes( - request.getRepositoryId(), request.getSdmCredentials(), request.getIsSystemUser()); - validSecondaryProperties = - sdmService.getValidSecondaryProperties( - secondaryTypes, - request.getSdmCredentials(), - request.getRepositoryId(), - request.getIsSystemUser()); - shouldValidateSecondaryProperties = true; - logger.debug( - "Fetched {} valid secondary properties for copy validation", - validSecondaryProperties.size()); - } catch (Exception e) { - logger.warn( - "Failed to fetch valid secondary properties for copy validation: {}. " - + "Proceeding without validation.", - e.getMessage()); - } - } for (String objectId : request.getObjectIds()) { logger.debug("Processing copy for objectId: {}", objectId); @@ -641,6 +584,7 @@ private CopyAttachmentsResult copyAttachmentsToSDM( populatedDocument.setType(cmisDocument.getType()); populatedDocument.setUrl(cmisDocument.getUrl()); populatedDocument.setUploadStatus(cmisDocument.getUploadStatus()); + populatedDocuments.add(populatedDocument); try { Map attachmentData = @@ -650,53 +594,6 @@ private CopyAttachmentsResult copyAttachmentsToSDM( request.getIsSystemUser(), customPropertiesInSDM); - // Validate secondary properties after copy (similar to move validation) - // Only validate if we successfully fetched valid secondary properties - List invalidProperties = new ArrayList<>(); - if (shouldValidateSecondaryProperties) { - for (String customProp : customPropertiesInSDM) { - if (attachmentData.containsKey(customProp) - && !validSecondaryProperties.contains(customProp)) { - invalidProperties.add(customProp); - } - } - } - - if (!invalidProperties.isEmpty()) { - // Validation failed - delete the copied attachment and record failure - String copiedObjectId = attachmentData.get("cmis:objectId"); - logger.error( - "Attachment {} validation FAILED - Found {} invalid properties: {}. " - + "Deleting copied attachment...", - objectId, - invalidProperties.size(), - invalidProperties); - try { - sdmService.deleteDocument( - "delete", - copiedObjectId, - request.getContext().getUserInfo().getName(), - request.getIsSystemUser()); - logger.info("Deleted invalid copied attachment: {}", copiedObjectId); - } catch (Exception deleteEx) { - logger.error( - "Failed to delete invalid copied attachment {}: {}", - copiedObjectId, - deleteEx.getMessage()); - } - // Add to failed attachments list instead of throwing exception - Map failure = new HashMap<>(); - failure.put(OBJECT_ID_KEY, objectId); - failure.put( - FAILURE_REASON_KEY, - SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") - + String.join(", ", invalidProperties) - + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); - failedAttachments.add(failure); - continue; - } - - populatedDocuments.add(populatedDocument); attachmentsMetadata.add(attachmentData); logger.debug("Successfully copied attachment: {}", objectId); } catch (ServiceException e) { @@ -710,11 +607,8 @@ private CopyAttachmentsResult copyAttachmentsToSDM( } } - logger.debug( - "END: Copy attachments to SDM - {} successful, {} failed", - attachmentsMetadata.size(), - failedAttachments.size()); - return new CopyAttachmentsResult(attachmentsMetadata, populatedDocuments, failedAttachments); + logger.debug("END: Copy attachments to SDM - {} successful", attachmentsMetadata.size()); + return new CopyAttachmentsResult(attachmentsMetadata, populatedDocuments); } /** From 35ad613efe814c61e447357465cd28c2256a6618 Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 16:09:27 +0530 Subject: [PATCH 11/16] fix copy attachment with invalid properties --- .../cds/sdm/constants/SDMErrorMessages.java | 6 + .../sap/cds/sdm/constants/SDMUIErrorKeys.java | 6 + .../handler/SDMCustomServiceHandler.java | 92 +++++++++- .../handler/SDMCustomServiceHandlerTest.java | 164 ++++++++++++++++++ 4 files changed, 267 insertions(+), 1 deletion(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java index 14cae539f..8ff266c24 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java +++ b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java @@ -143,6 +143,12 @@ private SDMErrorMessages() { public static final String INVALID_SECONDARY_PROPERTIES_FOR_MOVE_SUFFIX = ". Attachment rolled back to source."; public static final String SDM_MOVE_OPERATION_FAILED = "SDM move operation failed"; + public static final String FAILED_TO_COPY_ATTACHMENTS_PREFIX = + "Failed to copy the following attachments:\n"; + public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX = + "Invalid secondary properties detected: "; + public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX = + ". Attachment not copied."; public static final String FAILED_TO_ACCESS_ERROR_KEY_FIELDS = "Failed to access SDM error key fields"; public static final String FAILED_TO_ACCESS_ERROR_MESSAGES_FIELDS = diff --git a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java index 8eead9791..c98e89c7e 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java +++ b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMUIErrorKeys.java @@ -87,6 +87,12 @@ private SDMUIErrorKeys() {} "SDM.invalidSecondaryPropertiesForMovePrefix"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_MOVE_SUFFIX_KEY = "SDM.invalidSecondaryPropertiesForMoveSuffix"; + public static final String FAILED_TO_COPY_ATTACHMENTS_PREFIX_KEY = + "SDM.failedToCopyAttachmentsPrefix"; + public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX_KEY = + "SDM.invalidSecondaryPropertiesForCopyPrefix"; + public static final String INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX_KEY = + "SDM.invalidSecondaryPropertiesForCopySuffix"; public static final String MAX_COUNT_ERROR_MESSAGE_KEY = "SDM.maxCountErrorMessage"; public static Map getAllUIErrorKeys() { diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index b393a30a9..e26dd5d8c 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -135,10 +135,100 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti List objectIds = context.getObjectIds(); + // copy validation: Check for invalid secondary properties before copying + List validObjectIds = new ArrayList<>(objectIds); + List> copyFailures = new ArrayList<>(); + + if (!customPropertiesInSDM.isEmpty()) { + // Fetch valid secondary properties from SDM + List secondaryTypes = + sdmService.getSecondaryTypes(repositoryId, sdmCredentials, isSystemUser); + List validSecondaryProperties = + sdmService.getValidSecondaryProperties( + secondaryTypes, sdmCredentials, repositoryId, isSystemUser); + + logger.debug( + "Copy validation - checking {} attachments against {} valid secondary properties", + objectIds.size(), + validSecondaryProperties.size()); + + validObjectIds = new ArrayList<>(); + for (String objectId : objectIds) { + try { + JSONObject sdmMetadata = sdmService.getObject(objectId, sdmCredentials, isSystemUser); + if (sdmMetadata != null && sdmMetadata.has("succinctProperties")) { + JSONObject succinctProperties = sdmMetadata.getJSONObject("succinctProperties"); + Set sdmResponseProperties = new HashSet<>(succinctProperties.keySet()); + + List invalidProperties = new ArrayList<>(); + for (String targetSdmProperty : customPropertiesInSDM) { + if (sdmResponseProperties.contains(targetSdmProperty) + && !validSecondaryProperties.contains(targetSdmProperty)) { + invalidProperties.add(targetSdmProperty); + logger.warn( + "Copy validation - Attachment {} has invalid secondary property '{}'" + + " (present in SDM response but not in valid secondary properties list)", + objectId, + targetSdmProperty); + } + } + + if (!invalidProperties.isEmpty()) { + Map failure = new HashMap<>(); + failure.put(OBJECT_ID_KEY, objectId); + failure.put( + FAILURE_REASON_KEY, + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") + + String.join(", ", invalidProperties) + + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); + copyFailures.add(failure); + logger.warn( + "Copy validation - Skipping attachment {} due to {} invalid secondary properties: {}", + objectId, + invalidProperties.size(), + invalidProperties); + } else { + validObjectIds.add(objectId); + } + } else { + validObjectIds.add(objectId); + } + } catch (IOException e) { + logger.error( + "Copy validation - Failed to fetch metadata for attachment {}: {}", + objectId, + e.getMessage()); + validObjectIds.add(objectId); + } + } + } + + // Show warning if there are attachments with invalid secondary properties + if (!copyFailures.isEmpty()) { + StringBuilder warningMessage = + new StringBuilder(SDMUtils.getErrorMessage("FAILED_TO_COPY_ATTACHMENTS_PREFIX")); + for (Map failure : copyFailures) { + warningMessage + .append("- ObjectId: ") + .append(failure.get(OBJECT_ID_KEY)) + .append(", Reason: ") + .append(failure.get(FAILURE_REASON_KEY)) + .append("\n"); + } + context.getMessages().warn(warningMessage.toString()); + } + + if (validObjectIds.isEmpty()) { + logger.info("No valid attachments to copy after pre-copy validation for upID: {}", upID); + context.setCompleted(); + logger.debug("END: Copy attachments event"); + return; + } + CopyAttachmentsRequest request = CopyAttachmentsRequest.builder() .context(context) - .objectIds(objectIds) + .objectIds(validObjectIds) .folderId(folderId) .repositoryId(repositoryId) .sdmCredentials(sdmCredentials) diff --git a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java index 437410755..caf9a5874 100644 --- a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java +++ b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java @@ -325,6 +325,170 @@ void testCopyAttachments_AttachmentCopyFails_FolderExists_AttachmentsDeleted() assertTrue(ex.getMessage().contains("Copy failed")); } + @Test + void testCopyAttachments_InvalidSecondaryProperty_SkipsAttachment() throws IOException { + // Mock SDMCredentials + SDMCredentials sdmCredentials = mock(SDMCredentials.class); + when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + + // Mock folder id retrieval + when(sdmService.getFolderIdByPath(any(), any(), any(), anyBoolean())).thenReturn(FOLDER_ID); + + // Create mock context with custom property annotations on the entity + AttachmentCopyEventContext context = createMockContextWithCustomProperties(); + when(context.getObjectIds()).thenReturn(List.of(OBJECT_ID)); + + // Mock secondary types and valid secondary properties from SDM + when(sdmService.getSecondaryTypes(any(), any(), anyBoolean())) + .thenReturn(List.of("secondaryType1")); + when(sdmService.getValidSecondaryProperties(any(), any(), any(), anyBoolean())) + .thenReturn(List.of("validProp1", "validProp2")); + + // Mock getObject: SDM response contains "invalidCustomProp" which is NOT in valid list + JSONObject sdmMetadata = new JSONObject(); + JSONObject succinctProperties = new JSONObject(); + succinctProperties.put("cmis:name", "test.pdf"); + succinctProperties.put("invalidCustomProp", "someValue"); + sdmMetadata.put("succinctProperties", succinctProperties); + when(sdmService.getObject(eq(OBJECT_ID), any(), anyBoolean())).thenReturn(sdmMetadata); + + // Act + sdmCustomServiceHandler.copyAttachments(context); + + // Assert: attachment should be skipped, no copy operation performed + verify(sdmService, never()) + .copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any()); + // Warning message should be issued + verify(context.getMessages(), times(1)).warn(any(String.class)); + verify(context, times(1)).setCompleted(); + } + + @Test + void testCopyAttachments_MixedValidAndInvalidSecondaryProps_CopiesOnlyValid() throws IOException { + // Mock SDMCredentials + SDMCredentials sdmCredentials = mock(SDMCredentials.class); + when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + + // Mock folder id retrieval + when(sdmService.getFolderIdByPath(any(), any(), any(), anyBoolean())).thenReturn(FOLDER_ID); + + String validObjectId = "validObjectId"; + String invalidObjectId = "invalidObjectId"; + + // Create mock context with custom property annotations + AttachmentCopyEventContext context = createMockContextWithCustomProperties(); + when(context.getObjectIds()).thenReturn(List.of(validObjectId, invalidObjectId)); + + // Mock secondary types and valid secondary properties + when(sdmService.getSecondaryTypes(any(), any(), anyBoolean())) + .thenReturn(List.of("secondaryType1")); + // "invalidCustomProp" is NOT in the valid list - only "validProp1" and "validProp2" are valid + when(sdmService.getValidSecondaryProperties(any(), any(), any(), anyBoolean())) + .thenReturn(List.of("validProp1", "validProp2")); + + // Mock getObject for valid attachment (does NOT have "invalidCustomProp" in response) + JSONObject validMetadata = new JSONObject(); + JSONObject validProps = new JSONObject(); + validProps.put("cmis:name", "valid.pdf"); + validProps.put("validProp1", "someValue"); // only valid properties present + validMetadata.put("succinctProperties", validProps); + when(sdmService.getObject(eq(validObjectId), any(), anyBoolean())).thenReturn(validMetadata); + + // Mock getObject for invalid attachment (HAS "invalidCustomProp" in response, which is NOT in + // valid list) + JSONObject invalidMetadata = new JSONObject(); + JSONObject invalidProps = new JSONObject(); + invalidProps.put("cmis:name", "invalid.pdf"); + invalidProps.put("invalidCustomProp", "someValue"); // present in response but NOT in valid list + invalidMetadata.put("succinctProperties", invalidProps); + when(sdmService.getObject(eq(invalidObjectId), any(), anyBoolean())) + .thenReturn(invalidMetadata); + + // Mock attachment copy for valid attachment + Map attachmentData = new HashMap<>(); + attachmentData.put("cmis:name", "valid.pdf"); + attachmentData.put("cmis:contentStreamMimeType", "application/pdf"); + attachmentData.put("cmis:objectId", validObjectId); + when(sdmService.copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any())) + .thenReturn(attachmentData); + CmisDocument cmisDocument = new CmisDocument(); + cmisDocument.setType("sap-icon://document"); + when(dbQuery.getAttachmentForObjectID(any(), any(), any(AttachmentCopyEventContext.class))) + .thenReturn(cmisDocument); + + // Act + sdmCustomServiceHandler.copyAttachments(context); + + // Assert: only valid attachment should be copied + verify(sdmService, times(1)) + .copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any()); + verify(context, times(1)).setCompleted(); + } + + private AttachmentCopyEventContext createMockContextWithCustomProperties() { + AttachmentCopyEventContext context = mock(AttachmentCopyEventContext.class); + CdsElement mockAssociationElement = mock(CdsElement.class); + CdsAssociationType mockAssociationType = mock(CdsAssociationType.class); + CqnElementRef mockCqnElementRef = mock(CqnElementRef.class); + + when(context.getParentEntity()).thenReturn("prefix.someIdentifier." + FACET); + when(context.getCompositionName()).thenReturn(FACET); + when(context.getUpId()).thenReturn(UP_ID); + when(context.getSystemUser()).thenReturn(true); + when(context.getObjectIds()).thenReturn(List.of(OBJECT_ID)); + + // Mock CdsModel and relevant entities and associations + CdsModel model = mock(CdsModel.class); + CdsEntity parentEntity = mock(CdsEntity.class); + CdsEntity draftEntity = mock(CdsEntity.class); + CdsEntity targetEntity = mock(CdsEntity.class); + CdsEntity compositionEntity = mock(CdsEntity.class); + + // Mock composition element and its type + CdsElement compositionElement = mock(CdsElement.class); + CdsAssociationType compositionType = mock(CdsAssociationType.class); + + // Setup expected behavior for model and parent entity + when(context.getModel()).thenReturn(model); + when(model.findEntity("prefix.someIdentifier." + FACET)).thenReturn(Optional.of(parentEntity)); + when(model.findEntity("prefix.someIdentifier." + FACET + "." + FACET)) + .thenReturn(Optional.of(compositionEntity)); + when(model.findEntity(endsWith("_drafts"))).thenReturn(Optional.of(draftEntity)); + + // Mock the composition entity with a custom property annotation + CdsElement customPropertyElement = mock(CdsElement.class); + com.sap.cds.reflect.CdsAnnotation nameAnnotation = + mock(com.sap.cds.reflect.CdsAnnotation.class); + com.sap.cds.reflect.CdsType elementType = mock(com.sap.cds.reflect.CdsType.class); + when(elementType.isAssociation()).thenReturn(false); + when(customPropertyElement.getType()).thenReturn(elementType); + when(customPropertyElement.getName()).thenReturn("customField"); + when(customPropertyElement.findAnnotation("SDM.Attachments.AdditionalProperty.name")) + .thenReturn(Optional.of(nameAnnotation)); + when(nameAnnotation.getValue()).thenReturn("invalidCustomProp"); + when(compositionEntity.elements()).thenReturn(Stream.of(customPropertyElement)); + + // Mock the composition element in parent entity + when(parentEntity.findElement(FACET)).thenReturn(Optional.of(compositionElement)); + when(compositionElement.getType()).thenReturn(compositionType); + when(compositionType.isAssociation()).thenReturn(true); + when(compositionType.getTarget()).thenReturn(targetEntity); + when(targetEntity.getQualifiedName()).thenReturn("target.entity.name"); + + // Mock the draft entity's up_ association + when(draftEntity.findAssociation("up_")).thenReturn(Optional.of(mockAssociationElement)); + when(mockAssociationElement.getType()).thenReturn(mockAssociationType); + when(mockAssociationType.refs()).thenReturn(Stream.of(mockCqnElementRef)); + when(mockCqnElementRef.path()).thenReturn("ID"); + + // Mock messages + com.sap.cds.services.messages.Messages messages = + mock(com.sap.cds.services.messages.Messages.class); + when(context.getMessages()).thenReturn(messages); + + return context; + } + private AttachmentCopyEventContext createMockContext() { AttachmentCopyEventContext context = mock(AttachmentCopyEventContext.class); CdsElement mockAssociationElement = mock(CdsElement.class); From b1f6a15d0463882f6497443cc320095a1df7eeae Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 16:28:38 +0530 Subject: [PATCH 12/16] sonar fix --- .../handler/SDMCustomServiceHandler.java | 220 ++++++++++++------ 1 file changed, 143 insertions(+), 77 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index e26dd5d8c..1e3ecb539 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -135,87 +135,20 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti List objectIds = context.getObjectIds(); - // copy validation: Check for invalid secondary properties before copying - List validObjectIds = new ArrayList<>(objectIds); + // Pre-copy validation: Check for invalid secondary properties before copying List> copyFailures = new ArrayList<>(); - - if (!customPropertiesInSDM.isEmpty()) { - // Fetch valid secondary properties from SDM - List secondaryTypes = - sdmService.getSecondaryTypes(repositoryId, sdmCredentials, isSystemUser); - List validSecondaryProperties = - sdmService.getValidSecondaryProperties( - secondaryTypes, sdmCredentials, repositoryId, isSystemUser); - - logger.debug( - "Copy validation - checking {} attachments against {} valid secondary properties", - objectIds.size(), - validSecondaryProperties.size()); - - validObjectIds = new ArrayList<>(); - for (String objectId : objectIds) { - try { - JSONObject sdmMetadata = sdmService.getObject(objectId, sdmCredentials, isSystemUser); - if (sdmMetadata != null && sdmMetadata.has("succinctProperties")) { - JSONObject succinctProperties = sdmMetadata.getJSONObject("succinctProperties"); - Set sdmResponseProperties = new HashSet<>(succinctProperties.keySet()); - - List invalidProperties = new ArrayList<>(); - for (String targetSdmProperty : customPropertiesInSDM) { - if (sdmResponseProperties.contains(targetSdmProperty) - && !validSecondaryProperties.contains(targetSdmProperty)) { - invalidProperties.add(targetSdmProperty); - logger.warn( - "Copy validation - Attachment {} has invalid secondary property '{}'" - + " (present in SDM response but not in valid secondary properties list)", - objectId, - targetSdmProperty); - } - } - - if (!invalidProperties.isEmpty()) { - Map failure = new HashMap<>(); - failure.put(OBJECT_ID_KEY, objectId); - failure.put( - FAILURE_REASON_KEY, - SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") - + String.join(", ", invalidProperties) - + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); - copyFailures.add(failure); - logger.warn( - "Copy validation - Skipping attachment {} due to {} invalid secondary properties: {}", - objectId, - invalidProperties.size(), - invalidProperties); - } else { - validObjectIds.add(objectId); - } - } else { - validObjectIds.add(objectId); - } - } catch (IOException e) { - logger.error( - "Copy validation - Failed to fetch metadata for attachment {}: {}", - objectId, - e.getMessage()); - validObjectIds.add(objectId); - } - } - } + List validObjectIds = + validateObjectIdsForCopy( + objectIds, + customPropertiesInSDM, + repositoryId, + sdmCredentials, + isSystemUser, + copyFailures); // Show warning if there are attachments with invalid secondary properties if (!copyFailures.isEmpty()) { - StringBuilder warningMessage = - new StringBuilder(SDMUtils.getErrorMessage("FAILED_TO_COPY_ATTACHMENTS_PREFIX")); - for (Map failure : copyFailures) { - warningMessage - .append("- ObjectId: ") - .append(failure.get(OBJECT_ID_KEY)) - .append(", Reason: ") - .append(failure.get(FAILURE_REASON_KEY)) - .append("\n"); - } - context.getMessages().warn(warningMessage.toString()); + buildAndWarnCopyFailures(copyFailures, context); } if (validObjectIds.isEmpty()) { @@ -268,6 +201,139 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti logger.debug("END: Copy attachments event"); } + /** + * Validates object IDs for copy by checking for invalid secondary properties. Attachments with + * invalid properties are excluded and added to the failures list. + */ + private List validateObjectIdsForCopy( + List objectIds, + Set customPropertiesInSDM, + String repositoryId, + SDMCredentials sdmCredentials, + Boolean isSystemUser, + List> copyFailures) + throws IOException { + if (customPropertiesInSDM.isEmpty()) { + return new ArrayList<>(objectIds); + } + + List secondaryTypes = + sdmService.getSecondaryTypes(repositoryId, sdmCredentials, isSystemUser); + List validSecondaryProperties = + sdmService.getValidSecondaryProperties( + secondaryTypes, sdmCredentials, repositoryId, isSystemUser); + + logger.debug( + "Copy validation - checking {} attachments against {} valid secondary properties", + objectIds.size(), + validSecondaryProperties.size()); + + List validObjectIds = new ArrayList<>(); + for (String objectId : objectIds) { + validateSingleObjectForCopy( + objectId, + customPropertiesInSDM, + validSecondaryProperties, + sdmCredentials, + isSystemUser, + validObjectIds, + copyFailures); + } + return validObjectIds; + } + + /** + * Validates a single attachment for copy by checking its secondary properties against the valid + * list. Adds to validObjectIds if valid, or to copyFailures if invalid. + */ + private void validateSingleObjectForCopy( + String objectId, + Set customPropertiesInSDM, + List validSecondaryProperties, + SDMCredentials sdmCredentials, + Boolean isSystemUser, + List validObjectIds, + List> copyFailures) { + try { + JSONObject sdmMetadata = sdmService.getObject(objectId, sdmCredentials, isSystemUser); + if (sdmMetadata == null || !sdmMetadata.has("succinctProperties")) { + validObjectIds.add(objectId); + return; + } + + JSONObject succinctProperties = sdmMetadata.getJSONObject("succinctProperties"); + Set sdmResponseProperties = new HashSet<>(succinctProperties.keySet()); + + List invalidProperties = + findInvalidSecondaryProperties( + customPropertiesInSDM, sdmResponseProperties, validSecondaryProperties, objectId); + + if (invalidProperties.isEmpty()) { + validObjectIds.add(objectId); + } else { + Map failure = new HashMap<>(); + failure.put(OBJECT_ID_KEY, objectId); + failure.put( + FAILURE_REASON_KEY, + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") + + String.join(", ", invalidProperties) + + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); + copyFailures.add(failure); + logger.warn( + "Copy validation - Skipping attachment {} due to {} invalid secondary properties: {}", + objectId, + invalidProperties.size(), + invalidProperties); + } + } catch (IOException e) { + logger.error( + "Copy validation - Failed to fetch metadata for attachment {}: {}", + objectId, + e.getMessage()); + validObjectIds.add(objectId); + } + } + + /** + * Finds secondary properties that are present in the SDM response but not in the valid properties + * list. + */ + private List findInvalidSecondaryProperties( + Set customPropertiesInSDM, + Set sdmResponseProperties, + List validSecondaryProperties, + String objectId) { + List invalidProperties = new ArrayList<>(); + for (String targetSdmProperty : customPropertiesInSDM) { + if (sdmResponseProperties.contains(targetSdmProperty) + && !validSecondaryProperties.contains(targetSdmProperty)) { + invalidProperties.add(targetSdmProperty); + logger.warn( + "Copy validation - Attachment {} has invalid secondary property '{}'" + + " (present in SDM response but not in valid secondary properties list)", + objectId, + targetSdmProperty); + } + } + return invalidProperties; + } + + /** Builds and emits a warning message for copy failures. */ + private void buildAndWarnCopyFailures( + List> copyFailures, AttachmentCopyEventContext context) { + StringBuilder warningMessage = + new StringBuilder(SDMUtils.getErrorMessage("FAILED_TO_COPY_ATTACHMENTS_PREFIX")); + for (Map failure : copyFailures) { + warningMessage + .append("- ObjectId: ") + .append(failure.get(OBJECT_ID_KEY)) + .append(", Reason: ") + .append(failure.get(FAILURE_REASON_KEY)) + .append("\n"); + } + context.getMessages().warn(warningMessage.toString()); + } + /** * Moves attachments from source entity to target entity in SDM. Executes moves in parallel, * updates database records, and cleans up source metadata. If any step fails, the operation is From 482cadb4d90617491cd745f48b5b6e8588203ef6 Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 18:02:49 +0530 Subject: [PATCH 13/16] fix --- .../handler/SDMCustomServiceHandler.java | 158 ++++++------------ .../handler/SDMCustomServiceHandlerTest.java | 32 ++-- 2 files changed, 64 insertions(+), 126 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index 1e3ecb539..d57c52feb 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -127,41 +127,33 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti Boolean isSystemUser = context.getSystemUser(); SDMCredentials sdmCredentials = tokenHandler.getSDMCredentials(); - // Check if folder exists before trying to create it - boolean folderExists = - sdmService.getFolderIdByPath(folderName, repositoryId, sdmCredentials, isSystemUser) - != null; - String folderId = ensureFolderExists(folderName, repositoryId, sdmCredentials, isSystemUser); List objectIds = context.getObjectIds(); - // Pre-copy validation: Check for invalid secondary properties before copying - List> copyFailures = new ArrayList<>(); - List validObjectIds = - validateObjectIdsForCopy( - objectIds, - customPropertiesInSDM, - repositoryId, - sdmCredentials, - isSystemUser, - copyFailures); - - // Show warning if there are attachments with invalid secondary properties - if (!copyFailures.isEmpty()) { - buildAndWarnCopyFailures(copyFailures, context); + // Pre-copy validation: Block copy if any source attachment has invalid secondary properties + // This check runs BEFORE folder creation to avoid creating empty folders in SDM + if (!customPropertiesInSDM.isEmpty()) { + List> copyFailures = + findAttachmentsWithInvalidSecondaryProperties( + objectIds, customPropertiesInSDM, repositoryId, sdmCredentials, isSystemUser); + if (!copyFailures.isEmpty()) { + buildAndWarnCopyFailures(copyFailures, context); + context.setCompleted(); + logger.debug("END: Copy attachments event"); + return; + } } - if (validObjectIds.isEmpty()) { - logger.info("No valid attachments to copy after pre-copy validation for upID: {}", upID); - context.setCompleted(); - logger.debug("END: Copy attachments event"); - return; - } + // Check if folder exists before trying to create it + boolean folderExists = + sdmService.getFolderIdByPath(folderName, repositoryId, sdmCredentials, isSystemUser) + != null; + String folderId = ensureFolderExists(folderName, repositoryId, sdmCredentials, isSystemUser); CopyAttachmentsRequest request = CopyAttachmentsRequest.builder() .context(context) - .objectIds(validObjectIds) + .objectIds(objectIds) .folderId(folderId) .repositoryId(repositoryId) .sdmCredentials(sdmCredentials) @@ -202,120 +194,78 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti } /** - * Validates object IDs for copy by checking for invalid secondary properties. Attachments with - * invalid properties are excluded and added to the failures list. + * Checks source attachments for invalid secondary properties before copy. Returns a list of + * failures if any attachment has invalid properties; returns empty list if all are valid. */ - private List validateObjectIdsForCopy( + private List> findAttachmentsWithInvalidSecondaryProperties( List objectIds, Set customPropertiesInSDM, String repositoryId, SDMCredentials sdmCredentials, - Boolean isSystemUser, - List> copyFailures) + Boolean isSystemUser) throws IOException { - if (customPropertiesInSDM.isEmpty()) { - return new ArrayList<>(objectIds); - } - List secondaryTypes = sdmService.getSecondaryTypes(repositoryId, sdmCredentials, isSystemUser); List validSecondaryProperties = sdmService.getValidSecondaryProperties( secondaryTypes, sdmCredentials, repositoryId, isSystemUser); - logger.debug( - "Copy validation - checking {} attachments against {} valid secondary properties", - objectIds.size(), - validSecondaryProperties.size()); - - List validObjectIds = new ArrayList<>(); + List> failures = new ArrayList<>(); for (String objectId : objectIds) { - validateSingleObjectForCopy( - objectId, - customPropertiesInSDM, - validSecondaryProperties, - sdmCredentials, - isSystemUser, - validObjectIds, - copyFailures); + List invalidProperties = + getInvalidPropertiesForObject( + objectId, + customPropertiesInSDM, + validSecondaryProperties, + sdmCredentials, + isSystemUser); + if (!invalidProperties.isEmpty()) { + Map failure = new HashMap<>(); + failure.put(OBJECT_ID_KEY, objectId); + failure.put( + FAILURE_REASON_KEY, + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") + + String.join(", ", invalidProperties) + + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); + failures.add(failure); + } } - return validObjectIds; + return failures; } /** - * Validates a single attachment for copy by checking its secondary properties against the valid - * list. Adds to validObjectIds if valid, or to copyFailures if invalid. + * Gets the list of invalid secondary properties for a single object from SDM. Returns empty list + * if all properties are valid or if metadata cannot be fetched. */ - private void validateSingleObjectForCopy( + private List getInvalidPropertiesForObject( String objectId, Set customPropertiesInSDM, List validSecondaryProperties, SDMCredentials sdmCredentials, - Boolean isSystemUser, - List validObjectIds, - List> copyFailures) { + Boolean isSystemUser) { try { JSONObject sdmMetadata = sdmService.getObject(objectId, sdmCredentials, isSystemUser); if (sdmMetadata == null || !sdmMetadata.has("succinctProperties")) { - validObjectIds.add(objectId); - return; + return Collections.emptyList(); } - JSONObject succinctProperties = sdmMetadata.getJSONObject("succinctProperties"); Set sdmResponseProperties = new HashSet<>(succinctProperties.keySet()); - List invalidProperties = - findInvalidSecondaryProperties( - customPropertiesInSDM, sdmResponseProperties, validSecondaryProperties, objectId); - - if (invalidProperties.isEmpty()) { - validObjectIds.add(objectId); - } else { - Map failure = new HashMap<>(); - failure.put(OBJECT_ID_KEY, objectId); - failure.put( - FAILURE_REASON_KEY, - SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_PREFIX") - + String.join(", ", invalidProperties) - + SDMUtils.getErrorMessage("INVALID_SECONDARY_PROPERTIES_FOR_COPY_SUFFIX")); - copyFailures.add(failure); - logger.warn( - "Copy validation - Skipping attachment {} due to {} invalid secondary properties: {}", - objectId, - invalidProperties.size(), - invalidProperties); + List invalidProperties = new ArrayList<>(); + for (String targetSdmProperty : customPropertiesInSDM) { + if (sdmResponseProperties.contains(targetSdmProperty) + && !validSecondaryProperties.contains(targetSdmProperty)) { + invalidProperties.add(targetSdmProperty); + } } + return invalidProperties; } catch (IOException e) { logger.error( "Copy validation - Failed to fetch metadata for attachment {}: {}", objectId, e.getMessage()); - validObjectIds.add(objectId); - } - } - - /** - * Finds secondary properties that are present in the SDM response but not in the valid properties - * list. - */ - private List findInvalidSecondaryProperties( - Set customPropertiesInSDM, - Set sdmResponseProperties, - List validSecondaryProperties, - String objectId) { - List invalidProperties = new ArrayList<>(); - for (String targetSdmProperty : customPropertiesInSDM) { - if (sdmResponseProperties.contains(targetSdmProperty) - && !validSecondaryProperties.contains(targetSdmProperty)) { - invalidProperties.add(targetSdmProperty); - logger.warn( - "Copy validation - Attachment {} has invalid secondary property '{}'" - + " (present in SDM response but not in valid secondary properties list)", - objectId, - targetSdmProperty); - } + return Collections.emptyList(); } - return invalidProperties; } /** Builds and emits a warning message for copy failures. */ diff --git a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java index caf9a5874..c233b8fbb 100644 --- a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java +++ b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java @@ -326,7 +326,7 @@ void testCopyAttachments_AttachmentCopyFails_FolderExists_AttachmentsDeleted() } @Test - void testCopyAttachments_InvalidSecondaryProperty_SkipsAttachment() throws IOException { + void testCopyAttachments_InvalidSecondaryProperty_BlocksCopy() throws IOException { // Mock SDMCredentials SDMCredentials sdmCredentials = mock(SDMCredentials.class); when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); @@ -355,7 +355,7 @@ void testCopyAttachments_InvalidSecondaryProperty_SkipsAttachment() throws IOExc // Act sdmCustomServiceHandler.copyAttachments(context); - // Assert: attachment should be skipped, no copy operation performed + // Assert: copy should be blocked entirely, no copy operation performed verify(sdmService, never()) .copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any()); // Warning message should be issued @@ -364,7 +364,8 @@ void testCopyAttachments_InvalidSecondaryProperty_SkipsAttachment() throws IOExc } @Test - void testCopyAttachments_MixedValidAndInvalidSecondaryProps_CopiesOnlyValid() throws IOException { + void testCopyAttachments_MixedValidAndInvalidSecondaryProps_BlocksEntireCopy() + throws IOException { // Mock SDMCredentials SDMCredentials sdmCredentials = mock(SDMCredentials.class); when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); @@ -382,7 +383,6 @@ void testCopyAttachments_MixedValidAndInvalidSecondaryProps_CopiesOnlyValid() th // Mock secondary types and valid secondary properties when(sdmService.getSecondaryTypes(any(), any(), anyBoolean())) .thenReturn(List.of("secondaryType1")); - // "invalidCustomProp" is NOT in the valid list - only "validProp1" and "validProp2" are valid when(sdmService.getValidSecondaryProperties(any(), any(), any(), anyBoolean())) .thenReturn(List.of("validProp1", "validProp2")); @@ -390,38 +390,26 @@ void testCopyAttachments_MixedValidAndInvalidSecondaryProps_CopiesOnlyValid() th JSONObject validMetadata = new JSONObject(); JSONObject validProps = new JSONObject(); validProps.put("cmis:name", "valid.pdf"); - validProps.put("validProp1", "someValue"); // only valid properties present + validProps.put("validProp1", "someValue"); validMetadata.put("succinctProperties", validProps); when(sdmService.getObject(eq(validObjectId), any(), anyBoolean())).thenReturn(validMetadata); - // Mock getObject for invalid attachment (HAS "invalidCustomProp" in response, which is NOT in - // valid list) + // Mock getObject for invalid attachment (HAS "invalidCustomProp" which is NOT in valid list) JSONObject invalidMetadata = new JSONObject(); JSONObject invalidProps = new JSONObject(); invalidProps.put("cmis:name", "invalid.pdf"); - invalidProps.put("invalidCustomProp", "someValue"); // present in response but NOT in valid list + invalidProps.put("invalidCustomProp", "someValue"); invalidMetadata.put("succinctProperties", invalidProps); when(sdmService.getObject(eq(invalidObjectId), any(), anyBoolean())) .thenReturn(invalidMetadata); - // Mock attachment copy for valid attachment - Map attachmentData = new HashMap<>(); - attachmentData.put("cmis:name", "valid.pdf"); - attachmentData.put("cmis:contentStreamMimeType", "application/pdf"); - attachmentData.put("cmis:objectId", validObjectId); - when(sdmService.copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any())) - .thenReturn(attachmentData); - CmisDocument cmisDocument = new CmisDocument(); - cmisDocument.setType("sap-icon://document"); - when(dbQuery.getAttachmentForObjectID(any(), any(), any(AttachmentCopyEventContext.class))) - .thenReturn(cmisDocument); - // Act sdmCustomServiceHandler.copyAttachments(context); - // Assert: only valid attachment should be copied - verify(sdmService, times(1)) + // Assert: entire copy should be blocked since one attachment has invalid properties + verify(sdmService, never()) .copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any()); + verify(context.getMessages(), times(1)).warn(any(String.class)); verify(context, times(1)).setCompleted(); } From 12e287de8503ef475de7ae9c925fc4be096e8be6 Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 18:13:29 +0530 Subject: [PATCH 14/16] added comment --- .../sap/cds/sdm/service/handler/SDMCustomServiceHandler.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index d57c52feb..9f89e22f0 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -130,8 +130,6 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti List objectIds = context.getObjectIds(); - // Pre-copy validation: Block copy if any source attachment has invalid secondary properties - // This check runs BEFORE folder creation to avoid creating empty folders in SDM if (!customPropertiesInSDM.isEmpty()) { List> copyFailures = findAttachmentsWithInvalidSecondaryProperties( From 0ed9794d804bf087de2eecad0153bef29f32d3b8 Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 19:09:56 +0530 Subject: [PATCH 15/16] fixed multiple copy --- .../handler/SDMCustomServiceHandler.java | 21 ++++++++++++++++--- .../handler/SDMCustomServiceHandlerTest.java | 20 +++++++++++++++--- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index 9f89e22f0..f1f56886b 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -136,9 +136,24 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti objectIds, customPropertiesInSDM, repositoryId, sdmCredentials, isSystemUser); if (!copyFailures.isEmpty()) { buildAndWarnCopyFailures(copyFailures, context); - context.setCompleted(); - logger.debug("END: Copy attachments event"); - return; + // Remove invalid objectIds and proceed with valid ones + Set invalidObjectIds = + copyFailures.stream() + .map(failure -> failure.get(OBJECT_ID_KEY)) + .collect(Collectors.toSet()); + objectIds = + objectIds.stream() + .filter(id -> !invalidObjectIds.contains(id)) + .collect(Collectors.toList()); + if (objectIds.isEmpty()) { + context.setCompleted(); + logger.debug("END: Copy attachments event - all attachments have invalid properties"); + return; + } + logger.info( + "Proceeding with {} valid attachments after filtering out {} invalid ones", + objectIds.size(), + invalidObjectIds.size()); } } diff --git a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java index c233b8fbb..d992c9a86 100644 --- a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java +++ b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMCustomServiceHandlerTest.java @@ -364,7 +364,7 @@ void testCopyAttachments_InvalidSecondaryProperty_BlocksCopy() throws IOExceptio } @Test - void testCopyAttachments_MixedValidAndInvalidSecondaryProps_BlocksEntireCopy() + void testCopyAttachments_MixedValidAndInvalidSecondaryProps_CopiesValidRejectsInvalid() throws IOException { // Mock SDMCredentials SDMCredentials sdmCredentials = mock(SDMCredentials.class); @@ -403,13 +403,27 @@ void testCopyAttachments_MixedValidAndInvalidSecondaryProps_BlocksEntireCopy() when(sdmService.getObject(eq(invalidObjectId), any(), anyBoolean())) .thenReturn(invalidMetadata); + // Mock attachment metadata and copy for the valid attachment + CmisDocument cmisDocument = new CmisDocument(); + cmisDocument.setType("sap-icon://document"); + when(dbQuery.getAttachmentForObjectID(any(), any(), any(AttachmentCopyEventContext.class))) + .thenReturn(cmisDocument); + + Map attachmentData = new HashMap<>(); + attachmentData.put("cmis:name", "valid.pdf"); + attachmentData.put("cmis:contentStreamMimeType", "application/pdf"); + attachmentData.put("cmis:objectId", "newCopiedObjectId"); + when(sdmService.copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any())) + .thenReturn(attachmentData); + // Act sdmCustomServiceHandler.copyAttachments(context); - // Assert: entire copy should be blocked since one attachment has invalid properties - verify(sdmService, never()) + // Assert: valid attachment should be copied, invalid one should be warned about + verify(sdmService, times(1)) .copyAttachment(any(), any(SDMCredentials.class), anyBoolean(), any()); verify(context.getMessages(), times(1)).warn(any(String.class)); + verify(draftService, times(1)).newDraft(any()); verify(context, times(1)).setCompleted(); } From 58dca1a96eb0f48cd5abea31da226d39e28a2bf1 Mon Sep 17 00:00:00 2001 From: deepiksSingh2711 Date: Wed, 20 May 2026 19:58:12 +0530 Subject: [PATCH 16/16] change --- .../cds/sdm/service/handler/SDMCustomServiceHandler.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java index f1f56886b..d7789e67c 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMCustomServiceHandler.java @@ -208,7 +208,7 @@ public void copyAttachments(AttachmentCopyEventContext context) throws IOExcepti /** * Checks source attachments for invalid secondary properties before copy. Returns a list of - * failures if any attachment has invalid properties; returns empty list if all are valid. + * failures if any attachment has invalid properties; returns empty list if all are valid */ private List> findAttachmentsWithInvalidSecondaryProperties( List objectIds, @@ -248,7 +248,7 @@ private List> findAttachmentsWithInvalidSecondaryProperties( /** * Gets the list of invalid secondary properties for a single object from SDM. Returns empty list - * if all properties are valid or if metadata cannot be fetched. + * if all properties are valid or if metadata cannot be fetched */ private List getInvalidPropertiesForObject( String objectId, @@ -281,7 +281,7 @@ private List getInvalidPropertiesForObject( } } - /** Builds and emits a warning message for copy failures. */ + /** Builds and emits a warning message for copy failures */ private void buildAndWarnCopyFailures( List> copyFailures, AttachmentCopyEventContext context) { StringBuilder warningMessage =