Skip to content

Commit

Permalink
[JBPM-9611] Command resource returns HTTP OK response even for failure
Browse files Browse the repository at this point in the history
scenarios
  • Loading branch information
fjtirado committed Feb 23, 2021
1 parent bdfa3b8 commit 79d4dfc
Show file tree
Hide file tree
Showing 12 changed files with 181 additions and 56 deletions.
Expand Up @@ -41,7 +41,17 @@
"ignores": {
"revapi": {
"_comment": "Changes between 7.48.0.Final and the current branch. These changes are desired and thus ignored.",
"ignore": []
"ignore": [
{
"code": "java.method.addedToInterface",
"new": "method org.kie.server.api.model.ServiceResponse<org.kie.api.runtime.ExecutionResults> org.kie.server.client.RuleServicesClient::executeCommandsWithResults(java.lang.String, org.kie.api.command.Command<?>, javax.ws.rs.core.Response.Status)",
"package": "org.kie.server.client",
"classSimpleName": "RuleServicesClient",
"methodName": "executeCommandsWithResults",
"elementKind": "method",
"justification": "https://issues.redhat.com/browse/JBPM-9611"
}
]
}
}
}
Expand Up @@ -15,6 +15,8 @@

package org.kie.server.client;

import javax.ws.rs.core.Response.Status;

import org.kie.api.command.Command;
import org.kie.server.api.commands.CommandScript;
import org.kie.server.api.model.KieContainerResource;
Expand Down Expand Up @@ -80,7 +82,19 @@ public interface KieServicesClient {
* @deprecated
*/
@Deprecated
ServiceResponse<String> executeCommands(String id, Command<?> cmd);
default ServiceResponse<String> executeCommands(String id, Command<?> cmd) {
return executeCommands(id, cmd, Status.OK);
}

/**
* This method is deprecated on KieServicesClient as it was moved to RuleServicesClient
* @see RuleServicesClient#executeCommands(String, Command)
* @deprecated
*/
@Deprecated
default ServiceResponse<String> executeCommands(String id, Command<?> cmd, Status response) {
throw new UnsupportedOperationException();
}

/**
* Sets the classloader for user class unmarshalling
Expand Down
Expand Up @@ -15,6 +15,8 @@

package org.kie.server.client;

import javax.ws.rs.core.Response.Status;

import org.kie.api.command.Command;
import org.kie.api.runtime.ExecutionResults;
import org.kie.server.api.model.ServiceResponse;
Expand All @@ -36,7 +38,11 @@ public interface RuleServicesClient {

ServiceResponse<ExecutionResults> executeCommandsWithResults(String id, String payload);

ServiceResponse<ExecutionResults> executeCommandsWithResults(String id, Command<?> cmd);
default ServiceResponse<ExecutionResults> executeCommandsWithResults(String id, Command<?> cmd) {
return executeCommandsWithResults(id, cmd, Status.OK);
}

ServiceResponse<ExecutionResults> executeCommandsWithResults(String id, Command<?> cmd, Status status);

void setResponseHandler(ResponseHandler responseHandler);
}
Expand Down
Expand Up @@ -36,6 +36,7 @@
import javax.jms.TextMessage;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;

import org.kie.server.api.KieServerConstants;
import org.kie.server.api.commands.CommandScript;
Expand Down Expand Up @@ -285,12 +286,28 @@ protected <T> ServiceResponse<T> makeHttpPostRequestAndCreateServiceResponse(
return makeHttpPostRequestAndCreateServiceResponse( uri, serialize( bodyObject ), resultType, headers );
}

protected <T> ServiceResponse<T> makeHttpPostRequestAndCreateServiceResponse(String uri,
Object bodyObject,
Class<T> resultType,
Map<String, String> headers,
Status status) {
return makeHttpPostRequestAndCreateServiceResponse(uri, serialize(bodyObject), resultType, headers, status);
}

protected <T> ServiceResponse<T> makeHttpPostRequestAndCreateServiceResponse(String uri, String body, Class<T> resultType) {
return makeHttpPostRequestAndCreateServiceResponse( uri, body, resultType, new HashMap<String, String>() );
}

@SuppressWarnings("unchecked")
protected <T> ServiceResponse<T> makeHttpPostRequestAndCreateServiceResponse(String uri, String body, Class<T> resultType, Map<String, String> headers) {
return makeHttpPostRequestAndCreateServiceResponse(uri, body, resultType, headers, Status.OK);
}

protected <T> ServiceResponse<T> makeHttpPostRequestAndCreateServiceResponse(String uri,
String body,
Class<T> resultType,
Map<String, String> headers,
Status status) {
logger.debug("About to send POST request to '{}' with payload '{}'", uri, body);
KieServerHttpRequest request = invoke(uri, new RemoteHttpOperation(){
@Override
Expand All @@ -303,13 +320,14 @@ public KieServerHttpRequest doOperation(String url) {

owner.setConversationId(response.header(KieServerConstants.KIE_CONVERSATION_ID_TYPE_HEADER));

if ( response.code() == Response.Status.OK.getStatusCode() ) {
if (response.code() == status.getStatusCode()) {
ServiceResponse serviceResponse = deserialize( response.body(), ServiceResponse.class );
checkResultType( serviceResponse, resultType );
return serviceResponse;
} else {
throw createExceptionForUnexpectedResponseCode( request, response );
}

}


Expand Down Expand Up @@ -785,13 +803,21 @@ protected List<?> safeList(List<?> list) {
* override of the regular method to allow backward compatibility for string based result of ServiceResponse
*/
protected <T> ServiceResponse<T> makeBackwardCompatibleHttpPostRequestAndCreateServiceResponse(String uri, Object body, Class<T> resultType, Map<String, String> headers) {
return makeBackwardCompatibleHttpPostRequestAndCreateServiceResponse(uri, body, resultType, headers, Status.OK);
}

protected <T> ServiceResponse<T> makeBackwardCompatibleHttpPostRequestAndCreateServiceResponse(String uri,
Object body,
Class<T> resultType,
Map<String, String> headers,
Status expectedStatus) {
logger.debug("About to send POST request to '{}' with payload '{}'", uri, body);
KieServerHttpRequest request = newRequest( uri ).headers(headers).body(serialize( body )).post();
KieServerHttpResponse response = request.response();

owner.setConversationId(response.header(KieServerConstants.KIE_CONVERSATION_ID_TYPE_HEADER));

if ( response.code() == Response.Status.OK.getStatusCode() ) {
if (response.code() == expectedStatus.getStatusCode()) {
ServiceResponse serviceResponse = deserialize( response.body(), ServiceResponse.class );
// serialize it back to string to make it backward compatible
serviceResponse.setResult(serialize(serviceResponse.getResult()));
Expand All @@ -800,8 +826,10 @@ protected <T> ServiceResponse<T> makeBackwardCompatibleHttpPostRequestAndCreateS
} else {
throw createExceptionForUnexpectedResponseCode( request, response );
}

}


protected <T> ServiceResponse<T> makeBackwardCompatibleHttpPostRequestAndCreateServiceResponse(String uri, String body, Class<T> resultType) {
logger.debug("About to send POST request to '{}' with payload '{}'", uri, body);
KieServerHttpRequest request = newRequest( uri ).body( body ).post();
Expand Down
Expand Up @@ -15,24 +15,34 @@

package org.kie.server.client.impl;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.ServiceLoader;

import javax.ws.rs.core.Response.Status;

import org.kie.api.command.Command;
import org.kie.server.api.KieServerConstants;
import org.kie.server.api.commands.GetReleaseIdCommand;
import org.kie.server.api.model.KieContainerResourceFilter;
import org.kie.server.api.commands.ActivateContainerCommand;
import org.kie.server.api.commands.CallContainerCommand;
import org.kie.server.api.commands.CommandScript;
import org.kie.server.api.commands.CreateContainerCommand;
import org.kie.server.api.commands.DeactivateContainerCommand;
import org.kie.server.api.commands.DisposeContainerCommand;
import org.kie.server.api.commands.GetContainerInfoCommand;
import org.kie.server.api.commands.GetReleaseIdCommand;
import org.kie.server.api.commands.GetScannerInfoCommand;
import org.kie.server.api.commands.GetServerInfoCommand;
import org.kie.server.api.commands.GetServerStateCommand;
import org.kie.server.api.commands.ListContainersCommand;
import org.kie.server.api.commands.UpdateReleaseIdCommand;
import org.kie.server.api.commands.UpdateScannerCommand;
import org.kie.server.api.exception.KieServicesException;
import org.kie.server.api.model.KieContainerResource;
import org.kie.server.api.model.KieContainerResourceFilter;
import org.kie.server.api.model.KieContainerResourceList;
import org.kie.server.api.model.KieScannerResource;
import org.kie.server.api.model.KieServerCommand;
Expand All @@ -43,21 +53,13 @@
import org.kie.server.api.model.ServiceResponsesList;
import org.kie.server.client.KieServicesClient;
import org.kie.server.client.KieServicesConfiguration;
import org.kie.server.api.exception.KieServicesException;
import org.kie.server.client.RuleServicesClient;
import org.kie.server.client.helper.KieServicesClientBuilder;
import org.kie.server.client.jms.RequestReplyResponseHandler;
import org.kie.server.client.jms.ResponseHandler;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.ServiceLoader;

public class KieServicesClientImpl extends AbstractKieServicesClientImpl implements KieServicesClient {

private static Logger logger = LoggerFactory.getLogger(KieServicesClientImpl.class);
Expand Down Expand Up @@ -356,9 +358,11 @@ public ServiceResponse<String> executeCommands(String id, String payload) {
*/
@Deprecated
@Override
public ServiceResponse<String> executeCommands(String id, Command<?> cmd) {
public ServiceResponse<String> executeCommands(String id, Command<?> cmd, Status status) {
if( config.isRest() ) {
return makeBackwardCompatibleHttpPostRequestAndCreateServiceResponse(loadBalancer.getUrl() + "/containers/instances/" + id, cmd, String.class, getHeaders(cmd) );
return makeBackwardCompatibleHttpPostRequestAndCreateServiceResponse(loadBalancer.getUrl() +
"/containers/instances/" + id, cmd,
String.class, getHeaders(cmd), status);
} else {
CommandScript script = new CommandScript(Collections.singletonList((KieServerCommand) new CallContainerCommand(id, serialize(cmd))));
ServiceResponse<String> response = (ServiceResponse<String>) executeJmsCommand(script, cmd.getClass().getName(), null, id).getResponses().get(0);
Expand Down
Expand Up @@ -17,6 +17,8 @@

import java.util.Collections;

import javax.ws.rs.core.Response.Status;

import org.drools.core.runtime.impl.ExecutionResultImpl;
import org.kie.api.command.Command;
import org.kie.api.runtime.ExecutionResults;
Expand Down Expand Up @@ -61,9 +63,10 @@ public ServiceResponse<ExecutionResults> executeCommandsWithResults(String id, S


@Override
public ServiceResponse<ExecutionResults> executeCommandsWithResults(String id, Command<?> cmd) {
public ServiceResponse<ExecutionResults> executeCommandsWithResults(String id, Command<?> cmd, Status status) {
if( config.isRest() ) {
return makeHttpPostRequestAndCreateServiceResponse( loadBalancer.getUrl() + "/containers/instances/" + id, cmd, (Class)ExecutionResultImpl.class, getHeaders(cmd) );
return makeHttpPostRequestAndCreateServiceResponse(loadBalancer.getUrl() + "/containers/instances/" + id,
cmd, (Class) ExecutionResultImpl.class, getHeaders(cmd), status);
} else {
CommandScript script = new CommandScript( Collections.singletonList( (KieServerCommand) new CallContainerCommand( id, serialize(cmd) ) ) );
ServiceResponse response = executeJmsCommand( script, cmd.getClass().getName(), null, id ).getResponses().get( 0 );
Expand Down
Expand Up @@ -26,7 +26,51 @@
"ignores": {
"revapi": {
"_comment": "Changes between 7.48.0.Final and the current branch. These changes are desired and thus ignored.",
"ignore": []
}
"ignore": [
{
"code": "java.annotation.attributeRemoved",
"old": "method javax.ws.rs.core.Response org.kie.server.remote.rest.drools.CommandResource::manageContainer(javax.ws.rs.core.HttpHeaders, java.lang.String, java.lang.String)",
"new": "method javax.ws.rs.core.Response org.kie.server.remote.rest.drools.CommandResource::manageContainer(javax.ws.rs.core.HttpHeaders, java.lang.String, java.lang.String)",
"annotationType": "io.swagger.annotations.ApiOperation",
"annotation": "@io.swagger.annotations.ApiOperation(\"Executes one or more runtime commands\")",
"attribute": "response",
"value": "org.kie.server.api.model.ServiceResponse.class",
"package": "org.kie.server.remote.rest.drools",
"classSimpleName": "CommandResource",
"methodName": "manageContainer",
"elementKind": "method",
"justification": "https://issues.redhat.com/browse/RHPAM-3463"
},
{
"code": "java.annotation.attributeRemoved",
"old": "method javax.ws.rs.core.Response org.kie.server.remote.rest.drools.CommandResource::manageContainer(javax.ws.rs.core.HttpHeaders, java.lang.String, java.lang.String)",
"new": "method javax.ws.rs.core.Response org.kie.server.remote.rest.drools.CommandResource::manageContainer(javax.ws.rs.core.HttpHeaders, java.lang.String, java.lang.String)",
"annotationType": "io.swagger.annotations.ApiOperation",
"annotation": "@io.swagger.annotations.ApiOperation(\"Executes one or more runtime commands\")",
"attribute": "code",
"value": "200",
"package": "org.kie.server.remote.rest.drools",
"classSimpleName": "CommandResource",
"methodName": "manageContainer",
"elementKind": "method",
"justification": "https://issues.redhat.com/browse/RHPAM-3463"
},
{
"code": "java.annotation.attributeValueChanged",
"old": "method javax.ws.rs.core.Response org.kie.server.remote.rest.drools.CommandResource::manageContainer(javax.ws.rs.core.HttpHeaders, java.lang.String, java.lang.String)",
"new": "method javax.ws.rs.core.Response org.kie.server.remote.rest.drools.CommandResource::manageContainer(javax.ws.rs.core.HttpHeaders, java.lang.String, java.lang.String)",
"annotationType": "io.swagger.annotations.ApiResponses",
"annotation": "@io.swagger.annotations.ApiResponses({@io.swagger.annotations.ApiResponse(code = 200, message = \"Successful execution\", response = org.kie.server.api.model.ServiceResponse.class), @io.swagger.annotations.ApiResponse(code = 500, message = \"Unexpected error\", response = org.kie.server.api.model.ServiceResponse.class), @io.swagger.annotations.ApiResponse(code = 204, message = \"Command execute successfully, but without response\", response = org.kie.server.api.model.ServiceResponse.class)})",
"attribute": "value",
"oldValue": "{@io.swagger.annotations.ApiResponse(code = 500, message = \"Unexpected error\")}",
"newValue": "{@io.swagger.annotations.ApiResponse(code = 200, message = \"Successful execution\", response = org.kie.server.api.model.ServiceResponse.class), @io.swagger.annotations.ApiResponse(code = 500, message = \"Unexpected error\", response = org.kie.server.api.model.ServiceResponse.class), @io.swagger.annotations.ApiResponse(code = 204, message = \"Command execute successfully, but without response\", response = org.kie.server.api.model.ServiceResponse.class)}",
"package": "org.kie.server.remote.rest.drools",
"classSimpleName": "CommandResource",
"methodName": "manageContainer",
"elementKind": "method",
"justification": "https://issues.redhat.com/browse/RHPAM-3463"
}
]
}
}
}

0 comments on commit 79d4dfc

Please sign in to comment.