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.

Sign up for GitHub

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

Jump to bottom

[JBPM-9738] Adding support for headers in WebServiceWorkItemHandler #1992

Merged
merged 2 commits into from Aug 24, 2021
Merged

[JBPM-9738] Adding support for headers in WebServiceWorkItemHandler #1992

merged 2 commits into from Aug 24, 2021

Conversation

fjtirado
Copy link
Contributor

@fjtirado fjtirado commented Aug 5, 2021 •
edited

Supporting WebServiceWorkItemHandler parameters of the form HEADER_xxxx and HEADER_xxxx_NS, where xxxx is the name of the header and value is the content and the namespace of the soap header to be added

JIRA:

link

referenced Pull Requests:
kiegroup/droolsjbpm-integration#2566

@fjtirado fjtirado mentioned this pull request Aug 5, 2021
Merged
elguardian
elguardian approved these changes Aug 9, 2021
View reviewed changes
Copy link
Member

@elguardian elguardian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

gmunozfe
gmunozfe reviewed Aug 9, 2021
View reviewed changes
Copy link
Member

@gmunozfe gmunozfe left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think defining a hardcoded separator (like underscore in this case) may not work for all the customers. Suppose they want to send a header like "Pepito_Grillo", could it be configurable to make it more flexible?

@fjtirado fjtirado requested a review from gmunozfe August 10, 2021 10:17
@fjtirado
Copy link
Contributor Author

I think defining a hardcoded separator (like underscore in this case) may not work for all the customers. Suppose they want to send a header like "Pepito_Grillo", could it be configurable to make it more flexible?

Ok, changed approach, using HEADER_<HEADER_NAME> = <HEADER_CONTENT> and HEADER_PARAM_<PARAM_NAME><HEADER_NAME> = <PARAM_CONTENT>
"" separator is configurable using "org.kie.workitem.ws.header.separator" system property

gmunozfe
gmunozfe suggested changes Aug 11, 2021
View reviewed changes
Copy link
Member

@gmunozfe gmunozfe left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getHeaderInfo is not invoked at integration test.
Check also indentation (spaces vs. tabs)

...webservice/src/main/java/org/jbpm/process/workitem/webservice/WebServiceWorkItemHandler.java Show resolved Hide resolved
@fjtirado fjtirado requested a review from gmunozfe August 12, 2021 12:20
@fjtirado
Copy link
Contributor Author

jenkins retest this

@fjtirado
Copy link
Contributor Author

unrelated, jenking retest this

@fjtirado
Copy link
Contributor Author

jenkins retest this

@fjtirado fjtirado force-pushed the JBPM-9738 branch 8 times, most recently from b84cbe5 to 4c60219 Compare August 19, 2021 16:23
@fjtirado fjtirado force-pushed the JBPM-9738 branch 15 times, most recently from 25b8067 to 27b42ef Compare August 23, 2021 09:12
@fjtirado fjtirado force-pushed the JBPM-9738 branch 8 times, most recently from 48bef16 to ae1a965 Compare August 23, 2021 12:18
gmunozfe
gmunozfe approved these changes Aug 23, 2021
View reviewed changes
Copy link
Member

@gmunozfe gmunozfe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, brilliant work @fjtirado !

@sonarcloud
Copy link

sonarcloud bot commented Aug 23, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@elguardian
Copy link
Member

failure not related ... env problem related to container not up.

@elguardian elguardian merged commit 139a2a4 into kiegroup:main Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmunozfe gmunozfe gmunozfe approved these changes

@elguardian elguardian elguardian approved these changes

@afalhambra afalhambra Awaiting requested review from afalhambra

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@fjtirado @elguardian @gmunozfe