[ICAT] Broken windows tests

[Jay Rainey]

Changes made in #8581 broke the ​tests on clean windows 7 develop. Interestingly, ICAT within Mantid no longer works on W7.(If you attempt to log in it will crash).

Having looked into this, I believe this is due to how windows handles references to ICatPortBindingProxy. The issue occurs in getICATProxy() (specifically how the SSLContext is set on the proxy).

To fix this, I will instead modify the method to take an ICatPortBindingProxy reference and set the soap end-point and SSLContext for it.

[Jay Rainey]

Set properties for a given ICATProxy. Refs #8607.

Changeset: 9cc05743f49bf6d82855e9e85d9c9a4c1e17f3f9

[Jay Rainey]

Set properties for a given ICAT3Proxy. Refs #8607.

Changeset: 1db0923974c886f86b0260c5efea3ab7f1a58e8b

[Jay Rainey]

Merge branch 'feature/8607_icat_stests_windows' into develop. Refs #8607.

Conflicts:

• Code/Mantid/Framework/ICat/inc/MantidICat/ICat4/ICat4Catalog.h

Changeset: a0c3de8dedb68248853c8a3065d3b417e4c3b4f0

[Jay Rainey]

Note: This must be tested on Windows 7 in debug mode.

To test

1. Ensure that no ​unit tests fail. Especially ​clean_win7_develop_db.
2. Log into Mantid and perform searches/downloads etc. No crash should occur.

[Peter Parker]

Jay, I'll pass this since the unit tests are working now.

As we discussed the problem is related to the default copy constructors of ICatX::ICATPortBindingProxy, which you'll note are never needed in the code now you've made your changes. Before the changes, the copy constructors were being used, but only in Windows Debug mode. This is because the ​Named Return Value optimization is performed by our non-MSVC compilers, but not by the ​MSVC compiler in Debug mode.

I've done a bit of digging and this ​gSOAP FAQ states that actually the parent soap struct should never be passed around by value:

Make sure you don't pass the gSOAP run-time environment (struct soap) by value to your application functions. Always pass struct soap by reference or pointer, just like the internal gSOAP functions do. In case you need to make a copy of it, use soap_copy().

It seems to me there are two potential ways to deal with this so that it won't cause any more problems in the future:

1. Define our own copy constructors for the inherited classes of soap, but make sure we use soap_copy() internally. This way we'll be able to pass our soap objects by value.

2. Disable the copy constructor altogether by declaring one as private and then not providing an implementation. (This is the standard idiom for doing this in C++, you may have seen it elsewhere in Mantid.) This means we'll not be able to pass by value, but at least the compiler will complain and tell us why, which is a lot better than being presented with a seemingly unrelated error at runtime.

I don't really know much about gSOAP so I've cc'ed Martyn into the ticket in case he wants to comment on which method (if either) we should go with.

[Peter Parker]

Merge remote-tracking branch 'origin/feature/8607_icat_stests_windows'

Full changeset: ​335e05c62ff492d837b5b78d48238be52173f69b

[Peter Parker]

I've just realised, the ICATPortBindingProxy classes are auto-generated. So not sure if we can do anything about this now, except maybe to derive our own MantidICATPortBindingProxy class?

[Martyn Gigg]

Peter, nice work on finding the issues here.

Given it is auto-generated code I think this is something we just have to be aware of when using gsoap. If we start implementing our own wrappers then it will become more of a maintenance burden that just making sure we use the gsoap as it is intended to be used.

[Jay Rainey]

Merge branch 'feature/8607_icat_stests_windows' into develop. Refs #8607.

Conflicts:

Code/Mantid/Framework/ICat/inc/MantidICat/ICat4/ICat4Catalog.h

Changeset: a0c3de8dedb68248853c8a3065d3b417e4c3b4f0

[Stuart Campbell]

This ticket has been transferred to github issue ​9451