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

GH-2818: DSL support for -ws module #3178

Merged
merged 3 commits into from
Feb 20, 2020
Merged

GH-2818: DSL support for -ws module #3178

merged 3 commits into from
Feb 20, 2020

Conversation

garyrussell
Copy link
Contributor

Resolves #2818

Docs to follow tomorrow.

artembilan
artembilan requested changes Feb 13, 2020
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Some review to think about.

Thanks

...n-ws/src/main/java/org/springframework/integration/ws/AbstractWebServiceOutboundGateway.java Outdated
* @param messageSender the sender.
* @deprecated in favor of {@link #setMessageSenders(WebServiceMessageSender...)}
*/
@Deprecated
public void setMessageSender(WebServiceMessageSender messageSender) {
Assert.state(!this.webServiceTemplateExplicitlySet,
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind to change logic then in favor of calling a replacing API instead?
Then it is going to be much easier to support possible changes in the target logic afterward.

Copy link
Member

Choose a reason for hiding this comment

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

This has not been addressed.
Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I missed this; but, that said, I don't understand it.

Copy link
Member

Choose a reason for hiding this comment

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

I mean this:

public void setMessageSender(WebServiceMessageSender messageSender) {
   setMessageSenders(messageSender);
}

So, we don't need to have that Assert in this method any more. The logic from a deprecated method is fully delegated to replacing API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doh; ok.

...ation-ws/src/main/java/org/springframework/integration/ws/dsl/BaseWsOutboundGatewaySpec.java Show resolved Hide resolved
...ws/src/main/java/org/springframework/integration/ws/dsl/MarshallingWsInboundGatewaySpec.java Outdated Show resolved Hide resolved
...s/src/main/java/org/springframework/integration/ws/dsl/MarshallingWsOutboundGatewaySpec.java Outdated Show resolved Hide resolved
...ion-ws/src/main/java/org/springframework/integration/ws/dsl/SimpleWsOutboundGatewaySpec.java Outdated Show resolved Hide resolved
@artembilan
Copy link
Member

@garyrussell ,

any updates? Or do you want me to take over this from you?

Thanks

@garyrussell
Copy link
Contributor Author

garyrussell commented Feb 20, 2020 •
edited
Loading

?

I pushed rework a couple of days ago - didn't you get a notification?

d4bdbc0

@artembilan
Copy link
Member

garyrussell force-pushed the garyrussell:GH-2818 branch from 2a96e81 to d4bdbc0

Seems for me when we "force-pushed", there is no notification. GH flaw? 😉

OK. Sorry for noise: I should check PR before bothering you.
Will review soon.

Thanks

@garyrussell
Copy link
Contributor Author

I know force-push for a rebase doesn't notify, but I assumed force push with additional commits would - but I guess not.

In future, I'll try to remember to push the new commit; then force-push after rebasing.

artembilan
artembilan requested changes Feb 20, 2020
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Some non-critical review.

I can address them all on merge (including an integration test)

Thanks

P.S. Although there has to be some Docs on the matter! 😄

...n-ws/src/main/java/org/springframework/integration/ws/AbstractWebServiceOutboundGateway.java Outdated
* @param messageSender the sender.
* @deprecated in favor of {@link #setMessageSenders(WebServiceMessageSender...)}
*/
@Deprecated
public void setMessageSender(WebServiceMessageSender messageSender) {
Assert.state(!this.webServiceTemplateExplicitlySet,
Copy link
Member

Choose a reason for hiding this comment

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

This has not been addressed.
Thanks

...ration-ws/src/main/java/org/springframework/integration/ws/dsl/BaseWsInboundGatewaySpec.java Show resolved Hide resolved
...ion-ws/src/main/java/org/springframework/integration/ws/dsl/SimpleWsOutboundGatewaySpec.java Show resolved Hide resolved
...s/src/main/java/org/springframework/integration/ws/dsl/MarshallingWsOutboundGatewaySpec.java Outdated
* @param faultMessageResolver the resolver.
* @return the spec.
*/
public MarshallingWsOutboundGatewayNoTemplateSpec faultMessageResolver(FaultMessageResolver faultMessageResolver) {
Copy link
Member

Choose a reason for hiding this comment

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

Longer than 120.

...s/src/main/java/org/springframework/integration/ws/dsl/MarshallingWsOutboundGatewaySpec.java Outdated
this.gatewayUnmarshaller, this.webServiceMessageFactory);
}
else {
return new MarshallingWebServiceOutboundGateway(this.uri, this.gatewayMarshaller, this.gatewayUnmarshaller,
Copy link
Member

Choose a reason for hiding this comment

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

Longer than 120.

spring-integration-ws/src/test/java/org/springframework/integration/ws/dsl/WsDslTests.java Show resolved Hide resolved
@garyrussell
spring-projectsGH-2818: DSL support for -ws module
5789d74 
Resolves spring-projects#2818
@garyrussell
* Rework to improve fluency
1575fbe 
- reduce the number of factory methods and use a different spec when
  an external WST is provided.
artembilan
artembilan requested changes Feb 20, 2020
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

OK. One more comment and code style. Plus docs and we are good to go.
Everything else is in agreement silently.

@garyrussell
Copy link
Contributor Author

Force pushed with additional commit after rebase for doc conflicts.

@artembilan artembilan merged commit ea8c454 into spring-projects:master Feb 20, 2020
@garyrussell garyrussell deleted the GH-2818 branch February 21, 2020 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artembilan artembilan artembilan requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Java DSL for web services
2 participants
@garyrussell @artembilan