aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordanielhanrahan <daniel.hanrahan@est.tech>2022-02-14 11:47:09 +0000
committerDaniel Hanrahan <daniel.hanrahan@est.tech>2022-02-14 12:31:29 +0000
commit05947953238a436f32e1b18d1201cf53f8817a1e (patch)
treed5ffcec5df5c9a6ce11bcb6ebf756bad925de8fc
parentaec9b7e131be827e770e0124dff6ebc765dc1f7f (diff)
Fix race condition in apex-editor model upload
Fixed a race condition in /Model/Upload endpoint caused by userId query param overwriting a global parameter. New behavior is for userId to override global param for that request only. Updated JavaDocs to reflect behavior. Added JUnit test to verify userId overriding. Fixed issue where upload success message was not included in response. Issue-ID: POLICY-3929 Signed-off-by: danielhanrahan <daniel.hanrahan@est.tech> Change-Id: Ic1d78083778eef2df3675b3b4fbc1e41420da46f
-rw-r--r--gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/ApexEditorRestResource.java14
-rw-r--r--gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/ModelHandler.java12
-rw-r--r--gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/RestSession.java8
-rw-r--r--gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/plugin/upload/PolicyUploadHandler.java28
-rw-r--r--gui-editors/gui-editor-apex/src/test/java/org/onap/policy/gui/editors/apex/rest/handling/RestSessionTest.java4
-rw-r--r--gui-editors/gui-editor-apex/src/test/java/org/onap/policy/gui/editors/apex/rest/handling/plugin/upload/PolicyUploadHandlerTest.java51
6 files changed, 78 insertions, 39 deletions
diff --git a/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/ApexEditorRestResource.java b/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/ApexEditorRestResource.java
index 0b777b6..b86df28 100644
--- a/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/ApexEditorRestResource.java
+++ b/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/ApexEditorRestResource.java
@@ -1,7 +1,7 @@
/*-
* ============LICENSE_START=======================================================
* Copyright (C) 2016-2018 Ericsson. All rights reserved.
- * Modifications Copyright (C) 2020 Nordix Foundation.
+ * Modifications Copyright (C) 2020-2022 Nordix Foundation.
* Modifications Copyright (C) 2021 AT&T Intellectual Property. All rights reserved.
* Modifications Copyright (C) 2021 Bell Canada. All rights reserved.
* ================================================================================
@@ -34,11 +34,9 @@ import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.core.MediaType;
-import org.apache.commons.lang3.StringUtils;
import org.onap.policy.apex.model.modelapi.ApexApiResult;
import org.onap.policy.apex.model.modelapi.ApexApiResult.Result;
import org.onap.policy.common.utils.resources.TextFileUtils;
-import org.onap.policy.gui.editors.apex.rest.ApexEditorMain;
import org.slf4j.ext.XLogger;
import org.slf4j.ext.XLoggerFactory;
@@ -233,16 +231,14 @@ public class ApexEditorRestResource implements RestCommandHandler {
/**
* Uploads a TOSCA Policy Model to a configured endpoint.
*
- * @param userid the userid to use for upload
+ * @param userId the userId to use for upload. If blank, the commandline
+ * parameter "upload-userid" is used.
* @return an ApexAPIResult that contains the operation status and success/error messages
*/
@GET
@Path("Model/Upload")
- public ApexApiResult uploadModel(@QueryParam("userId") final String userid) {
- if (!StringUtils.isBlank(userid)) {
- ApexEditorMain.getParameters().setUploadUserid(userid);
- }
- return processRestCommand(RestCommandType.MODEL, RestCommand.UPLOAD);
+ public ApexApiResult uploadModel(@QueryParam("userId") final String userId) {
+ return processRestCommand(RestCommandType.MODEL, RestCommand.UPLOAD, userId);
}
/**
diff --git a/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/ModelHandler.java b/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/ModelHandler.java
index 38c7fec..18dc227 100644
--- a/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/ModelHandler.java
+++ b/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/ModelHandler.java
@@ -1,7 +1,7 @@
/*-
* ============LICENSE_START=======================================================
* Copyright (C) 2018 Ericsson. All rights reserved.
- * Modifications Copyright (C) 2019-2020 Nordix Foundation.
+ * Modifications Copyright (C) 2019-2022 Nordix Foundation.
* Modifications Copyright (C) 2021 AT&T Intellectual Property. All rights reserved.
* Modifications Copyright (C) 2021 Bell Canada. All rights reserved.
* ================================================================================
@@ -76,8 +76,6 @@ public class ModelHandler implements RestCommandHandler {
return listModel(session);
case DOWNLOAD:
return downloadModel(session);
- case UPLOAD:
- return uploadModel(session);
case DELETE:
return deleteModel(session);
default:
@@ -102,6 +100,8 @@ public class ModelHandler implements RestCommandHandler {
return createModel(session, jsonString);
case UPDATE:
return updateModel(session, jsonString);
+ case UPLOAD:
+ return uploadModel(session, jsonString);
default:
return getUnsupportedCommandResultMessage(session, commandType, command);
}
@@ -276,12 +276,14 @@ public class ModelHandler implements RestCommandHandler {
* Upload the model for this session to the configured URL.
*
* @param session the Apex model editing session
+ * @param userId the userId to use for upload. If blank, the commandline
+ * parameter "upload-userid" is used.
* @return a result indicating if the upload was successful or not
*/
- private ApexApiResult uploadModel(final RestSession session) {
+ private ApexApiResult uploadModel(final RestSession session, String userId) {
LOGGER.entry();
- ApexApiResult result = session.uploadModel();
+ ApexApiResult result = session.uploadModel(userId);
LOGGER.exit("Model/Download" + (result != null && result.isOk() ? OK : NOT_OK));
return result;
diff --git a/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/RestSession.java b/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/RestSession.java
index 662c634..c41513f 100644
--- a/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/RestSession.java
+++ b/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/RestSession.java
@@ -1,7 +1,7 @@
/*-
* ============LICENSE_START=======================================================
* Copyright (C) 2018 Ericsson. All rights reserved.
- * Modifications Copyright (C) 2020 Nordix Foundation.
+ * Modifications Copyright (C) 2020-2022 Nordix Foundation.
* Modifications Copyright (C) 2021 AT&T Intellectual Property. All rights reserved.
* Modifications Copyright (C) 2021 Bell Canada. All rights reserved.
* ================================================================================
@@ -200,9 +200,11 @@ public class RestSession {
/**
* Upload the apex model as a TOSCA service template YAML string to the configured URL.
*
+ * @param userId the userId to use for upload. If blank, the commandline
+ * parameter "upload-userid" is used.
* @return a result indicating if the upload was successful or not
*/
- public ApexApiResult uploadModel() {
+ public ApexApiResult uploadModel(final String userId) {
// Get the model in TOSCA format
ApexApiResult result = downloadModel();
if (result.isNok()) {
@@ -215,7 +217,7 @@ public class RestSession {
var policyModelUUid = apexModelBeingUploaded.getPolicyModel().getKeyInformation().get(policyModelKey)
.getUuid().toString();
- return new PolicyUploadHandler().doUpload(result.getMessage(), policyModelKey, policyModelUUid);
+ return new PolicyUploadHandler().doUpload(result.getMessage(), policyModelKey, policyModelUUid, userId);
}
/**
diff --git a/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/plugin/upload/PolicyUploadHandler.java b/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/plugin/upload/PolicyUploadHandler.java
index ebbe3db..1766831 100644
--- a/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/plugin/upload/PolicyUploadHandler.java
+++ b/gui-editors/gui-editor-apex/src/main/java/org/onap/policy/gui/editors/apex/rest/handling/plugin/upload/PolicyUploadHandler.java
@@ -1,6 +1,6 @@
/*
* ============LICENSE_START=======================================================
- * Copyright (C) 2020 Nordix Foundation
+ * Copyright (C) 2020-2022 Nordix Foundation
* Modifications Copyright (C) 2021 AT&T Intellectual Property. All rights reserved.
* Modifications Copyright (C) 2021 Bell Canada. All rights reserved.
* ================================================================================
@@ -49,45 +49,50 @@ public class PolicyUploadHandler {
* @param toscaServiceTemplate the TOSCA service template
* @param policyModelKey the key of the policy model
* @param policyModelUuid the UUID of the policy model
+ * @param uploadUserId the userId to use for upload. If blank, the commandline
+ * parameter "upload-userid" is used.
* @return the result of the upload process
*/
public ApexApiResult doUpload(final String toscaServiceTemplate, final AxArtifactKey policyModelKey,
- final String policyModelUuid) {
+ final String policyModelUuid, String uploadUserId) {
LOGGER.entry();
- if (StringUtils.isBlank(ApexEditorMain.getParameters().getUploadUrl())) {
+ final String uploadUrl = ApexEditorMain.getParameters().getUploadUrl();
+ if (StringUtils.isBlank(uploadUrl)) {
final var apexApiResult = new ApexApiResult(Result.FAILED);
apexApiResult.addMessage("Model upload is disabled, parameter upload-url is not set on server");
LOGGER.exit(MODEL_UPLOAD_NOT_OK);
return apexApiResult;
+ }
+ if (StringUtils.isBlank(uploadUserId)) {
+ uploadUserId = ApexEditorMain.getParameters().getUploadUserid();
}
final var uploadPolicyRequestDto = new UploadPolicyRequestDto();
- uploadPolicyRequestDto.setUserId(ApexEditorMain.getParameters().getUploadUserid());
+ uploadPolicyRequestDto.setUserId(uploadUserId);
uploadPolicyRequestDto
.setFileData(Base64.getEncoder().encodeToString(toscaServiceTemplate.getBytes(StandardCharsets.UTF_8)));
uploadPolicyRequestDto.setFilename(
String.format("%s.%s.%s", policyModelUuid, policyModelKey.getName(), policyModelKey.getVersion()));
try {
- final var response = ClientBuilder.newClient().target(ApexEditorMain.getParameters().getUploadUrl())
+ final var response = ClientBuilder.newClient().target(uploadUrl)
.request(MediaType.APPLICATION_JSON)
.post(Entity.entity(uploadPolicyRequestDto, MediaType.APPLICATION_JSON));
if (response.getStatus() == 201) {
final var apexApiResult = new ApexApiResult(Result.SUCCESS);
- String.format("uploading Policy '%s' to URL '%s' with userId '%s' was successful",
- policyModelKey.getId(), ApexEditorMain.getParameters().getUploadUrl(),
- ApexEditorMain.getParameters().getUploadUserid());
+ apexApiResult.addMessage(
+ String.format("uploading Policy '%s' to URL '%s' with userId '%s' was successful",
+ policyModelKey.getId(), uploadUrl, uploadUserId));
LOGGER.exit("Model/Upload: OK");
return apexApiResult;
} else {
final var apexApiResult = new ApexApiResult(Result.FAILED);
apexApiResult.addMessage(
String.format("uploading Policy '%s' to URL '%s' with userId '%s' failed with status %s",
- policyModelKey.getId(), ApexEditorMain.getParameters().getUploadUrl(),
- ApexEditorMain.getParameters().getUploadUserid(), response.getStatus()));
+ policyModelKey.getId(), uploadUrl, uploadUserId, response.getStatus()));
LOGGER.exit(MODEL_UPLOAD_NOT_OK);
return apexApiResult;
}
@@ -95,8 +100,7 @@ public class PolicyUploadHandler {
final var apexApiResult = new ApexApiResult(Result.FAILED);
apexApiResult
.addMessage(String.format("uploading Policy '%s' to URL '%s' with userId '%s' failed with error %s",
- policyModelKey.getId(), ApexEditorMain.getParameters().getUploadUrl(),
- ApexEditorMain.getParameters().getUploadUserid(), e.getMessage()));
+ policyModelKey.getId(), uploadUrl, uploadUserId, e.getMessage()));
LOGGER.exit(MODEL_UPLOAD_NOT_OK);
return apexApiResult;
}
diff --git a/gui-editors/gui-editor-apex/src/test/java/org/onap/policy/gui/editors/apex/rest/handling/RestSessionTest.java b/gui-editors/gui-editor-apex/src/test/java/org/onap/policy/gui/editors/apex/rest/handling/RestSessionTest.java
index b2158ce..121faa0 100644
--- a/gui-editors/gui-editor-apex/src/test/java/org/onap/policy/gui/editors/apex/rest/handling/RestSessionTest.java
+++ b/gui-editors/gui-editor-apex/src/test/java/org/onap/policy/gui/editors/apex/rest/handling/RestSessionTest.java
@@ -1,6 +1,6 @@
/*-
* ============LICENSE_START=======================================================
- * Copyright (C) 2021 Nordix Foundation.
+ * Copyright (C) 2021-2022 Nordix Foundation.
* Modifications Copyright (C) 2021 AT&T Intellectual Property. All rights reserved.
* ================================================================================
* Licensed under the Apache License, Version 2.0 (the "License");
@@ -139,7 +139,7 @@ public class RestSessionTest {
final var toscaPath = Path.of("src/test/resources/models/PolicyModel.yaml");
final var toscaString = Files.readString(toscaPath);
restSession.loadFromString(toscaString);
- final var apexApiResult = restSession.uploadModel();
+ final var apexApiResult = restSession.uploadModel("");
assertThat(apexApiResult.isNok()).isTrue();
assertThat(apexApiResult.getMessage()).contains("Model upload is disabled");
}
diff --git a/gui-editors/gui-editor-apex/src/test/java/org/onap/policy/gui/editors/apex/rest/handling/plugin/upload/PolicyUploadHandlerTest.java b/gui-editors/gui-editor-apex/src/test/java/org/onap/policy/gui/editors/apex/rest/handling/plugin/upload/PolicyUploadHandlerTest.java
index 7c97734..ff4cfa2 100644
--- a/gui-editors/gui-editor-apex/src/test/java/org/onap/policy/gui/editors/apex/rest/handling/plugin/upload/PolicyUploadHandlerTest.java
+++ b/gui-editors/gui-editor-apex/src/test/java/org/onap/policy/gui/editors/apex/rest/handling/plugin/upload/PolicyUploadHandlerTest.java
@@ -1,6 +1,6 @@
/*-
* ============LICENSE_START=======================================================
- * Copyright (C) 2021 Nordix Foundation.
+ * Copyright (C) 2021-2022 Nordix Foundation.
* ================================================================================
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@ import java.nio.file.Files;
import java.nio.file.Path;
import javax.ws.rs.client.Client;
import javax.ws.rs.client.ClientBuilder;
+import javax.ws.rs.client.Entity;
import javax.ws.rs.client.Invocation;
import javax.ws.rs.client.ResponseProcessingException;
import javax.ws.rs.client.WebTarget;
@@ -38,6 +39,7 @@ import org.apache.commons.lang3.RandomStringUtils;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
+import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
@@ -46,10 +48,12 @@ import org.onap.policy.gui.editors.apex.rest.ApexEditorMain;
public class PolicyUploadHandlerTest {
+ private static final String CMDLINE_UPLOAD_USERID = "MyUser";
private PolicyUploadHandler uploadHandler;
private AxArtifactKey axArtifactKey;
private String toscaServiceTemplate;
private MockedStatic<ClientBuilder> clientBuilderMockedStatic;
+ private ArgumentCaptor<Entity<UploadPolicyRequestDto>> dtoEntityCaptor;
/**
* Prepares test environment.
@@ -78,12 +82,12 @@ public class PolicyUploadHandlerTest {
@Test
public void testDoUploadNoUrl() {
- final String[] args = {"--upload-userid", "MyUser"};
+ final String[] args = {"--upload-userid", CMDLINE_UPLOAD_USERID};
final var outBaStream = new ByteArrayOutputStream();
final var outStream = new PrintStream(outBaStream);
new ApexEditorMain(args, outStream);
- final var result = uploadHandler.doUpload(toscaServiceTemplate, axArtifactKey, "");
+ final var result = uploadHandler.doUpload(toscaServiceTemplate, axArtifactKey, "", "");
assertThat(result.isNok()).isTrue();
assertThat(result.getMessage()).contains("Model upload is disable");
}
@@ -96,7 +100,7 @@ public class PolicyUploadHandlerTest {
prepareApexEditorMain();
- final var result = uploadHandler.doUpload(toscaServiceTemplate, axArtifactKey, "");
+ final var result = uploadHandler.doUpload(toscaServiceTemplate, axArtifactKey, "", "");
assertThat(result.isNok()).isTrue();
assertThat(result.getMessage()).contains("failed with error");
@@ -111,7 +115,7 @@ public class PolicyUploadHandlerTest {
prepareApexEditorMain();
- final var result = uploadHandler.doUpload(toscaServiceTemplate, axArtifactKey, "");
+ final var result = uploadHandler.doUpload(toscaServiceTemplate, axArtifactKey, "", "");
assertThat(result.isOk()).isTrue();
}
@@ -125,12 +129,41 @@ public class PolicyUploadHandlerTest {
prepareApexEditorMain();
- final var result = uploadHandler.doUpload(toscaServiceTemplate, axArtifactKey, "");
+ final var result = uploadHandler.doUpload(toscaServiceTemplate, axArtifactKey, "", "");
assertThat(result.isNok()).isTrue();
assertThat(result.getMessage()).contains("failed with status 500");
}
+ @Test
+ public void testDoUploadUserId() {
+ final var response = Mockito.mock(Response.class);
+ mockRsHttpClient(response);
+
+ Mockito.doReturn(201).when(response).getStatus();
+
+ prepareApexEditorMain();
+
+ // If uploadUserId is specified, that value should be in DTO.
+ var result = uploadHandler.doUpload(toscaServiceTemplate, axArtifactKey, "",
+ "OverrideUser");
+ assertThat(result.isOk()).isTrue();
+ var dto = dtoEntityCaptor.getValue().getEntity();
+ assertThat(dto.getUserId()).isEqualTo("OverrideUser");
+
+ // If uploadUserId is blank, the value from command line parameter 'upload-userid' is used.
+ result = uploadHandler.doUpload(toscaServiceTemplate, axArtifactKey, "", "");
+ assertThat(result.isOk()).isTrue();
+ dto = dtoEntityCaptor.getValue().getEntity();
+ assertThat(dto.getUserId()).isEqualTo(CMDLINE_UPLOAD_USERID);
+
+ // If uploadUserId is null, the value from command line parameter 'upload-userid' is used.
+ result = uploadHandler.doUpload(toscaServiceTemplate, axArtifactKey, "", null);
+ assertThat(result.isOk()).isTrue();
+ dto = dtoEntityCaptor.getValue().getEntity();
+ assertThat(dto.getUserId()).isEqualTo(CMDLINE_UPLOAD_USERID);
+ }
+
private void mockRsHttpClient(Response response) {
final var webTarget = Mockito.mock(WebTarget.class);
final var client = Mockito.mock(Client.class);
@@ -139,15 +172,17 @@ public class PolicyUploadHandlerTest {
clientBuilderMockedStatic = Mockito.mockStatic(ClientBuilder.class);
+ dtoEntityCaptor = ArgumentCaptor.forClass(Entity.class);
+
Mockito.when(ClientBuilder.newClient()).thenReturn(client);
Mockito.when(client.target(ArgumentMatchers.anyString())).thenReturn(webTarget);
Mockito.when(webTarget.request(MediaType.APPLICATION_JSON)).thenReturn(invocationBuilder);
Mockito.when(webTarget.request(MediaType.APPLICATION_JSON)).thenReturn(invocationBuilder);
- Mockito.when(invocationBuilder.post(ArgumentMatchers.any())).thenReturn(response);
+ Mockito.when(invocationBuilder.post(dtoEntityCaptor.capture())).thenReturn(response);
}
private void prepareApexEditorMain() {
- final String[] args = {"--upload-userid", "MyUser", "--upload-url", "http://127.0.0.1"};
+ final String[] args = {"--upload-userid", CMDLINE_UPLOAD_USERID, "--upload-url", "http://127.0.0.1"};
final var outBaStream = new ByteArrayOutputStream();
final var outStream = new PrintStream(outBaStream);
new ApexEditorMain(args, outStream);