Guestbook - Downloads via API are not counted

[raprasad]

originally part of #3324

---

Update: Downloads via the API are not counted in the GuestBookResponses -- e.g. don't seem to be counted at all.

See this file: Access.java and this method--and add a new GuestBookResponse object to it:

@Path("datafile/{fileId}")
    @GET
    @Produces({ "application/xml" })
    public DownloadInstance datafile(@PathParam("fileId") Long fileId, @QueryParam("key") String apiToken, @Context UriInfo uriInfo, @Context HttpHeaders headers, @Context HttpServletResponse response) /*throws NotFoundException, ServiceUnavailableException, PermissionDeniedException, AuthorizationRequiredException*/ {                

[sekmiller]

This has been addressed in conjunction with the file landing page redesign of the download processing. Guestbook records will be created when the user invokes a download via the API.

[djbrooke]

Closing this. Will leave a note in #2465.

[pdurbin]

The non-counting of downloads via API has been noted in #2911.

[pdurbin]

The comment by @sekmiller makes it sound like this was addressed in 4.6 so I'll add that milestone to this issue.

[kcondon]

Reopen per Danny and testing shows it still exists.

[shlake]

When a user uses the Download URL, this type of download is not being counted.

Does "downloadtype" include counts from non-dataverse-clicking - like a person entering the DownloadURL in a browser window and "clicking go" from the browser.

Because what I see right now, if I copy/paste the download URL, view that URL in a browser window, that "download" is not counted.

[landreev]

From a quick look, unfortunately this may have been broken for more than a year. It looks like in the fall of 2016 we rearranged how the guestbook records are written by the API. The actual writing was moved from Access.java into DownloadInstanceWriter.java; in order to write the record after the download is complete, not before; to ensure that only the successful downloads are counted.
However, in Nov. 2016 it looks like these lines in Access.java were commented out:

/*
        if (gbrecs == null && df.isReleased()){
            //commenting out for 4.6 SEK
           // gbr = guestbookResponseService.initDefaultGuestbookResponse(df.getOwner(), df, session);
        }
        */

this means gbr stays null; but then further down we have:

if (gbr != null){
            downloadInstance.setGbr(gbr);
            downloadInstance.setDataverseRequestService(dvRequestService);
            downloadInstance.setCommand(engineSvc);
        }

And because of this, since the gbr element in the DownloadInstance is null, the following code in DownloadInstanceWriter.java never gets executed:

if (di.getGbr() != null && !(isThumbnailDownload(di) || isPreprocessedMetadataDownload(di))) {
                        try {
                            logger.fine("writing guestbook response.");
                            Command<?> cmd = new CreateGuestbookResponseCommand(di.getDataverseRequestService().getDataverseRequest(), di.getGbr(), di.getGbr().getDataFile().getOwner());
                            di.getCommand().submit(cmd);
                        } catch (CommandException e) {
                            //if an error occurs here then download won't happen no need for response recs...
                        }
                    } else {
                        logger.fine("not writing guestbook response");
                    } 

(and even if it were executed, because downloadInstance.setCommand(...) was never done, the command could not be executed either...)
Hmmm... Once again, that was a very quick look, so I may be missing something.
But what's particularly annoying is that the last message on the issue is Kevin saying back in Feb. that yes, it is still broken.

[djbrooke]

Thanks for the summary @landreev, we'll pick it up when someone has capacity.

[landreev]

It may be an honest 1 - just a matter of uncommenting/changing a couple of lines of code in the 2 class files.
Either way, what I entered should be enough to estimate, and for somebody to fix it, when capacity presents itself.

[mheppler]

Related #3572

[sekmiller]

Fixed and checked in the getting user key info into the default record. Also assigned to the gb selected at dataset level, but doesn't add answers for custom questions or enforce required fields.

[sekmiller]

@pdurbin and @pameyer thanks for your research on the integration test. Since I'm already in the code there I'll take care of it.

[pdurbin]

I just pulled the latest (d30aeca) and all seven tests in SwordIT are passing now.

[sekmiller]

Fixed the multiple download issue, so all of the items noted by Kevin above have been addressed.

[landreev]

@sekmiller - I was about to offer to help with this issue, but it looks like you had already fixed it all! Sorry I didn't get to it sooner.

[kcondon]

OK, all reported issues above were fixed, including download bundle. Did find one new issue: clicking on Two Ravens explore increments count by 2 and responses shows 1 TwoRavens and 1 download.

[landreev]

I have moved the issue back into QA.
The fixes for counting zipped downloads of multiple files and "download bundles" look great.
As for the issue of the extra count coming from the data file download coming from TwoRavens, this is addressed on the TwoRavens side - and does not require any code changes in the Dataverse.

OK, let's merge it... since all these downloads that are killing the prod. servers are not getting counted!

[kcondon]

OK 2 ravens was patched in production and we agreed to supply that patch in the release notes with instructions on how to apply it.