Skip to content

Commit

Permalink
Fix remarks review and adapting tests.
Browse files Browse the repository at this point in the history
Signed-off-by: AAJELLAL <[email protected]>
  • Loading branch information
AAJELLAL committed May 27, 2024
1 parent 48aa888 commit 2d0aed8
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 42 deletions.
12 changes: 10 additions & 2 deletions src/main/java/org/gridsuite/explore/server/ExploreController.java
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,11 @@ public ResponseEntity<Void> replaceFilterWithScript(@PathVariable("id") UUID id,

@DeleteMapping(value = "/explore/elements/{elementUuid}")
@Operation(summary = "Remove directory/element")
@ApiResponses(@ApiResponse(responseCode = "200", description = "Directory/element was successfully removed"))
@ApiResponses(value = {
@ApiResponse(responseCode = "200", description = "Directory/element was successfully removed"),
@ApiResponse(responseCode = "404", description = "Directory/element was not found"),
@ApiResponse(responseCode = "403", description = "Access forbidden for the directory/element")
})
public ResponseEntity<Void> deleteElement(@PathVariable("elementUuid") UUID elementUuid,
@RequestHeader("userId") String userId) {
exploreService.deleteElement(elementUuid, userId);
Expand All @@ -216,7 +220,11 @@ public ResponseEntity<Void> deleteElement(@PathVariable("elementUuid") UUID elem

@DeleteMapping(value = "/explore/elements/{directoryUuid}", params = "ids")
@Operation(summary = "Remove directories/elements")
@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "directories/elements was successfully removed")})
@ApiResponses(value = {
@ApiResponse(responseCode = "200", description = "directories/elements was successfully removed"),
@ApiResponse(responseCode = "404", description = "At least one directory/element was not found"),
@ApiResponse(responseCode = "403", description = "Access forbidden for at least one directory/element")
})
public ResponseEntity<Void> deleteElements(@RequestParam("ids") List<UUID> elementsUuid,
@RequestHeader("userId") String userId,
@PathVariable UUID directoryUuid) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
public class ExploreException extends RuntimeException {

public enum Type {
NOT_FOUND,
NOT_ALLOWED,
UNKNOWN_ELEMENT_TYPE,
REMOTE_ERROR,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ protected ResponseEntity<Object> handleExploreException(ExploreException excepti
}
ExploreException exploreException = exception;
switch (exploreException.getType()) {
case NOT_FOUND:
return ResponseEntity.status(HttpStatus.NOT_FOUND).build();
case NOT_ALLOWED:
return ResponseEntity.status(HttpStatus.FORBIDDEN).body(NOT_ALLOWED);
case REMOTE_ERROR:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,10 @@
import org.gridsuite.explore.server.utils.ParametersType;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.core.ParameterizedTypeReference;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.MediaType;
import org.springframework.http.*;
import org.springframework.stereotype.Service;
import org.springframework.util.CollectionUtils;
import org.springframework.web.client.HttpStatusCodeException;
import org.springframework.web.client.RestTemplate;
import org.springframework.web.util.UriComponentsBuilder;

Expand All @@ -42,6 +40,9 @@ public class DirectoryService implements IDirectoryElementsService {
private static final String ELEMENTS_SERVER_ROOT_PATH = DELIMITER + DIRECTORY_SERVER_API_VERSION + DELIMITER
+ "elements";

private static final String PARAM_IDS = "ids";
private static final String PARAM_FOR_DELETION = "forDeletion";

private final Map<String, IDirectoryElementsService> genericServices;
private final RestTemplate restTemplate;
private String directoryServerBaseUri;
Expand Down Expand Up @@ -115,26 +116,35 @@ public void deleteDirectoryElement(UUID elementUuid, String userId) {
restTemplate.exchange(directoryServerBaseUri + path, HttpMethod.DELETE, new HttpEntity<>(headers), Void.class);
}

public Optional<Boolean> canDeleteDirectoryElement(List<UUID> elementUuids, String userId) {
public void areDirectoryElementsDeletable(List<UUID> elementUuids, String userId) {
var ids = elementUuids.stream().map(UUID::toString).collect(Collectors.joining(","));

HttpHeaders headers = new HttpHeaders();
headers.add(HEADER_USER_ID, userId);
String path = UriComponentsBuilder
.fromPath(ELEMENTS_SERVER_ROOT_PATH + "/can-delete")
.queryParam("ids", ids)
.fromPath(ELEMENTS_SERVER_ROOT_PATH)
.queryParam(PARAM_FOR_DELETION, true)
.queryParam(PARAM_IDS, ids)
.buildAndExpand()
.toUriString();
return Optional.ofNullable(restTemplate.exchange(directoryServerBaseUri + path, HttpMethod.GET, new HttpEntity<>(headers), Boolean.class)
.getBody());

try {
restTemplate.exchange(directoryServerBaseUri + path, HttpMethod.HEAD, new HttpEntity<>(headers), Void.class);
} catch (HttpStatusCodeException e) {
if (HttpStatus.FORBIDDEN.equals(e.getStatusCode())) {
throw new ExploreException(NOT_ALLOWED);
} else if (HttpStatus.NOT_FOUND.equals(e.getStatusCode())) {
throw new ExploreException(NOT_FOUND);
} else {
throw e;
}
}
}

public void deleteElementsFromDirectory(List<UUID> elementUuids, UUID parentDirectoryUuid, String userId) {
var ids = elementUuids.stream().map(UUID::toString).collect(Collectors.joining(","));
String path = UriComponentsBuilder
.fromPath(ELEMENTS_SERVER_ROOT_PATH)
.queryParam("ids", ids)
.queryParam(PARAM_IDS, ids)
.queryParam("parentDirectoryUuid", parentDirectoryUuid)
.buildAndExpand()
.toUriString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ public class ExploreService {
static final String MODIFICATION = "MODIFICATION";
static final String DIRECTORY = "DIRECTORY";

private DirectoryService directoryService;
private StudyService studyService;
private ContingencyListService contingencyListService;
private NetworkModificationService networkModificationService;
private FilterService filterService;
private CaseService caseService;
private ParametersService parametersService;
private final DirectoryService directoryService;
private final StudyService studyService;
private final ContingencyListService contingencyListService;
private final NetworkModificationService networkModificationService;
private final FilterService filterService;
private final CaseService caseService;
private final ParametersService parametersService;

private static final Logger LOGGER = LoggerFactory.getLogger(ExploreService.class);

Expand Down Expand Up @@ -175,7 +175,7 @@ public void replaceFilterWithScript(UUID id, String userId) {
public void deleteElement(UUID id, String userId) {
// Verify if the user is allowed to delete the element.
// FIXME: to be deleted when it's properly handled by the gateway
canDeleteDirectoryElement(List.of(id), userId);
directoryService.areDirectoryElementsDeletable(List.of(id), userId);
try {
directoryService.deleteElement(id, userId);
directoryService.deleteDirectoryElement(id, userId);
Expand All @@ -190,7 +190,7 @@ public void deleteElementsFromDirectory(List<UUID> uuids, UUID parentDirectoryUu

// Verify if the user is allowed to delete the elements.
// FIXME: to be deleted when it's properly handled by the gateway
canDeleteDirectoryElement(uuids, userId);
directoryService.areDirectoryElementsDeletable(uuids, userId);
try {
uuids.forEach(id -> directoryService.deleteElement(id, userId));
// FIXME dirty fix to ignore errors and still delete the elements in the directory-server. To delete when handled properly.
Expand Down Expand Up @@ -275,11 +275,4 @@ public void duplicateNetworkModifications(UUID sourceId, UUID parentDirectoryUui
// create corresponding directory element
directoryService.duplicateElement(sourceId, newNetworkModification, parentDirectoryUuid, userId);
}

private void canDeleteDirectoryElement(List<UUID> elementUuids, String userId) {
if (!directoryService.canDeleteDirectoryElement(elementUuids, userId).orElse(false)) {
throw new ExploreException(NOT_ALLOWED);
}
}

}
31 changes: 18 additions & 13 deletions src/test/java/org/gridsuite/explore/server/ExploreTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ public class ExploreTest {
private static final UUID PARENT_DIRECTORY_UUID = UUID.randomUUID();
private static final UUID PARENT_DIRECTORY_WITH_ERROR_UUID = UUID.randomUUID();
private static final UUID PRIVATE_STUDY_UUID = UUID.randomUUID();
private static final UUID NOT_ALLOWED_STUDY_UUID = UUID.randomUUID();
private static final UUID FORBIDDEN_STUDY_UUID = UUID.randomUUID();
private static final UUID NOT_FOUND_STUDY_UUID = UUID.randomUUID();
private static final UUID PUBLIC_STUDY_UUID = UUID.randomUUID();
private static final UUID FILTER_UUID = UUID.randomUUID();
private static final UUID FILTER_UUID_2 = UUID.randomUUID();
Expand Down Expand Up @@ -298,12 +299,6 @@ public MockResponse dispatch(RecordedRequest request) {
} else if (path.matches("/v1/studies/metadata[?]ids=" + PRIVATE_STUDY_UUID)) {
return new MockResponse().setBody(listOfPrivateStudyAttributesAsString.replace("elementUuid", "id")).setResponseCode(200)
.addHeader("Content-Type", "application/json; charset=utf-8");
} else if (path.matches("/v1/elements/can-delete\\?ids=" + NOT_ALLOWED_STUDY_UUID)) {
return new MockResponse().setBody("false").setResponseCode(200)
.addHeader("Content-Type", "application/json; charset=utf-8");
} else if (path.matches("/v1/elements/can-delete\\?ids=.*")) {
return new MockResponse().setBody("true").setResponseCode(200)
.addHeader("Content-Type", "application/json; charset=utf-8");
}
} else if ("DELETE".equals(request.getMethod())) {
if (path.matches("/v1/filters/" + FILTER_UUID)) {
Expand Down Expand Up @@ -336,6 +331,14 @@ public MockResponse dispatch(RecordedRequest request) {
return new MockResponse().setResponseCode(200);
}
return new MockResponse().setResponseCode(404);
} else if ("HEAD".equals(request.getMethod())) {
if (path.matches("/v1/elements\\?forDeletion=true&ids=" + FORBIDDEN_STUDY_UUID)) {
return new MockResponse().setResponseCode(403);
} else if (path.matches("/v1/elements\\?forDeletion=true&ids=" + NOT_FOUND_STUDY_UUID)) {
return new MockResponse().setResponseCode(404);
} else if (path.matches("/v1/elements\\?forDeletion=true&ids=.*")) {
return new MockResponse().setResponseCode(200);
}
}
return new MockResponse().setResponseCode(418);
}
Expand Down Expand Up @@ -510,17 +513,17 @@ public void deleteElementInvalidType(UUID elementUUid) throws Exception {
.andExpect(status().is2xxSuccessful());
}

public void deleteElementNotAllowed(UUID elementUUid) throws Exception {
public void deleteElementNotAllowed(UUID elementUUid, int status) throws Exception {
mockMvc.perform(delete("/v1/explore/elements/{elementUuid}",
elementUUid).header("userId", USER1))
.andExpect(status().is(403));
.andExpect(status().is(status));
}

public void deleteElementsNotAllowed(List<UUID> elementUuids, UUID parentUuid) throws Exception {
public void deleteElementsNotAllowed(List<UUID> elementUuids, UUID parentUuid, int status) throws Exception {
var ids = elementUuids.stream().map(UUID::toString).collect(Collectors.joining(","));
mockMvc.perform(delete("/v1/explore/elements/{parentUuid}?ids=" + ids, parentUuid)
.header("userId", USER1))
.andExpect(status().is(403));
.andExpect(status().is(status));
}

@Test
Expand All @@ -534,8 +537,10 @@ public void testDeleteElement() throws Exception {
deleteElement(CASE_UUID);
deleteElement(PARAMETERS_UUID);
deleteElement(MODIFICATION_UUID);
deleteElementNotAllowed(NOT_ALLOWED_STUDY_UUID);
deleteElementsNotAllowed(List.of(NOT_ALLOWED_STUDY_UUID), PARENT_DIRECTORY_UUID);
deleteElementsNotAllowed(List.of(FORBIDDEN_STUDY_UUID), PARENT_DIRECTORY_UUID, 403);
deleteElementsNotAllowed(List.of(NOT_FOUND_STUDY_UUID), PARENT_DIRECTORY_UUID, 404);
deleteElementNotAllowed(FORBIDDEN_STUDY_UUID, 403);
deleteElementNotAllowed(NOT_FOUND_STUDY_UUID, 404);
}

@Test
Expand Down

0 comments on commit 2d0aed8

Please sign in to comment.