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

Configuration keys overwrite each other #47

Open
jwulf opened this issue Feb 11, 2020 · 4 comments
Open

Configuration keys overwrite each other #47

jwulf opened this issue Feb 11, 2020 · 4 comments

Comments

@jwulf
Copy link
Member

jwulf commented Feb 11, 2020

Configuration keys from variable mappings clobber those from custom headers when merged to form the body of a POST request. For example:

Custom Header:
body: {event_type: "message", client_payload: {message_name: "THROW_YO"}}

Input Variable mapping:
test: body.client_payload.test

Expected Output:

{
    body: {
		event_type: "message", 
		client_payload: {
			message_name: "THROW_YO",
			test: ${value of variables.test}
		}
	}
}

Actual output:

{
	body: {
		client_payload: {
			test:"value"
	}
}

I expect a JSON merge. It's probably non-trivial to write in Java, but if you create a ScriptEngine you can have it convert the Maps to JSON and do an Object.assign.

"JavaScript: JSON done right"

@jwulf
Copy link
Member Author

jwulf commented Feb 11, 2020

I'll PR it in.

@saig0
Copy link
Contributor

saig0 commented Feb 11, 2020

Hey @jwulf, I'm not sure about the expected behavior you describe. Merging the custom headers with the variables was not intended originally. And it is nothing that is supported by the broker. Introducing this special logic only for this worker could be confusing for users.

We will add advanced expression support in the broker that will allow to do this in the variable mappings without defining the task header (camunda/camunda#3417).

@jwulf
Copy link
Member Author

jwulf commented Feb 11, 2020

Ar the moment you can merge headers and variables, but not in depth. Top-level keys overwrite everything when merged.

The current naive implementation is here:

https://github.com/zeebe-io/zeebe-http-worker/blob/master/src/main/java/io/zeebe/http/ConfigurationMaps.java#L41

This is in the body of the POST request.

So I can either set the POST body via a custom header, or via an input variable mapping, but I cannot do both - because any input variable mapping I do overwrites the entire custom header object.

This is a problem because the CAMUNDA-HTTP task cannot be specialised in the model where the specialisation requires a constant field in the POST body. It is so close right now.

It means that - as it stands - if you create a message publishing task using the Camunda Cloud GitHub Message Publish Workflow, you can either specialise the task in the model, but send no variables in the message, or else you have to put the message name in the variable payload, which is pretty useless.

@jwulf jwulf mentioned this issue Feb 11, 2020
@jwulf
Copy link
Member Author

jwulf commented Feb 12, 2020

See here for the use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants