Masterpricer Expert Search #170

m0veax · 2018-03-27T07:11:29Z

Hi,

i created a base PR so you can see my progress regarding Issue #168.

There are no Unit Tests or Docs added right now. But this is working and returning Results.

I tried to copy only what is needed but i somehow think, maybe the Master Pricer Base Struct could be modified to hold more of the three Master Pricer types?

Any thoughts for me?

Thanks for your fast Support :)

coveralls · 2018-03-27T07:14:40Z

Coverage decreased (-0.01%) to 99.844% when pulling f5737d1 on proappers:mpexpert into 58f7240 on amabnl:master.

DerMika · 2018-03-27T09:01:41Z

The first thing I notice is that the request options object src/Amadeus/Client/RequestOptions/FareMasterPricerExSearch.php should follow the naming scheme of the other messages and end in "Options" (I know MasterPricerTravelBoardSearch is an exception to that naming scheme, but that was a mistake by me ;) )

Also, it seems to copy a lot of the MasterPricerTravelBoardSearch request options, perhaps you should consider extending FareMasterPricerTbSearch from your new request options object, whichever is most limited.

The same with building the request structure, you seem to have copy-pasted a lot (which is a good start, but I don't want unnecessary copy-pasting in the released version).

And obviously, unittests still need to be added, because I want the library at 100% (minus 1 file which I haven't figured out how to test completely yet).

so, you're off to a good start

DerMika · 2018-03-27T09:03:38Z

Also, MPBaseOptions contains all props shared between the MasterPricer call and the Ticket_CheckEligibility message, so you can't add anything to MPBaseOptions which isn't available in Ticket_CheckEligibility

m0veax · 2018-03-27T09:11:15Z

Thanks for your feedback. I have some other things to solve right now and will try to come back to that soon.

DerMika · 2018-03-27T09:13:38Z

Sure, no rush! I'm already happy you're willing to contribute ;)

signOut method is called securitySignOut in Client.php

DerMika · 2018-07-07T07:14:48Z

Do you plan to continue working on this PR?

m0veax · 2018-07-09T11:22:20Z

Hey,

yeah i want too.

I saw last week, that you changed things in Travelboard Search, so i will have to adapt that stuff too.

DerMika · 2018-07-09T11:29:39Z

It was only a small change, but yes, I merged yet another @bimusiek PR ;)

Let me know if you need any help, I'd really like to have this message in the library too!

m0veax · 2018-07-22T20:09:42Z

Hey there, i was able to normalize most of the things you called for. One call is extending another one and i was able to solve some other things too. The naming convention is fullfilled now.

Beside of the missing unit tests, is that ok now? If you give me your go, i will add the missing tests.

What would be the best way to gather the needed XML for a response test?

DerMika · 2018-07-22T21:16:55Z

What would be the best way to gather the needed XML for a response test?

I usually either make a real call with a message and remove personal info from the response, or just look if I can find a response sample in the docs

I can't do a review now, I'll have to get back to you in a few days when I have access to my computer again.

m0veax · 2018-08-29T19:08:30Z

Are the ClientTest.php Methods are autoloading some xml files? I have some trouble there. Would you be up for a short chat on irc or something?

DerMika · 2018-09-23T07:48:32Z

Sorry for the delay. Are you developing on Windows? In that case it could be a problem with case sensitivity (the unittests are ran on a linux box at TravisCI)

DerMika · 2018-09-23T07:50:03Z

You should be able to find an e-mail address in the code, use that to contact me privately ;)

DerMika · 2018-10-24T20:22:20Z

Are you going to continue working on this in short-ish timescales? I would like to release 1.8.0 and I'd like to include this message in the release, but I won't wait too long - in which case it'll be for a future release.

therealartz · 2018-10-25T05:55:20Z

Waaait a minute.

Tests are failed with error
Error: Class 'Amadeus\Client\RequestOptions\FareMasterPricerExSearch' not found
But there is not class FareMasterPricerExSearch, there is FareMasterPricerExSearchOptions.

It's just need to change the class name in the tests and in samples...

@m0veax when you ran the tests, have you ran composer dump?
On your branch I get a notice Undefined class FareMasterPricerExSearch everywhere.
Maybe just refactor class names?

m0veax · 2018-11-06T05:34:56Z

Thanks for your insights. I will check this up in my Codebase.

Are there any Resources to understand how responses are loaded in the Testframework? That is the Showstopper for me right now. I really want to finish this, but i didn't had the time to get that tests done (the kids needed a bit more attention the lasts weeks and i wanted to serve the attention they need).

Maybe the comment from @therealartz does fix my problem any way. I will try to apply his changes this month.

m0veax · 2019-01-10T20:06:18Z

Waaait a minute.

Tests are failed with error
Error: Class 'Amadeus\Client\RequestOptions\FareMasterPricerExSearch' not found
But there is not class FareMasterPricerExSearch, there is FareMasterPricerExSearchOptions.

