icat external download file creation

[Jay Rainey]

If the user attempts to download a file from the catalog and does not have access to the archives it's downloaded over HTTPS.

However, if an error occurs then the content of this is written to the name of the file the user tried to download.

This can be mis-leading and lead to problems. Especially if the user presses "Load" as the file is downloaded and then attempted to load into Mantid, which will not work.

As such, we should never create a file nor write to it if an error occurs on the IDS. Instead, we must output a valid error message to the user. (The contents of which were previously written to a file).

[Jay Rainey]

Moved IDSError method to algorithm helper. Refs #8727.

• As I will use the exact same logic when checking for an IDS error in CatalogDownloadDataFiles.
• Updated IDSError method signature and logic to be more robust and usable across algorithms.

Changeset: 927cda45cfea4173ee414d3c3c744a4b7f3d67a3

[Jay Rainey]

Added IDS error handling to catalog download algorithm. Refs #8727.

• The file being downloaded is no longer saved to disk if an error occurs.

Changeset: 83e5faf4a9e9039fa2b4fa08dc9d15da78d2b49e

[Jay Rainey]

Handle exception if file being loaded does not exist. Refs #8727.

• As the "Load" button involves downloading and loading the file from the archives I have had to return early in the load method if a path to the file does not exist (e.g. it was not written to disk as an error has occurred).

Changeset: 635e267e14db1af06b0a3b40bee8ddf8ee7553b1

[Jay Rainey]

Note: you must not have access to the archives in order to test this ticket.

Problem

A file was being created (the one the user wanted to download) even IF an error occurred on the IDS. Moreover, the error returned from the IDS was written to the aforementioned file, and no user-friendly error was output to the user.

Forcing failure

As it's difficult to force the IDS to return an error you will have to force one yourself, such as a 404. To do this:

Obtaining a valid URL

1. Open Mantid and set the Log level to Debug.
2. Log into the catalog and click Search.
3. Perform a search and the investigation table will be populated.
4. Double click an investigation to select it and the datafiles table will be populated.
5. Select a datafile to download. Take note (copy) the URL that is output into debug.

Invalidating the URL (force 404)

As you now have a valid URL that you must make invalid! To do this:

1. Find ICat4Catalog.cpp.
2. Find the method getDownloadUrl and change the value being assigned to url to the URL you previous copied.
3. Update the URL you just pasted by appending more numbers to datafileIds=??? where ??? is the id of the datafile.

Every download you perform on the archives will now attempt to use this URL. As such, you should now receive an error message from the IDS.

Testing

1. As you have modified the file you will need to rebuild Mantid.
2. Log into the catalog and perform a search. The investigation table will be populated.
3. Double click an investigation to select it and the datafiles table will be populated.
4. Select a datafile to download and click Download to...
5. You should now receive a better error from the IDS and no files should be written to disk (since an error occurred).

[Peter Parker]

Two small things we could improve in the code:

• Populating a set using tmp, tmp + sizeof(tmp) / sizeof(tmp[0]) is quite messy, and error-prone. We've used ​boost::assign::list_of elsewhere and it works quite well, at least until we can get C++11 initialiser lists supported on all platforms.

• Rather than converting HTTP Response codes to strings, just keep them in their original HTTPStatus form. It just makes things a little simpler, and is something extra the compiler can check for us.

[Jay Rainey]

Use HTTPStatus enum types and not strings. Refs #8727.

Changeset: 448a32c875408b5b26355ac69106c4475e13a648

[Jay Rainey]

Update algorithms to use HTTPResponse status. Refs #8727.

Changeset: 002b320a593f2f24917134cc361ba60597f4e5ed

[Jay Rainey]

I refactored the code as suggested by Peter in comment:8. The functionality should be tested again to ensure these changes are correct.

[Arturs Bekasovs]

The code seems to be clear and the changes are appropriate. The code re-factoring was very useful - converting enums to strings for comparison wasn't neat.

Reproduced the problem on the current master.

The changes have fixed the problem - I get an error message when trying to download a file with purposefully tweaked URL. The file doesn't get created at all in that case. Files are downloaded/loaded fine when URL is not touched.

[Arturs Bekasovs]

Merge remote-tracking branch 'origin/feature/8727_icat_external_dl_file_creation'

Full changeset: ​44c3ccc11795a4bd7c05e7955087fb56755d4416

[Stuart Campbell]

This ticket has been transferred to github issue ​9571