-
Notifications
You must be signed in to change notification settings - Fork 249
Issue-3938 Upgrade the Knative custom functions to have visibility over the returned status code and headers #3939
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
base: main
Are you sure you want to change the base?
Changes from 44 commits
02fadd7
515c491
153b334
f05f97b
f045a43
7a5ae12
ebde233
c1f4e7a
b09d8ff
c0d0fa2
0bba1b5
f981ecd
61e5ecb
1728867
c746a4e
08ff33d
f340c45
0d27227
a2eb1a4
8d90e4b
033521a
db2e0e7
7fbc99b
736dd68
d75081a
ec21e7b
a0374d9
c05d384
3ded6c6
ad6c659
36cc08d
f13a9e6
8c9f038
bdb5419
8d65650
b5e6832
4da423e
ad843d2
f60f723
bced3b3
d476723
c0fdab3
cda741f
70e17f0
6bf4218
481b736
8d9b8b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,17 +18,25 @@ | |
| */ | ||
| package org.kie.kogito.addons.quarkus.knative.serving.customfunctions; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
|
|
||
| import org.kie.kogito.event.cloudevents.utils.CloudEventUtils; | ||
| import org.kie.kogito.internal.process.workitem.KogitoWorkItem; | ||
| import org.kogito.workitem.rest.decorators.PrefixParamsDecorator; | ||
|
|
||
| import com.fasterxml.jackson.core.JsonProcessingException; | ||
| import com.fasterxml.jackson.databind.JsonNode; | ||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import com.fasterxml.jackson.databind.node.ObjectNode; | ||
|
|
||
| import io.vertx.mutiny.ext.web.client.HttpRequest; | ||
|
|
||
| import static org.kie.kogito.addons.quarkus.knative.serving.customfunctions.KnativeWorkItemHandler.CLOUDEVENT_SENT_AS_PLAIN_JSON_ERROR_MESSAGE; | ||
| import static org.kie.kogito.addons.quarkus.knative.serving.customfunctions.KnativeWorkItemHandler.ID; | ||
| import static org.kie.kogito.serverless.workflow.SWFConstants.MODEL_WORKFLOW_VAR; | ||
|
|
||
| public final class PlainJsonKnativeParamsDecorator extends PrefixParamsDecorator { | ||
|
|
||
|
|
@@ -37,12 +45,44 @@ public void decorate(KogitoWorkItem workItem, Map<String, Object> parameters, Ht | |
| if (isCloudEvent(KnativeFunctionPayloadSupplier.getPayload(parameters))) { | ||
| throw new IllegalArgumentException(CLOUDEVENT_SENT_AS_PLAIN_JSON_ERROR_MESSAGE); | ||
| } | ||
|
|
||
| super.decorate(workItem, parameters, request); | ||
| buildFromParams(workItem, parameters, request); | ||
| } | ||
|
|
||
| private static boolean isCloudEvent(Map<String, Object> payload) { | ||
| List<String> cloudEventMissingAttributes = CloudEventUtils.getMissingAttributes(payload); | ||
| return !payload.isEmpty() && (cloudEventMissingAttributes.isEmpty() || (cloudEventMissingAttributes.size() == 1 && cloudEventMissingAttributes.contains(ID))); | ||
| } | ||
|
|
||
| private void buildFromParams(KogitoWorkItem workItem, Map<String, Object> parameters, HttpRequest<?> request) { | ||
| Map<String, Object> inputModel = new HashMap<>(); | ||
|
|
||
| Object inputModelObject = parameters.get(MODEL_WORKFLOW_VAR); | ||
|
|
||
| ObjectNode inputModelCopy = null; | ||
| if (inputModelObject instanceof ObjectNode objectNode) { | ||
| ObjectMapper mapper = new ObjectMapper(); | ||
| objectNode.fields().forEachRemaining(entry -> { | ||
| JsonNode value = entry.getValue(); | ||
| Object rawValue = mapper.convertValue(value, Object.class); | ||
| inputModel.put(entry.getKey(), rawValue); | ||
| }); | ||
|
|
||
| try { | ||
| inputModelCopy = (ObjectNode) mapper.readTree(objectNode.toString()); | ||
| } catch (JsonProcessingException e) { | ||
| throw new RuntimeException("Failed to copy MODEL_WORKFLOW_VAR", e); | ||
| } | ||
| } | ||
|
|
||
| Set<String> paramsRemove = super.extractHeadersQueries(workItem, inputModel, request); | ||
| super.decorate(workItem, parameters, request); | ||
|
|
||
| if (inputModelCopy != null) { | ||
| // mutate the safe copy | ||
| inputModelCopy.remove(paramsRemove); | ||
|
|
||
| // replace the original entry in parameters with the copy | ||
| parameters.put(MODEL_WORKFLOW_VAR, inputModelCopy); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Im unable to understand the purpose of this code.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is too confusing.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I implemented this a long time ago. But from what I remember, this was to avoid sending the headers and queries from the params (HEADER_* and QUERY_*) as arguments to the service when they are already processed as headers and queries. But yes, I agree with you; this is too confusing at the moment. I will find another way of doing this more cleanly. |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need to do that?
How is this change related with the issue description?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was a leftover from the previous implementation where I was not considering the $WORKFLOW variable yet. However, this should be added to the Knative custom function to give control to the developer over the status code, message, and headers. What do you think? Do I create another issue/PR for that?