Skip to content

Commit 7bc622b

Browse files
rashmiangadi05rashmiangadi05
committed
Review comments
1 parent 91b392d commit 7bc622b

5 files changed

Lines changed: 31 additions & 140 deletions

File tree

‎sdm/src/main/java/com/sap/cds/sdm/constants/SDMConstants.java‎

Lines changed: 5 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,14 @@ private SDMConstants() {
1717
"SDM.Attachments.AdditionalProperty.name";
1818
public static final String SDM_ANNOTATION_ADDITIONALPROPERTY =
1919
"SDM.Attachments.AdditionalProperty";
20-
public static final String DUPLICATE_FILE_IN_DRAFT_ERROR_MESSAGE =
21-
"The file(s) %s have been added multiple times. Please rename and try again.";
22-
public static final String FILES_RENAME_WARNING_MESSAGE =
23-
"The following files could not be renamed as they already exist:\n%s\n";
24-
public static final String COULD_NOT_UPDATE_THE_ATTACHMENT = "Could not update the attachment";
25-
public static final String ATTACHMENT_NOT_FOUND = "Attachment not found";
20+
2621
public static final String GENERIC_ERROR = "Could not %s the document.";
27-
public static final String VERSIONED_REPO_ERROR =
28-
"Upload not supported for versioned repositories.";
29-
public static final String VIRUS_REPO_ERROR_MORE_THAN_400MB =
30-
"You cannot upload files that are larger than 400 MB";
31-
public static final String VIRUS_REPO_ERROR_MORE_THAN_400MB_MESSAGE = "SDM.VirusRepoErrorMessage";
22+
3223
public static final String VIRUS_ERROR = "%s contains potential malware and cannot be uploaded.";
33-
public static final String VIRUS_ERROR_MESSAGE = "SDM.VirusErrorMessage";
34-
public static final String SDM_DUPLICATE_ATTACHMENT = "SDM.DuplicateAttachment";
35-
public static final String REPOSITORY_ERROR = "Failed to get repository info.";
24+
3625
public static final String SDM_MISSING_ROLES_EXCEPTION_MSG =
3726
"You do not have the required permissions to update attachments. Kindly contact the admin";
38-
public static final String SDM_ROLES_ERROR_MESSAGE =
39-
"Unable to rename the file due to an error at the server";
27+
4028
public static final String SDM_ENV_NAME = "sdm";
4129
public static final String ENTITY_PROCESSING_ERROR_LINK =
4230
"Failed to create link due to error while processing entity";
@@ -47,28 +35,16 @@ private SDMConstants() {
4735
public static final String SDM_CONNECTIONPOOL_PREFIX = "cds.attachments.sdm.http.%s";
4836
public static final String USER_NOT_AUTHORISED_ERROR =
4937
"You do not have the required permissions to upload attachments. Please contact your administrator for access.";
50-
public static final String MIMETYPE_INVALID_ERROR =
51-
"This file type is not allowed in this repository. Contact your administrator for assistance.";
52-
public static final String USER_NOT_AUTHORISED_ERROR_LINK =
53-
"You do not have the required permissions to create links. Please contact your administrator for access.";
38+
5439
public static final String USER_NOT_AUTHORISED_ERROR_OPEN_LINK =
5540
"You do not have the required permissions to open links. Please contact your administrator for access.";
5641
public static final String FILE_NOT_FOUND_ERROR = "Object not found in repository";
5742
public static final Integer MAX_CONNECTIONS = 100;
5843
public static final int CONNECTION_TIMEOUT = 1200;
5944
public static final int CHUNK_SIZE = 20 * 1024 * 1024; // 20MB Chunk Size
60-
public static final String ONBOARD_REPO_MESSAGE =
61-
"Repository with name %s and id %s onboarded successfully";
62-
public static final String REPOSITORY_ALREADY_EXIST =
63-
"Repository with name %s and id %s already exists. Skipping onboarding.";
64-
public static final String ONBOARD_REPO_ERROR_MESSAGE =
65-
"Error in onboarding repository with name %s";
66-
public static final String UPDATE_ATTACHMENT_ERROR = "Could not update the attachment";
6745
public static final String ATTACHMENT_MAXCOUNT = "SDM.Attachments.maxCount";
68-
public static final String ATTACHMENT_MAXCOUNT_ERROR_MSG = "SDM.Attachments.maxCountError";
6946
public static final String MAX_COUNT_ERROR_MESSAGE =
7047
"Cannot upload more than %s attachments as set up by the application";
71-
public static final String FETCH_CHANGELOG_ERROR = "Could not fetch the changelog";
7248
public static final String DRAFT_READONLY_CONTEXT = "DRAFT_READONLY_CONTEXT";
7349
public static final Integer TIMEOUT_MILLISECONDS = 900000;
7450
public static final Integer MAX_CONNECTIONS_PER_ROUTE = 50;
@@ -77,29 +53,6 @@ private SDMConstants() {
7753
public static final String TECHNICAL_USER_FLOW = "TECHNICAL_CREDENTIALS_FLOW";
7854
public static final String NAMED_USER_FLOW = "TOKEN_EXCHANGE";
7955
public static final String ANNOTATION_IS_MEDIA_DATA = "_is_media_data";
80-
public static final String FAILED_TO_COPY_ATTACHMENT = "Failed to copy attachment";
81-
82-
// Error messages for move operations
83-
public static final String SDM_MOVE_OPERATION_FAILED = "SDM move operation failed";
84-
public static final String VALIDATION_FAILED_PREFIX = "Validation failed: ";
85-
public static final String VALIDATION_FAILED_DEFAULT_MESSAGE =
86-
"Validation failed: Unable to process attachment properties or metadata";
87-
public static final String INVALID_SECONDARY_PROPERTIES_PREFIX =
88-
"Invalid secondary properties detected: ";
89-
public static final String INVALID_SECONDARY_PROPERTIES_SUFFIX =
90-
". Attachment rolled back to source.";
91-
public static final String FAILED_TO_MOVE_ATTACHMENT = "Failed to move attachment";
92-
public static final String FAILED_TO_MOVE_ATTACHMENT_MSG = "SDM.Move.failedToMoveAttachmentError";
93-
public static final String MOVE_OPERATION_PARTIAL_FAILURE =
94-
"Move operation completed with some failures";
95-
public static final String FAILED_TO_FETCH_UP_ID = "Failed to fetch up_id";
96-
public static final String FAILED_TO_FETCH_FACET =
97-
"Invalid facet format, unable to extract required information.";
98-
public static final String PARENT_ENTITY_NOT_FOUND_ERROR = "Unable to find parent entity: %s";
99-
public static final String COMPOSITION_NOT_FOUND_ERROR =
100-
"Unable to find composition '%s' in entity: %s";
101-
public static final String TARGET_ATTACHMENT_ENTITY_NOT_FOUND_ERROR =
102-
"Unable to find target attachment entity: %s";
10356

10457
public static final String SINGLE_RESTRICTED_CHARACTER_IN_FILE =
10558
"\"%s\" contains unsupported characters (‘/’ or ‘\\’). Rename and try again.";

‎sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/SDMCreateAttachmentsHandler.java‎

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,6 @@ private void processEntity(
228228
String targetEntity = context.getTarget().getQualifiedName();
229229
List<Map<String, Object>> attachments =
230230
AttachmentsHandlerUtils.fetchAttachments(targetEntity, entity, attachmentCompositionName);
231-
List<String> virusDetectedFiles = new ArrayList<>();
232-
List<String> virusScanInProgressFiles = new ArrayList<>();
233231
List<String> scanFailedFiles = new ArrayList<>();
234232
List<String> uploadInProgressFiles = new ArrayList<>();
235233

@@ -247,16 +245,12 @@ private void processEntity(
247245
attachmentEntity,
248246
secondaryPropertiesWithInvalidDefinitions,
249247
noSDMRoles,
250-
virusDetectedFiles,
251-
virusScanInProgressFiles,
252248
scanFailedFiles,
253249
uploadInProgressFiles);
254250
}
255251

256-
// Throw exception if any files failed virus scan or scan failed
257-
String errorMessage =
258-
buildErrorMessage(
259-
virusDetectedFiles, virusScanInProgressFiles, scanFailedFiles, uploadInProgressFiles);
252+
// Throw exception if any files failed scan or upload in progress
253+
String errorMessage = buildErrorMessage(scanFailedFiles, uploadInProgressFiles);
260254
if (!errorMessage.isEmpty()) {
261255
throw new ServiceException(errorMessage);
262256
}
@@ -280,8 +274,6 @@ private void processAttachment(
280274
Optional<CdsEntity> attachmentEntity,
281275
Map<String, String> secondaryPropertiesWithInvalidDefinitions,
282276
List<String> noSDMRoles,
283-
List<String> virusDetectedFiles,
284-
List<String> virusScanInProgressFiles,
285277
List<String> scanFailedFiles,
286278
List<String> uploadInProgressFiles)
287279
throws IOException {
@@ -297,13 +289,7 @@ private void processAttachment(
297289

298290
// Check upload status and collect problematic files
299291
if (checkUploadStatus(
300-
attachment,
301-
fileNameInDB,
302-
filenameInRequest,
303-
virusDetectedFiles,
304-
virusScanInProgressFiles,
305-
scanFailedFiles,
306-
uploadInProgressFiles)) {
292+
attachment, fileNameInDB, filenameInRequest, scanFailedFiles, uploadInProgressFiles)) {
307293
return; // Skip further processing if upload status is problematic
308294
}
309295

@@ -336,8 +322,6 @@ private boolean checkUploadStatus(
336322
Map<String, Object> attachment,
337323
String fileNameInDB,
338324
String filenameInRequest,
339-
List<String> virusDetectedFiles,
340-
List<String> virusScanInProgressFiles,
341325
List<String> scanFailedFiles,
342326
List<String> uploadInProgressFiles) {
343327
Map<String, Object> readonlyData = (Map<String, Object>) attachment.get(SDM_READONLY_CONTEXT);
@@ -348,14 +332,6 @@ private boolean checkUploadStatus(
348332
String uploadStatus = readonlyData.get("uploadStatus").toString();
349333
String fileName = fileNameInDB != null ? fileNameInDB : filenameInRequest;
350334

351-
if (uploadStatus.equalsIgnoreCase(SDMConstants.UPLOAD_STATUS_VIRUS_DETECTED)) {
352-
virusDetectedFiles.add(fileName);
353-
return true;
354-
}
355-
if (uploadStatus.equalsIgnoreCase(SDMConstants.VIRUS_SCAN_INPROGRESS)) {
356-
virusScanInProgressFiles.add(fileName);
357-
return true;
358-
}
359335
if (uploadStatus.equalsIgnoreCase(SDMConstants.UPLOAD_STATUS_IN_PROGRESS)) {
360336
uploadInProgressFiles.add(fileName);
361337
return true;
@@ -539,20 +515,9 @@ private void handleWarnings(
539515
}
540516

541517
private String buildErrorMessage(
542-
List<String> virusDetectedFiles,
543-
List<String> virusScanInProgressFiles,
544-
List<String> scanFailedFiles,
545-
List<String> uploadInProgressFiles) {
518+
List<String> scanFailedFiles, List<String> uploadInProgressFiles) {
546519
StringBuilder errorMessage = new StringBuilder();
547520

548-
if (!virusDetectedFiles.isEmpty()) {
549-
errorMessage.append(SDMErrorMessages.virusDetectedFilesMessage(virusDetectedFiles));
550-
}
551-
if (!virusScanInProgressFiles.isEmpty()) {
552-
appendWithSpace(errorMessage);
553-
errorMessage.append(
554-
SDMErrorMessages.virusScanInProgressFilesMessage(virusScanInProgressFiles));
555-
}
556521
if (!scanFailedFiles.isEmpty()) {
557522
appendWithSpace(errorMessage);
558523
errorMessage.append(SDMErrorMessages.scanFailedFilesMessage(scanFailedFiles));

‎sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/SDMReadAttachmentsHandler.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ public SDMReadAttachmentsHandler(
6363
this.dbQuery = dbQuery;
6464
}
6565

66+
/*
67+
Error message caching requires the CAP context to retrieve localized messages, which may not be
68+
available at all error throw sites. To ensure availability, error messages are cached during the
69+
before read event when the context is guaranteed to be present.
70+
*/
6671
private void setErrorMessagesInCache(CdsReadEventContext context) {
6772
// Check if cache is available
6873
Cache<ErrorMessageKey, String> errorMessageCache = CacheConfig.getErrorMessageCache();
@@ -147,13 +152,17 @@ public void processBefore(CdsReadEventContext context) throws IOException {
147152

148153
if (!hasKeys) {
149154
// Apply repositoryId filter for collection reads
155+
Predicate repositoryFilter = CQL.get("repositoryId").eq(repositoryId);
150156
modifiedCqn =
151157
CQL.copy(
152158
modifiedCqn,
153159
new Modifier() {
154160
@Override
155161
public Predicate where(Predicate where) {
156-
return CQL.and(where, CQL.get("repositoryId").eq(repositoryId));
162+
if (where == null) {
163+
return repositoryFilter;
164+
}
165+
return CQL.and(where, repositoryFilter);
157166
}
158167
});
159168
}
@@ -164,8 +173,9 @@ public Predicate where(Predicate where) {
164173
// Re-throw to maintain error handling behavior
165174
throw e;
166175
}
176+
} else {
177+
context.setCqn(context.getCqn());
167178
}
168-
// No action needed for non-media entities
169179
}
170180

171181
/**

‎sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/SDMUpdateAttachmentsHandler.java‎

Lines changed: 3 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,6 @@ private void processAttachments(
237237
Map<String, String> secondaryPropertiesWithInvalidDefinitions,
238238
List<String> noSDMRoles)
239239
throws IOException {
240-
List<String> virusDetectedFiles = new ArrayList<>();
241-
List<String> virusScanInProgressFiles = new ArrayList<>();
242240
List<String> scanFailedFiles = new ArrayList<>();
243241
List<String> uploadInProgressFiles = new ArrayList<>();
244242

@@ -256,28 +254,13 @@ private void processAttachments(
256254
badRequest,
257255
secondaryPropertiesWithInvalidDefinitions,
258256
noSDMRoles,
259-
virusDetectedFiles,
260-
virusScanInProgressFiles,
261257
scanFailedFiles,
262258
uploadInProgressFiles);
263259
}
264260

265-
// Throw exception if any files failed virus scan or scan failed
266-
if (!virusDetectedFiles.isEmpty()
267-
|| !virusScanInProgressFiles.isEmpty()
268-
|| !scanFailedFiles.isEmpty()
269-
|| !uploadInProgressFiles.isEmpty()) {
261+
// Throw exception if any files failed scan or upload in progress
262+
if (!scanFailedFiles.isEmpty() || !uploadInProgressFiles.isEmpty()) {
270263
StringBuilder errorMessage = new StringBuilder();
271-
if (!virusDetectedFiles.isEmpty()) {
272-
errorMessage.append(SDMErrorMessages.virusDetectedFilesMessage(virusDetectedFiles));
273-
}
274-
if (!virusScanInProgressFiles.isEmpty()) {
275-
if (errorMessage.length() > 0) {
276-
errorMessage.append(" ");
277-
}
278-
errorMessage.append(
279-
SDMErrorMessages.virusScanInProgressFilesMessage(virusScanInProgressFiles));
280-
}
281264
if (!scanFailedFiles.isEmpty()) {
282265
if (errorMessage.length() > 0) {
283266
errorMessage.append(" ");
@@ -311,8 +294,6 @@ public void processAttachment(
311294
Map<String, String> badRequest,
312295
Map<String, String> secondaryPropertiesWithInvalidDefinitions,
313296
List<String> noSDMRoles,
314-
List<String> virusDetectedFiles,
315-
List<String> virusScanInProgressFiles,
316297
List<String> scanFailedFiles,
317298
List<String> uploadInProgressFiles)
318299
throws IOException {
@@ -331,13 +312,7 @@ public void processAttachment(
331312

332313
// Check for upload status issues
333314
if (handleUploadStatusCheck(
334-
attachment,
335-
fileNameInDB,
336-
filenameInRequest,
337-
virusDetectedFiles,
338-
virusScanInProgressFiles,
339-
scanFailedFiles,
340-
uploadInProgressFiles)) {
315+
attachment, fileNameInDB, filenameInRequest, scanFailedFiles, uploadInProgressFiles)) {
341316
return;
342317
}
343318

@@ -395,8 +370,6 @@ private boolean handleUploadStatusCheck(
395370
Map<String, Object> attachment,
396371
String fileNameInDB,
397372
String filenameInRequest,
398-
List<String> virusDetectedFiles,
399-
List<String> virusScanInProgressFiles,
400373
List<String> scanFailedFiles,
401374
List<String> uploadInProgressFiles) {
402375
Map<String, Object> readonlyData = (Map<String, Object>) attachment.get(SDM_READONLY_CONTEXT);
@@ -407,14 +380,6 @@ private boolean handleUploadStatusCheck(
407380
String uploadStatus = readonlyData.get("uploadStatus").toString();
408381
String fileName = fileNameInDB != null ? fileNameInDB : filenameInRequest;
409382

410-
if (uploadStatus.equalsIgnoreCase(SDMConstants.UPLOAD_STATUS_VIRUS_DETECTED)) {
411-
virusDetectedFiles.add(fileName);
412-
return true;
413-
}
414-
if (uploadStatus.equalsIgnoreCase(SDMConstants.VIRUS_SCAN_INPROGRESS)) {
415-
virusScanInProgressFiles.add(fileName);
416-
return true;
417-
}
418383
if (uploadStatus.equalsIgnoreCase(SDMConstants.UPLOAD_STATUS_SCAN_FAILED)) {
419384
scanFailedFiles.add(fileName);
420385
return true;

‎sdm/src/main/resources/cds/com.sap.cds/sdm/attachments.cds‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,23 +37,21 @@ annotate Attachments with @UI: {
3737
TypeNamePlural: '{i18n>Attachments}',
3838
},
3939
LineItem : [
40-
{Value: type, @HTML5.CssDefaults: {width: '10%'}},
41-
{Value: fileName, @HTML5.CssDefaults: {width: '20%'}},
42-
{Value: content, @HTML5.CssDefaults: {width: '0%'}},
43-
{Value: createdAt, @HTML5.CssDefaults: {width: '20%'}},
44-
{Value: createdBy, @HTML5.CssDefaults: {width: '20%'}},
45-
{Value: note, @HTML5.CssDefaults: {width: '20%'}},
46-
40+
{Value: fileName, @HTML5.CssDefaults: {width: '15%'}},
41+
{Value: content, @HTML5.CssDefaults: {width: '20%'}},
42+
{Value: createdAt, @HTML5.CssDefaults: {width: '20%'}},
43+
{Value: createdBy, @HTML5.CssDefaults: {width: '15%'}},
44+
{Value: note, @HTML5.CssDefaults: {width: '15%'}},
4745
{
4846
Value : uploadStatus,
4947
Criticality: uploadStatusNav.criticality,
5048
@Common.FieldControl: #ReadOnly,
51-
@HTML5.CssDefaults: {width: '10%'},
49+
@HTML5.CssDefaults: {width: '15%'},
5250
@UI.Hidden: IsActiveEntity
5351
},
5452
]
5553
} {
56-
note @(title: '{i18n>Description}', UI.MultiLineText);
54+
note @(title: '{i18n>Description}');
5755
fileName @(title: '{i18n>Filename}');
5856
modifiedAt @(odata.etag: null);
5957
uploadStatus @(title: '{i18n>uploadStatus}', Common.Text : uploadStatusNav.name, Common.TextArrangement : #TextOnly);

0 commit comments

Comments
 (0)