It's just need to change the class name in the tests and in samples...

Thanks a lot for that! Somewhere i started to rename that and somehow i missed a lot.

So i just have one test that still fails and will investigate further :)

therealartz · 2019-01-10T20:33:10Z

Now the mistake in this line of your test method:

// ClientTest.php

$mockedSendResult->responseXml = 'dummyfarepricemasterpricerexpertsearchresponsemessage';

You must specify responseXml to be taken from example response file in this way:

$mockedSendResult->responseXml = $this->getTestFile('fareMasterPricerExpertSearchReply-12.4.txt');

Also I want recommend you to change your example response (fareMasterPricerExpertSearchReply-12.4.txt) to be one-lined XML, because 21k added lines confuses 😄

m0veax · 2019-01-10T21:32:01Z

@therealartz appreciated!

Thanks for your hints!

I hopefully get this done within the next two weeks. But real life is a bit demanding the last year.

m0veax · 2019-03-14T19:16:08Z

Well it seems the next two weeks was too optimistic, but i am working on this again. At the moment i have the Problem, that my XML does not generated properly. I am investigating at the moment and hope to find that last bug soon.

DerMika · 2019-03-14T21:27:07Z

Thanks for coming back to finish this :)

m0veax · 2019-03-15T07:11:00Z

Sure, i don't like unfinished things :)

Could you do me a favor and point me to the Class which is responsible for generating XML out of the Request Object?

My Request Objects seems to miss something and i need to debug this.

DerMika · 2019-03-15T07:55:37Z

Well... What this library does: it builds a PHP object through a Amadeus\Client\Struct\... message. That object is given to the PHP \SoapClient object (through the php_soap extension). It's the magic of the \SoapClient which converts the PHP object to an XML document. Unfortunately you have very little control over that. The SoapClient uses the provided WSDL definition for the message to match the PHP object and convert it to XML.

Usually when you are missing an expected part in the XML document, one of these things happened:

Something is missing or wrong in the WSDL / XSD definition loaded by the SoapClient
Your object is missing a parameter or doesn't have the correct name/datatype expected
the Soapclient somehow can't decide which definition to use when decoding part of your object, in that case providing it with a custom ClassMap can help (https://stackoverflow.com/questions/29800939/advantage-of-php-soapclient-classmap)

So if you suspect there is a mismatch between your object and the WSDL definition, you may try these things:

double check your object, did you name every property correctly, does it contain the expected value
try to hack the WDL / XSD into coercion
let Amadeus know the WSDL seems broken, they may know of the issue and provide you with an updated WSDL
add a map of WSDL/ XSD namespace definitions --> PHP objects to the ClassMap (https://github.com/amabnl/amadeus-ws-client/blob/master/src/Amadeus/Client/Session/Handler/Classmap.php)
if you're really determined, you can go debug the php soapclient to see why it does what it does. I've never ventured into that territory yet myself...

m0veax · 2019-03-15T10:33:57Z

Very helpfull, thanks for your input!

m0veax · 2019-03-15T19:36:59Z

So, due to your informations i was able to trace down, that my code generates valid SOAP Requests.

I tested the library against the API too and get valid results. So i am doing something wrong in ClientTest.php itself. I gonna dig down that path now.

therealartz · 2019-03-27T12:03:27Z

I had a free hour and I decided to help you.

Found a mistake in test set up.

Could you please give me access to push in your fork?

m0veax · 2019-03-27T14:17:55Z

@therealartz I granted push permissions for you.

I am looking forward to see your changes. Guess I can learn a thing there :)

m0veax · 2019-03-27T15:20:19Z

@therealartz big thanks! I guess I would have spend some working days to find this, since tests are not my daily business yet. But I started to improve my knowledge there already.

@DerMika is this mergable or do we need some more changes :)

DerMika · 2019-03-27T15:28:54Z

I will review tonight...

m0veax · 2019-03-27T15:30:15Z

No hurry :) I just have been happy about this solution

m0veax · 2019-04-25T18:49:36Z

Hey Mika,

anything you need me to do here?

I don't want to hurry you up, but i have some spare time this weekend and would be able to investigate :)

DerMika · 2019-05-25T06:49:53Z

Ok, finally going to merge this. Sorry it took so long!

And thanks for your contribution, much appreciated!

Patrick Kilter added 6 commits March 27, 2018 08:21

added method to $client refs amabnl#168

572c5bf

copied request obj for later reducing refs amabnl#168

aaa4e01

added expert search struct class refs amabnl#168

db49208

added request creation refs amabnl#168

964bcb6

added response handler refs amabnl#168

4db47dd

fixed search option call refs amabnl#168

e1dfe6a

Patrick Kilter added 3 commits March 27, 2018 09:26

styleguide refs amabnl#168

f79d4db

added the right tabbing refs amabnl#168

d8f09d8

styleci indention / empty rows refs amabnl#168

815bdf2

m0veax and others added 2 commits March 28, 2018 13:49

Update how-to.rst

1f83144

signOut method is called securitySignOut in Client.php

corrected docs for sign out

41f760e

DerMika added the new message label Mar 31, 2018

DerMika added this to the 1.7.0 milestone Mar 31, 2018

DerMika modified the milestones: 1.7.0, 1.8.0 Apr 30, 2018

Patrick Kilter added 3 commits July 16, 2018 22:02

Merge branch 'master' of https://github.com/amabnl/amadeus-ws-client

f8da7dd

Merge branch 'master' into mpexpert_v2

374e72b

extending each other refs amabnl#170

e25e92e

Patrick Kilter added 2 commits July 22, 2018 22:10

styleguide fixes refs amabnl#170

d08cd9b

options naming conventions refs amabnl#170

13687c0

Patrick Kilter added 2 commits August 26, 2018 05:32

Merge branch 'mpexpert_dev' into mpexpert

4b83c30

fixed style ci issues by applying diff

a7f546a

fixed several naming errors refs amabnl#170

80ba023

Patrick Kilter added 2 commits March 14, 2019 19:15

made test file a one liner

221e600

added proper testfile

6b3c4bf

therealartz added 2 commits March 27, 2019 14:00

DRY in HandlerMasterPricerExpertSearch

ca0699a

Mock session handler

f5737d1

DerMika merged commit b20918e into amabnl:master May 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Masterpricer Expert Search #170

Masterpricer Expert Search #170

m0veax commented Mar 27, 2018

coveralls commented Mar 27, 2018 •

edited

Loading

DerMika commented Mar 27, 2018 •

edited

Loading

DerMika commented Mar 27, 2018 •

edited

Loading

m0veax commented Mar 27, 2018

DerMika commented Mar 27, 2018

DerMika commented Jul 7, 2018

m0veax commented Jul 9, 2018

DerMika commented Jul 9, 2018

m0veax commented Jul 22, 2018 •

edited

Loading

DerMika commented Jul 22, 2018

m0veax commented Aug 29, 2018

DerMika commented Sep 23, 2018

DerMika commented Sep 23, 2018

DerMika commented Oct 24, 2018

therealartz commented Oct 25, 2018 •

edited

Loading

m0veax commented Nov 6, 2018

m0veax commented Jan 10, 2019

therealartz commented Jan 10, 2019

m0veax commented Jan 10, 2019

m0veax commented Mar 14, 2019

DerMika commented Mar 14, 2019

m0veax commented Mar 15, 2019

DerMika commented Mar 15, 2019

m0veax commented Mar 15, 2019

m0veax commented Mar 15, 2019

therealartz commented Mar 27, 2019

m0veax commented Mar 27, 2019

m0veax commented Mar 27, 2019

DerMika commented Mar 27, 2019

m0veax commented Mar 27, 2019

m0veax commented Apr 25, 2019

DerMika commented May 25, 2019

Masterpricer Expert Search #170

Masterpricer Expert Search #170

Conversation

m0veax commented Mar 27, 2018

coveralls commented Mar 27, 2018 • edited Loading

DerMika commented Mar 27, 2018 • edited Loading

DerMika commented Mar 27, 2018 • edited Loading

m0veax commented Mar 27, 2018

DerMika commented Mar 27, 2018

DerMika commented Jul 7, 2018

m0veax commented Jul 9, 2018

DerMika commented Jul 9, 2018

m0veax commented Jul 22, 2018 • edited Loading

DerMika commented Jul 22, 2018

m0veax commented Aug 29, 2018

DerMika commented Sep 23, 2018

DerMika commented Sep 23, 2018

DerMika commented Oct 24, 2018

therealartz commented Oct 25, 2018 • edited Loading

m0veax commented Nov 6, 2018

m0veax commented Jan 10, 2019

therealartz commented Jan 10, 2019

m0veax commented Jan 10, 2019

m0veax commented Mar 14, 2019

DerMika commented Mar 14, 2019

m0veax commented Mar 15, 2019

DerMika commented Mar 15, 2019

m0veax commented Mar 15, 2019

m0veax commented Mar 15, 2019

therealartz commented Mar 27, 2019

m0veax commented Mar 27, 2019

m0veax commented Mar 27, 2019

DerMika commented Mar 27, 2019

m0veax commented Mar 27, 2019

m0veax commented Apr 25, 2019

DerMika commented May 25, 2019

coveralls commented Mar 27, 2018 •

edited

Loading

DerMika commented Mar 27, 2018 •

edited

Loading

DerMika commented Mar 27, 2018 •

edited

Loading

m0veax commented Jul 22, 2018 •

edited

Loading

therealartz commented Oct 25, 2018 •

edited

Loading