Skip to content

Commit

Permalink
Consume REST params and consistently handle error messages (#295)
Browse files Browse the repository at this point in the history
* Always consume the workflow_id param

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Delegate no-content error message to BaseRestHandler

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Don't lose FlowFrameworkException status

Signed-off-by: Daniel Widdis <widdis@gmail.com>

---------

Signed-off-by: Daniel Widdis <widdis@gmail.com>
  • Loading branch information
dbwiddis committed Dec 18, 2023
1 parent 08588c1 commit fae80a5
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public List<Route> routes() {

@Override
protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
String workflowId = request.param(WORKFLOW_ID);
if (!flowFrameworkFeatureEnabledSetting.isFlowFrameworkEnabled()) {
FlowFrameworkException ffe = new FlowFrameworkException(
"This API is disabled. To enable it, set [" + FLOW_FRAMEWORK_ENABLED.getKey() + "] to true.",
Expand All @@ -88,8 +89,6 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
);
}
try {

String workflowId = request.param(WORKFLOW_ID);
Template template = Template.parse(request.content().utf8ToString());
boolean dryRun = request.paramAsBoolean(DRY_RUN, false);
boolean provision = request.paramAsBoolean(PROVISION_WORKFLOW, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ protected BaseRestHandler.RestChannelConsumer prepareRequest(RestRequest request
}
// Validate content
if (request.hasContent()) {
throw new FlowFrameworkException("Invalid request format", RestStatus.BAD_REQUEST);
// BaseRestHandler will give appropriate error message
return channel -> channel.sendResponse(null);
}
// Validate params
if (workflowId == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public String getName() {

@Override
protected BaseRestHandler.RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {

String workflowId = request.param(WORKFLOW_ID);
try {
if (!flowFrameworkFeatureEnabledSetting.isFlowFrameworkEnabled()) {
throw new FlowFrameworkException(
Expand All @@ -67,10 +67,10 @@ protected BaseRestHandler.RestChannelConsumer prepareRequest(RestRequest request
}
// Validate content
if (request.hasContent()) {
throw new FlowFrameworkException("No request body is required", RestStatus.BAD_REQUEST);
// BaseRestHandler will give appropriate error message
return channel -> channel.sendResponse(null);
}
// Validate params
String workflowId = request.param(WORKFLOW_ID);
if (workflowId == null) {
throw new FlowFrameworkException("workflow_id cannot be null", RestStatus.BAD_REQUEST);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,21 +62,20 @@ public List<Route> routes() {

@Override
protected BaseRestHandler.RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {

String workflowId = request.param(WORKFLOW_ID);
try {
if (!flowFrameworkFeatureEnabledSetting.isFlowFrameworkEnabled()) {
throw new FlowFrameworkException(
"This API is disabled. To enable it, update the setting [" + FLOW_FRAMEWORK_ENABLED.getKey() + "] to true.",
RestStatus.FORBIDDEN
);
}

// Validate content
if (request.hasContent()) {
throw new FlowFrameworkException("Invalid request format", RestStatus.BAD_REQUEST);
// BaseRestHandler will give appropriate error message
return channel -> channel.sendResponse(null);
}
// Validate params
String workflowId = request.param(WORKFLOW_ID);
if (workflowId == null) {
throw new FlowFrameworkException("workflow_id cannot be null", RestStatus.BAD_REQUEST);
}
Expand All @@ -87,7 +86,9 @@ protected BaseRestHandler.RestChannelConsumer prepareRequest(RestRequest request
channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder));
}, exception -> {
try {
FlowFrameworkException ex = new FlowFrameworkException(exception.getMessage(), ExceptionsHelper.status(exception));
FlowFrameworkException ex = exception instanceof FlowFrameworkException
? (FlowFrameworkException) exception
: new FlowFrameworkException(exception.getMessage(), ExceptionsHelper.status(exception));
XContentBuilder exceptionBuilder = ex.toXContent(channel.newErrorBuilder(), ToXContent.EMPTY_PARAMS);
channel.sendResponse(new BytesRestResponse(ex.getRestStatus(), exceptionBuilder));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public String getName() {

@Override
protected BaseRestHandler.RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {

String workflowId = request.param(WORKFLOW_ID);
try {
if (!flowFrameworkFeatureEnabledSetting.isFlowFrameworkEnabled()) {
throw new FlowFrameworkException(
Expand All @@ -68,10 +68,10 @@ protected BaseRestHandler.RestChannelConsumer prepareRequest(RestRequest request

// Validate content
if (request.hasContent()) {
throw new FlowFrameworkException("No request body present", RestStatus.BAD_REQUEST);
// BaseRestHandler will give appropriate error message
return channel -> channel.sendResponse(null);
}
// Validate params
String workflowId = request.param(WORKFLOW_ID);
if (workflowId == null) {
throw new FlowFrameworkException("workflow_id cannot be null", RestStatus.BAD_REQUEST);
}
Expand All @@ -83,7 +83,9 @@ protected BaseRestHandler.RestChannelConsumer prepareRequest(RestRequest request
channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder));
}, exception -> {
try {
FlowFrameworkException ex = new FlowFrameworkException(exception.getMessage(), ExceptionsHelper.status(exception));
FlowFrameworkException ex = exception instanceof FlowFrameworkException
? (FlowFrameworkException) exception
: new FlowFrameworkException(exception.getMessage(), ExceptionsHelper.status(exception));
XContentBuilder exceptionBuilder = ex.toXContent(channel.newErrorBuilder(), ToXContent.EMPTY_PARAMS);
channel.sendResponse(new BytesRestResponse(ex.getRestStatus(), exceptionBuilder));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
}
// Validate content
if (request.hasContent()) {
throw new FlowFrameworkException("Invalid request format", RestStatus.BAD_REQUEST);
// BaseRestHandler will give appropriate error message
return channel -> channel.sendResponse(null);
}
// Validate params
if (workflowId == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void testRestGetWorkflowStateActionRoutes() {
public void testNullWorkflowId() throws Exception {

// Request with no params
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.POST)
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.GET)
.withPath(this.getPath)
.build();

Expand All @@ -76,7 +76,7 @@ public void testNullWorkflowId() throws Exception {
}

public void testInvalidRequestWithContent() {
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.POST)
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.GET)
.withPath(this.getPath)
.withContent(new BytesArray("request body"), MediaTypeRegistry.JSON)
.build();
Expand All @@ -86,14 +86,14 @@ public void testInvalidRequestWithContent() {
restGetWorkflowStateAction.handleRequest(request, channel, nodeClient);
});
assertEquals(
"request [POST /_plugins/_flow_framework/workflow/{workflow_id}/_status] does not support having a body",
"request [GET /_plugins/_flow_framework/workflow/{workflow_id}/_status] does not support having a body",
ex.getMessage()
);
}

public void testFeatureFlagNotEnabled() throws Exception {
when(flowFrameworkFeatureEnabledSetting.isFlowFrameworkEnabled()).thenReturn(false);
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.POST)
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.GET)
.withPath(this.getPath)
.build();
FakeRestChannel channel = new FakeRestChannel(request, false, 1);
Expand Down

0 comments on commit fae80a5

Please sign in to comment.