Added the possibility to override the webservice location #7

floriansimon1 · 2015-09-03T13:18:22Z

Allows one to load the WSDL from one place and to query a server that is different from the one specified in the WSDL

…e to load the WSDL from one place and to query a server that is different from the one specified in the WSDL

clue · 2015-09-03T13:27:05Z

Awesome, the feature looks great and is very much appreciated! 👍

I understand this could be a bit cumbersome, but have you tried looking into adding a test? :-)

I also like the immutable interface. Should we take #5 into account here?

floriansimon1 · 2015-09-03T14:09:21Z

I will add the test now while working on another feature I need to make your library work in my project, don't worry.

I don't understand what you mean by "take #5" into account, though. My commit doesn't address this. If there is something I can do to help, let me know !

To satisfy the needs of our customer, I need to be able to get the location in the WSDL, append a URL parameter to it and use the overrideLocation() method to send queries to that new location. SoapClient makes it impossible to retrieve the location stored in the WSDL. To get it, I'd have to resort to using a hack. I have several solutions, but they're all hackish. The best I found is this one :

The client constructor calls seekWSDLLocationMode() on ClientEncoder which sets some flag to true.
When the flag is set to true, the __doRequest method takes the location, stores it inside the ClientEncoder and sets the flag to false.
In the client constructor, we send a fake request to make the encoder set its 'WSDLLocation' variable.
We put a method on the ClientEncoder AND the client to retrieve the location, something like getWSDLLocation().

I need that feature. I know it's hackish. As far as I know, the alternatives are no better. My questions are :

Do you know of any better way ?
If I implement this, will you accept to merge my work on your repo ? I would understand if you didn't, but it would help me a great deal, while providing your users more features.

- Added tests for the location override feature

floriansimon1 · 2015-09-03T14:31:55Z

I've pushed tests. Not optimal, but they work !

clue · 2015-09-03T14:38:22Z

I will add the test now while working on another feature I need to make your library work in my project, don't worry.

Awesome! :-)

I don't understand what you mean by "take #5" into account, though. My commit doesn't address this.

Indeed, this isn't directly related. I was more referring to how PHP's normal SoapClient API works. Eventually, we should also support non-WSDL-mode and hence offer a way to specify the location and uri attributes. This is why this might be related to this ticket.

SoapClient makes it impossible to retrieve the location stored in the WSDL. To get it, I'd have to resort to using a hack. I have several solutions, but they're all hackish. The best I found is this one […]
If I implement this, will you accept to merge my work on your repo ?

Now this is a very interesting hack :-D
In fact, I consider most of this library to be a huge hack, so I'm not opposed to getting something like this in at all :-)

As far as I know, the alternatives are no better. […] Do you know of any better way ?

Have you looked into using the __setLocation() method already? Untested pseudo code ahead:

public function getLocation()
{
    $ret = $this->soap->__setLocation();
    $this->soap->__setLocation($ret);

    return $ret;
}

floriansimon1 · 2015-09-03T14:41:29Z

Well done sir. It seems to work. I should be reading docs more !

UPDATE : No, it doesn't. It returns what you set during the previous setLocation call :(

clue · 2015-09-03T15:05:26Z

src/Protocol/ClientEncoder.php

+        $copy = clone $this;
+        $this->locationOverride = $newLocation;
+        return $copy;
+    }


PHP's SoapClient already offers a __setLocation() method, perhaps we can rely on this there instead of manually keeping track of the location? (see also above)

I tried it, it does not work :(

- Test for the location getter

- Renamed overrideTarget to withOverridenTarget() everywhere

floriansimon1 · 2015-09-03T15:29:07Z

Indeed, this isn't directly related. I was more referring to how PHP's normal SoapClient API works. Eventually, we should also support non-WSDL-mode and hence offer a way to specify the location and uri attributes. This is why this might be related to this ticket.

OK. This new feature should be implemented with the future in mind. If I rename withOverridenTarget to withTarget, which I will do in a minute, you'll be able to offer the same interface to your users when #5 is implemented, while adding more functions. Is it OK for you ?

…array() call

…as passed a reference

…array() call

clue · 2015-09-04T16:11:14Z

It seems to work […]
UPDATE : No, it doesn't. It returns what you set during the previous setLocation call :(

Indeed :) Initially, this ticket was about only overriding the location setting. This is now about accessing the location setting, so I'd rather like to keep this separate. See #13 for more details.

rename withOverridenTarget to withTarget […]. Is it OK for you ?

Looks good so far :) May I ask you to remove the other changes related to accessing the location so that we can ease the review? Thanks!

floriansimon1 · 2015-09-15T06:41:04Z

I've read all your comments. I'll make the changes soon

floriansimon1 · 2015-09-19T10:44:16Z

Looks good so far :) May I ask you to remove the other changes related to accessing the location so > that we can ease the review? Thanks!

Done !

(Tests fails for the same unknown reason as for PR #14)

clue · 2016-03-15T14:38:27Z

The changes LGTM and I'd love to get this in Can you update the PR / rebase on current master? 👍

Conflicts: README.md src/Client.php

floriansimon1 · 2016-03-16T07:37:33Z

Done!

pascal-hofmann · 2017-11-07T12:10:54Z

Is there a timeline for v0.3.0? I need this feature. :-)

clue · 2017-11-07T12:29:44Z

This currently has a merge conflict and there are currently no immediate plans to build this from my end (no demand at the moment and more important outstanding issues currently), but I would be really happy to accept any help with this 👍

(If you need this for a commercial project and you want a quote, please check out my profile and send me an email) :-)

pascal-hofmann · 2017-11-07T16:32:40Z

I resolved the merge conflict and cleaned up the code a bit. See #23.

Would be great to have v0.3.0 including this.

clue · 2018-09-08T14:44:02Z

@floriansimon1 Thank you for your patience and thank you for working on this!

I've recently started working on this project again and I agree that it makes sense to have this feature. I see that @pascal-hofmann has picked up your work and added some tests for this in #23, so I will close this PR for now and I will get back to the other PR as soon as time permits 👍

Added the possibility to override the webservice location to allow on…

7d76036

…e to load the WSDL from one place and to query a server that is different from the one specified in the WSDL

clue added the new feature label Sep 3, 2015

- Introduced PHP Unit as a development composer dependency

ed30018

- Added tests for the location override feature

clue reviewed Sep 3, 2015
View reviewed changes

Florian Simon added 2 commits September 3, 2015 17:12

- Documentation/better naming for the location getter/override.

913d3fa

- Test for the location getter

- Removed the dependency to PHPUnit in composer.json

c20c08f

- Renamed overrideTarget to withOverridenTarget() everywhere

Florian Simon added 2 commits September 3, 2015 17:30

Renamed withOverridenTarget() to withTarget() everywhere

f9b067d

Replaced one instance of shorthand array syntax use with a plain old …

d39e8ea

…array() call

floriansimon1 closed this Sep 3, 2015

floriansimon1 reopened this Sep 3, 2015

Florian Simon added 2 commits September 3, 2015 18:29

Made a copy of the location before assigning it because __doRequest w…

cd0f0b9

…as passed a reference

Replaced one instance of shorthand array syntax use with a plain old …

2dfeb4c

…array() call

floriansimon1 force-pushed the master branch from 150c78c to 2dfeb4c Compare September 4, 2015 08:00

Overwriting the target of the copy instead of the original ClientEncoder

e82a1cb

Removed features that don't belong in that pull request

cbb9743

Florian Simon added 3 commits March 16, 2016 08:22

Merge branch 'master-clue'

5a3008c

Conflicts: README.md src/Client.php

Merge branch 'master' of github.com:floriansimon1/php-soap-react

d308f8d

Conflicts: README.md src/Client.php

Fixes tests

bf6966e

clue added this to the v0.3.0 milestone Oct 2, 2017

pascal-hofmann mentioned this pull request Nov 7, 2017

Added the possibility to override the webservice location #23

Closed

clue removed this from the v0.3.0 milestone Sep 8, 2018

clue closed this Sep 8, 2018

clue mentioned this pull request Nov 6, 2018

Add new Client::withLocation() method #38

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added the possibility to override the webservice location #7

Added the possibility to override the webservice location #7

floriansimon1 commented Sep 3, 2015

clue commented Sep 3, 2015

floriansimon1 commented Sep 3, 2015

floriansimon1 commented Sep 3, 2015

clue commented Sep 3, 2015

floriansimon1 commented Sep 3, 2015

clue Sep 3, 2015

floriansimon1 Sep 3, 2015

floriansimon1 commented Sep 3, 2015

clue commented Sep 4, 2015

floriansimon1 commented Sep 15, 2015

floriansimon1 commented Sep 19, 2015

clue commented Mar 15, 2016

floriansimon1 commented Mar 16, 2016

pascal-hofmann commented Nov 7, 2017

clue commented Nov 7, 2017

pascal-hofmann commented Nov 7, 2017 •

edited

Loading

clue commented Sep 8, 2018

Added the possibility to override the webservice location #7

Added the possibility to override the webservice location #7

Conversation

floriansimon1 commented Sep 3, 2015

clue commented Sep 3, 2015

floriansimon1 commented Sep 3, 2015

floriansimon1 commented Sep 3, 2015

clue commented Sep 3, 2015

floriansimon1 commented Sep 3, 2015

clue Sep 3, 2015

Choose a reason for hiding this comment

floriansimon1 Sep 3, 2015

Choose a reason for hiding this comment

floriansimon1 commented Sep 3, 2015

clue commented Sep 4, 2015

floriansimon1 commented Sep 15, 2015

floriansimon1 commented Sep 19, 2015

clue commented Mar 15, 2016

floriansimon1 commented Mar 16, 2016

pascal-hofmann commented Nov 7, 2017

clue commented Nov 7, 2017

pascal-hofmann commented Nov 7, 2017 • edited Loading

clue commented Sep 8, 2018

pascal-hofmann commented Nov 7, 2017 •

edited

Loading