Skip to content

Added the possibility to override the webservice location #23

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

Closed
wants to merge 12 commits into from

Conversation

pascal-hofmann
Copy link

Allows to use a custom request-target for soap requests. Based on work from @floriansimon1 in #7.

I rebased on master, fixed the unit tests and renamed withTarget() to withRequestTarget(). I think this name is better (same name is used in PSR-7: RequestInterface::withRequestTarget()).

Copy link
Owner

@clue clue left a comment

Choose a reason for hiding this comment

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

Thanks for digging into this and filing this PR! I like the direction this is heading!

#### withRequestTarget($requestTarget)

This method allows you to change the destination of your SOAP calls. It returns a new Client instance
with the specified request-target.
Copy link
Owner

Choose a reason for hiding this comment

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

How does this relate to the getLocation() method (and its name)?

This could also use some tests I suppose :-)

Copy link
Author

Choose a reason for hiding this comment

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

I can rename to setLocation, but thought this may be confusing because getLocation and setLocation are not really getters / setter for the same thing.

This PR adds tests in tests/FunctionalTest.php.

@@ -35,4 +39,12 @@ public function __doRequest($request, $location, $action, $version, $one_way = 0
// do not actually block here, just pretend we're done...
return '';
}

public function requestTarget($requestTarget)
Copy link
Owner

Choose a reason for hiding this comment

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

How does this method name relate to the one in the Client? It's my understanding that they should use related names?

Copy link
Author

Choose a reason for hiding this comment

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

Oops. Will fix this.

@pascal-hofmann
Copy link
Author

ClientEncoder::requestTarget has been renamed to withRequestTarget now.

@pascal-hofmann
Copy link
Author

This PR has now been open for nearly 4 months, the similar PR #7 by @floriansimon1 for over 2 1/2 years. How can I help to get this merged?

@clue
Copy link
Owner

clue commented Sep 8, 2018

@pascal-hofmann Thank you for your patience and thank you for looking into this!

I've recently started working on this project again and I agree that it makes sense to have this feature. I will get back this PR as soon as time permits 👍

@clue
Copy link
Owner

clue commented Nov 6, 2018

@floriansimon1 and @pascal-hofmann, again thank you for your patience and for working on this! I've finally(!) merged #38 which builds on top of your work and renamed this to withLocation() to match the existing getLocation() method and PHP's __setLocation() method.

Closed via #38 :shipit:

@clue clue closed this Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants