Implement GET DownloadStatus endpoint #52

[patrick-austin]

Should be independent of other changes.

• Add endpoint in UserResource to check Downloads status by id.
• Add function in DownloadRepository for the above.

Closes #52
Closes #53

[kevinphippsstfc]

I think this section creating the comma separated list can be replaced with:
String commaSepString = String.join(",", downloadIds);
(I think there is a similar section in another PR which I didn't comment on so I need to go back and try to find that)

[patrick-austin]

String.join() requires the second argument to be a CharSequence or Iterable<? extends CharSequence> - but we have a List<Long>, and Long doesn't extend CharSequence. To use String.join I think we'd need to first iterate over every id, get the String value, then join (unless there's a neater/more efficient way of casting):

		List<String> stringDownloadIds = new ArrayList<>();
		downloadIds.forEach(downloadId -> {
			stringDownloadIds.add(downloadId.toString());
		});
		String joinedDownloadIds = String.join(",", stringDownloadIds);

I guess it's 2 lines shorter (so maybe looks nicer?) but I can only assume it's less efficient because we're looping twice . Let me know if you still prefer it and I can make the change (or not).

[kevinphippsstfc]

I think I prefer the shorter version as it's much simpler despite the slight inefficiency of looping twice.
It could be simplified even further using a standard for-each loop:

List<String> stringDownloadIds = new ArrayList<>(); 
for (Long downloadId : downloadIds) { 
    stringDownloadIds.add(downloadId.toString()); 
} 
String joinedDownloadIds = String.join(",", stringDownloadIds);

which gets rid of the (I find) rather awkward syntax of Lambda expressions.

[patrick-austin]

Sure, I can't see the inefficiency actually mattering/bottlenecking here, so happy to go for whatever is clearest to someone else reading the code

(I do actually like lambdas though... 😬 )

[kevinphippsstfc]

As with other PRs, can the facility name be made optional?

[patrick-austin]

I think this one should be optional once it's merged onto the changes from #47 - it's using a QueryParam so can be left out, and the update FacilityMap functions will handle null values by returning the configured default.

[kevinphippsstfc]

I feel like as the download IDs is a list, the test ought to have more than one item in it.
And if the status were set different on those Downloads then it would also be possible to check that the statuses are returned in an order matching the request.
So that the code doesn't get too repetitive with creating the downloads, it might be nice to make use of the createDownload method that you created in one of the other PRs.

[patrick-austin]

Yes it would ensure it properly tests the String joining stuff above as well. Will refactor the dummyDownload functions from StatusCheckTest to TestHelpers so we can use them here as well.