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

Implicit process variables have names appended with query character #48

Open
NickTheArchitect opened this issue Feb 28, 2019 · 9 comments

Comments

@NickTheArchitect
Copy link

Referencing my Forum post: https://forum.camunda.org/t/what-is-issue-36-introduce-optional-variables-in-the-camunda-camel-extension/11453

If the process calls the camel service bean with no explicit variables, in the past, all the process variables were put into the Camel exchange as name-value pairs in a Map in the Body. We use this capability to decouple our Camel routes from our Camunda processes. The routes can pick whichever variables they want to use from the set of all process variables in the process execution, instead of being told what they can have by the call to the Camel service bean from that process.

There is a commit; 87322c8 which is part of pull request 37 (#37), which is titled ’ Issue 36: Introduce optional variables’. This change seems to be a breaking change to the Camel service bean, which has changed all the names of the process variables being implicitly fetched from the process execution by appending '?'. What is the intent of this? AFAIK, this should be a defect, since the documentation doesn't tell me why I should rename all the process variables that we use in all our Camel routes to have a '?' on the end, so that I can upgrade to version 0.6.

Sadly, I also need to do the upgrade because 0.5 seems to be responsible for the asyncResponseTimeout in the REST ExternalTask API 'fetchAndLock' not working properly. See my earlier forum post https://forum.camunda.org/t/fetchandlock-using-asyncresponsetimeout-returns-empty-body/9511

@stephanpelikan
Copy link
Contributor

The commit 87322c8 did not change the behavior you describe:

Still, if there are no variables declared in the routes, all process variables will be exposed. Previously, if there was a null-valued variable, you got an exception but now you get a map entry with a null value. This change is also documented.

If you declare variables in the route the question marks may be placed but don't need to. Without them it behaves like before: you get an exception for variables with null values. Placing the question mark only skips this null check.

But yes, there seems to be a bug that all variable names are exposed including the question marks. I have to test this and I will send an update for this.

But upgrading to a newer Camunda version should be possible without upgrading the Camunda camel plugin. I don't see any dependency because the plugin does not use the rest API.

@berndruecker
Copy link
Contributor

Thanks @stephanpelikan for looking into it! If anybody could provide a PR I am more than happy to merge it - and then we could also release a fix version

@berndruecker
Copy link
Contributor

Probably this line?
87322c8#diff-c2b0f268eee6f094e1054fe109f4dd7bR55

@NickTheArchitect
Copy link
Author

The problem seems to be that the code has changed between version 0.5 and 0.6 to append a '?' to the variable name in the list, if the list is pulled from the execution, rather than explicitly named. When the value of the variable is fetched, the code recognizes if the name ends in a '?' and fetches by the name without that character, but when the name/value pair is added to the list to send, the name used is the one that includes the '?'. Unfortunately, I don't know what the requirement is that caused the code to change, so I can only fix it by removing all the '?' usage and effectively put the code back to how it was in 0.5.
I'm happy to do that and provide a PR, but I think I'm not seeing the whole picture.
@berndruecker WDYT?

NickTheArchitect pushed a commit to NickTheArchitect/camunda-bpm-camel that referenced this issue Mar 2, 2019
NickTheArchitect added a commit to NickTheArchitect/camunda-bpm-camel that referenced this issue Mar 2, 2019
stephanpelikan pushed a commit to stephanpelikan/camunda-bpm-camel that referenced this issue Mar 2, 2019
stephanpelikan pushed a commit to stephanpelikan/camunda-bpm-camel that referenced this issue Mar 2, 2019
@stephanpelikan
Copy link
Contributor

@NickTheArchitect:
You changed a lot of whitespace. May you use a different code formatter than defined at https://github.com/camunda/camunda-bpm-platform/blob/master/CONTRIBUTING.md.
I adopted your commit by removing those changes and also tried to make it a litte bit more readable. I hope you agree with this.

@NickTheArchitect
Copy link
Author

@stephanpelikan
I agree. Thank you so much for interpreting my code change into a useful form.
What would be the next steps for this? Will you create a PR?

@berndruecker
Copy link
Contributor

as far as I can see the changes are committed to master. Can you confirm that this issue is fixed by them? Then I could release a new version with that change.

@NickTheArchitect
Copy link
Author

@berndruecker
Sorry, I don't see the commit in master. AFAIK it is only in stephanpelikan/camunda-bpm-camel

@stephanpelikan
Copy link
Contributor

Yes, I have to do a pull request. I wanted to test it before. Sorry for the delay, there are a lot of things to do at the moment. I try to make it this week.

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

No branches or pull requests

3 participants