Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JBPM-9611] Command resource returns HTTP OK response status code even for failure scenarios #2396

Merged
merged 1 commit into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
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"
}
]
}
}
}
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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"
}
]
}
}
}