From b0831d5a5cc20b942d7b3259e45dc38d1b300e94 Mon Sep 17 00:00:00 2001 From: Ekaterina Kazachkova Date: Tue, 28 Feb 2023 15:11:01 +0300 Subject: [PATCH] Issue #3029: Optionally exclude archived files from GUI S3 storage listing - fix for tags browsing (#3097) --- .../datastorage/DataStorageApiService.java | 17 +++++--- .../datastorage/DataStorageController.java | 9 ++-- .../datastorage/DataStorageManager.java | 5 ++- .../DataStorageApiServiceFileTest.java | 42 ++++++++++++------- .../DataStorageItemControllerTest.java | 16 +++---- 5 files changed, 54 insertions(+), 35 deletions(-) diff --git a/api/src/main/java/com/epam/pipeline/acl/datastorage/DataStorageApiService.java b/api/src/main/java/com/epam/pipeline/acl/datastorage/DataStorageApiService.java index 086fa62c1e..0d8eb07197 100644 --- a/api/src/main/java/com/epam/pipeline/acl/datastorage/DataStorageApiService.java +++ b/api/src/main/java/com/epam/pipeline/acl/datastorage/DataStorageApiService.java @@ -302,14 +302,19 @@ public Map deleteDataStorageObjectTags(Long id, String path, Str return dataStorageManager.deleteDataStorageObjectTags(id, path, version, tags); } - @PreAuthorize(AclExpressions.STORAGE_ID_READ) - public AbstractDataStorageItem getDataStorageItemWithTags(Long id, String path, Boolean showVersion) { - return dataStorageManager.getDataStorageItemWithTags(id, path, showVersion); + @PreAuthorize(AclExpressions.STORAGE_ID_READ + AclExpressions.AND + + AclExpressions.STORAGE_SHOW_ARCHIVED_PERMISSIONS) + public AbstractDataStorageItem getDataStorageItemWithTags(final Long id, final String path, + final Boolean showVersion, final boolean showArchived) { + return dataStorageManager.getDataStorageItemWithTags(id, path, showVersion, showArchived); } - @PreAuthorize(AclExpressions.STORAGE_ID_OWNER) - public AbstractDataStorageItem getDataStorageItemOwnerWithTags(Long id, String path, Boolean showVersion) { - return dataStorageManager.getDataStorageItemWithTags(id, path, showVersion); + @PreAuthorize(AclExpressions.STORAGE_ID_OWNER + AclExpressions.AND + + AclExpressions.STORAGE_SHOW_ARCHIVED_PERMISSIONS) + public AbstractDataStorageItem getDataStorageItemOwnerWithTags(final Long id, final String path, + final Boolean showVersion, + final boolean showArchived) { + return dataStorageManager.getDataStorageItemWithTags(id, path, showVersion, showArchived); } @PreAuthorize(AclExpressions.STORAGE_ID_OWNER) diff --git a/api/src/main/java/com/epam/pipeline/controller/datastorage/DataStorageController.java b/api/src/main/java/com/epam/pipeline/controller/datastorage/DataStorageController.java index 2b1a4b9f20..78d037c7c5 100644 --- a/api/src/main/java/com/epam/pipeline/controller/datastorage/DataStorageController.java +++ b/api/src/main/java/com/epam/pipeline/controller/datastorage/DataStorageController.java @@ -685,10 +685,13 @@ public Result> deleteTagsById(@PathVariable(value = ID) fina public Result getDataStorageItemsWithTags( @PathVariable(value = ID) final Long id, @RequestParam(value = PATH) final String path, - @RequestParam(defaultValue = FALSE) final Boolean showVersion) { + @RequestParam(defaultValue = FALSE) final Boolean showVersion, + @RequestParam(defaultValue = FALSE) final boolean showArchived) { return showVersion - ? Result.success(dataStorageApiService.getDataStorageItemOwnerWithTags(id, path, showVersion)) - : Result.success(dataStorageApiService.getDataStorageItemWithTags(id, path, showVersion)); + ? Result.success(dataStorageApiService.getDataStorageItemOwnerWithTags( + id, path, showVersion, showArchived)) + : Result.success(dataStorageApiService.getDataStorageItemWithTags( + id, path, showVersion, showArchived)); } @RequestMapping(value = "/datastorage/tags/search", method = RequestMethod.POST) diff --git a/api/src/main/java/com/epam/pipeline/manager/datastorage/DataStorageManager.java b/api/src/main/java/com/epam/pipeline/manager/datastorage/DataStorageManager.java index adc29cd2b3..2200cadfa4 100644 --- a/api/src/main/java/com/epam/pipeline/manager/datastorage/DataStorageManager.java +++ b/api/src/main/java/com/epam/pipeline/manager/datastorage/DataStorageManager.java @@ -754,9 +754,10 @@ public Map deleteDataStorageObjectTags(Long id, String path, Str @Transactional public AbstractDataStorageItem getDataStorageItemWithTags(final Long dataStorageId, final String path, - final Boolean showVersion) { + final Boolean showVersion, + final boolean showArchived) { final List dataStorageItems = getDataStorageItems(dataStorageId, path, showVersion, - null, null, false).getResults(); + null, null, showArchived).getResults(); if (CollectionUtils.isEmpty(dataStorageItems)) { return null; } diff --git a/api/src/test/java/com/epam/pipeline/acl/datastorage/DataStorageApiServiceFileTest.java b/api/src/test/java/com/epam/pipeline/acl/datastorage/DataStorageApiServiceFileTest.java index c81c6c52f8..81dad1b1ce 100644 --- a/api/src/test/java/com/epam/pipeline/acl/datastorage/DataStorageApiServiceFileTest.java +++ b/api/src/test/java/com/epam/pipeline/acl/datastorage/DataStorageApiServiceFileTest.java @@ -891,10 +891,11 @@ public void shouldDenyDeleteObjectTagsWhenSharedPermissionIsNotGranted() { @Test @WithMockUser(roles = ADMIN_ROLE) public void shouldGetItemWithTagsForAdmin() { - doReturn(dataStorageFile).when(mockDataStorageManager).getDataStorageItemWithTags(ID, TEST_STRING, true); + doReturn(dataStorageFile).when(mockDataStorageManager) + .getDataStorageItemWithTags(ID, TEST_STRING, true, false); mockUserContext(context); - assertThat(dataStorageApiService.getDataStorageItemWithTags(ID, TEST_STRING, true)) + assertThat(dataStorageApiService.getDataStorageItemWithTags(ID, TEST_STRING, true, false)) .isEqualTo(dataStorageFile); } @@ -902,10 +903,12 @@ public void shouldGetItemWithTagsForAdmin() { @WithMockUser(username = SIMPLE_USER) public void shouldGetItemWithTagsWhenPermissionIsGranted() { initAclEntity(s3bucket, AclPermission.READ); - doReturn(dataStorageFile).when(mockDataStorageManager).getDataStorageItemWithTags(ID, TEST_STRING, true); + doReturn(dataStorageFile).when(mockDataStorageManager) + .getDataStorageItemWithTags(ID, TEST_STRING, true, false); initUserAndEntityMocks(SIMPLE_USER, s3bucket, context); - assertThat(dataStorageApiService.getDataStorageItemWithTags(ID, TEST_STRING, true)) + assertThat(dataStorageApiService + .getDataStorageItemWithTags(ID, TEST_STRING, true, false)) .isEqualTo(dataStorageFile); } @@ -913,31 +916,34 @@ public void shouldGetItemWithTagsWhenPermissionIsGranted() { @WithMockUser public void shouldDenyGetItemWithTagsWhenStoragePermissionIsNotGranted() { initAclEntity(s3bucket); - doReturn(dataStorageFile).when(mockDataStorageManager).getDataStorageItemWithTags(ID, TEST_STRING, true); + doReturn(dataStorageFile).when(mockDataStorageManager) + .getDataStorageItemWithTags(ID, TEST_STRING, true, false); initUserAndEntityMocks(ANOTHER_SIMPLE_USER, s3bucket, context); assertThrows(AccessDeniedException.class, () -> dataStorageApiService - .getDataStorageItemWithTags(ID, TEST_STRING, true)); + .getDataStorageItemWithTags(ID, TEST_STRING, true, false)); } @Test @WithMockUser public void shouldDenyGetItemWithTagsWhenSharedPermissionIsNotGranted() { initAclEntity(notSharedS3bucket, AclPermission.READ); - doReturn(dataStorageFile).when(mockDataStorageManager).getDataStorageItemWithTags(ID, TEST_STRING, true); + doReturn(dataStorageFile).when(mockDataStorageManager) + .getDataStorageItemWithTags(ID, TEST_STRING, true, false); initUserAndEntityMocks(ANOTHER_SIMPLE_USER, notSharedS3bucket, externalContext); assertThrows(AccessDeniedException.class, () -> dataStorageApiService - .getDataStorageItemWithTags(ID, TEST_STRING, true)); + .getDataStorageItemWithTags(ID, TEST_STRING, true, false)); } @Test @WithMockUser(roles = ADMIN_ROLE) public void shouldGetItemOwnerWithTagsForAdmin() { - doReturn(dataStorageFile).when(mockDataStorageManager).getDataStorageItemWithTags(ID, TEST_STRING, true); + doReturn(dataStorageFile).when(mockDataStorageManager) + .getDataStorageItemWithTags(ID, TEST_STRING, true, false); mockUserContext(context); - assertThat(dataStorageApiService.getDataStorageItemOwnerWithTags(ID, TEST_STRING, true)) + assertThat(dataStorageApiService.getDataStorageItemOwnerWithTags(ID, TEST_STRING, true, false)) .isEqualTo(dataStorageFile); } @@ -945,10 +951,12 @@ public void shouldGetItemOwnerWithTagsForAdmin() { @WithMockUser public void shouldGetItemOwnerWithTagsWhenPermissionIsGranted() { initAclEntity(notSharedS3bucket, AclPermission.OWNER); - doReturn(dataStorageFile).when(mockDataStorageManager).getDataStorageItemWithTags(ID, TEST_STRING, true); + doReturn(dataStorageFile).when(mockDataStorageManager) + .getDataStorageItemWithTags(ID, TEST_STRING, true, false); initUserAndEntityMocks(SIMPLE_USER, notSharedS3bucket, context); - assertThat(dataStorageApiService.getDataStorageItemOwnerWithTags(ID, TEST_STRING, true)) + assertThat(dataStorageApiService + .getDataStorageItemOwnerWithTags(ID, TEST_STRING, true, false)) .isEqualTo(dataStorageFile); } @@ -956,22 +964,24 @@ public void shouldGetItemOwnerWithTagsWhenPermissionIsGranted() { @WithMockUser public void shouldDenyGetItemOwnerWithTagsWhenStoragePermissionIsNotGranted() { initAclEntity(s3bucket); - doReturn(dataStorageFile).when(mockDataStorageManager).getDataStorageItemWithTags(ID, TEST_STRING, true); + doReturn(dataStorageFile).when(mockDataStorageManager) + .getDataStorageItemWithTags(ID, TEST_STRING, true, false); initUserAndEntityMocks(ANOTHER_SIMPLE_USER, s3bucket, context); assertThrows(AccessDeniedException.class, () -> dataStorageApiService - .getDataStorageItemOwnerWithTags(ID, TEST_STRING, true)); + .getDataStorageItemOwnerWithTags(ID, TEST_STRING, true, false)); } @Test @WithMockUser public void shouldDenyGetItemOwnerWithTagsWhenSharedPermissionIsNotGranted() { initAclEntity(notSharedS3bucket, AclPermission.OWNER); - doReturn(dataStorageFile).when(mockDataStorageManager).getDataStorageItemWithTags(ID, TEST_STRING, true); + doReturn(dataStorageFile).when(mockDataStorageManager) + .getDataStorageItemWithTags(ID, TEST_STRING, true, false); initUserAndEntityMocks(ANOTHER_SIMPLE_USER, notSharedS3bucket, externalContext); assertThrows(AccessDeniedException.class, () -> dataStorageApiService - .getDataStorageItemOwnerWithTags(ID, TEST_STRING, true)); + .getDataStorageItemOwnerWithTags(ID, TEST_STRING, true, false)); } @Test diff --git a/api/src/test/java/com/epam/pipeline/controller/datastorage/DataStorageItemControllerTest.java b/api/src/test/java/com/epam/pipeline/controller/datastorage/DataStorageItemControllerTest.java index dfc2eb028d..79be7a3810 100644 --- a/api/src/test/java/com/epam/pipeline/controller/datastorage/DataStorageItemControllerTest.java +++ b/api/src/test/java/com/epam/pipeline/controller/datastorage/DataStorageItemControllerTest.java @@ -366,11 +366,11 @@ public void shouldGetDataStorageFileWithTags() { final MultiValueMap params = new LinkedMultiValueMap<>(); params.add(ID_PARAM, ID_AS_STRING); params.add(PATH, TEST); - Mockito.doReturn(file).when(mockStorageApiService).getDataStorageItemWithTags(ID, TEST, false); + Mockito.doReturn(file).when(mockStorageApiService).getDataStorageItemWithTags(ID, TEST, false, false); final MvcResult mvcResult = performRequest(get(String.format(TAGS_LIST_URL, ID)).params(params)); - Mockito.verify(mockStorageApiService).getDataStorageItemWithTags(ID, TEST, false); + Mockito.verify(mockStorageApiService).getDataStorageItemWithTags(ID, TEST, false, false); assertResponse(mvcResult, file, DatastorageCreatorUtils.DATA_STORAGE_FILE_TYPE); } @@ -381,11 +381,11 @@ public void shouldGetDataStorageFileOwnerWithTags() { params.add(ID_PARAM, ID_AS_STRING); params.add(PATH, TEST); params.add(SHOW_VERSION, TRUE_AS_STRING); - Mockito.doReturn(file).when(mockStorageApiService).getDataStorageItemOwnerWithTags(ID, TEST, true); + Mockito.doReturn(file).when(mockStorageApiService).getDataStorageItemOwnerWithTags(ID, TEST, true, false); final MvcResult mvcResult = performRequest(get(String.format(TAGS_LIST_URL, ID)).params(params)); - Mockito.verify(mockStorageApiService).getDataStorageItemOwnerWithTags(ID, TEST, true); + Mockito.verify(mockStorageApiService).getDataStorageItemOwnerWithTags(ID, TEST, true, false); assertResponse(mvcResult, file, DatastorageCreatorUtils.DATA_STORAGE_FILE_TYPE); } @@ -395,11 +395,11 @@ public void shouldGetDataStorageFolderWithTags() { final MultiValueMap params = new LinkedMultiValueMap<>(); params.add(ID_PARAM, ID_AS_STRING); params.add(PATH, TEST); - Mockito.doReturn(folder).when(mockStorageApiService).getDataStorageItemWithTags(ID, TEST, false); + Mockito.doReturn(folder).when(mockStorageApiService).getDataStorageItemWithTags(ID, TEST, false, false); final MvcResult mvcResult = performRequest(get(String.format(TAGS_LIST_URL, ID)).params(params)); - Mockito.verify(mockStorageApiService).getDataStorageItemWithTags(ID, TEST, false); + Mockito.verify(mockStorageApiService).getDataStorageItemWithTags(ID, TEST, false, false); assertResponse(mvcResult, folder, DatastorageCreatorUtils.DATA_STORAGE_FOLDER_TYPE); } @@ -410,11 +410,11 @@ public void shouldGetDataStorageFolderOwnerWithTags() { params.add(ID_PARAM, ID_AS_STRING); params.add(PATH, TEST); params.add(SHOW_VERSION, TRUE_AS_STRING); - Mockito.doReturn(folder).when(mockStorageApiService).getDataStorageItemOwnerWithTags(ID, TEST, true); + Mockito.doReturn(folder).when(mockStorageApiService).getDataStorageItemOwnerWithTags(ID, TEST, true, false); final MvcResult mvcResult = performRequest(get(String.format(TAGS_LIST_URL, ID)).params(params)); - Mockito.verify(mockStorageApiService).getDataStorageItemOwnerWithTags(ID, TEST, true); + Mockito.verify(mockStorageApiService).getDataStorageItemOwnerWithTags(ID, TEST, true, false); assertResponse(mvcResult, folder, DatastorageCreatorUtils.DATA_STORAGE_FOLDER_TYPE); }