#1612 - Add or Replace File via API

[raprasad]

RFI Checklist

Before submitting the pull request, fill out sections (1.) Related Issues and (2.) Pull Request Checklist.

1. Related Issues

#1612 Allow file uploads for non-zip files through the API

---

2. Pull Request Checklist

• Functionality completed as described in FRD
• Dependencies, risks, assumptions in FRD addressed
• Unit tests completed
• REST Assured tests added or modified: DatasetsIT, FilesIT
• Deployment requirements identified (e.g., SQL scripts, indexing)
• Documentation completed
• All code checkins completed

---

3. Review Checklist

After the pull request has been submitted, fill out this section.

• Code review completed or waived
• Testing requirements completed
• Usability testing completed or waived
• Support testing completed or waived
• Merged with develop branch and resolved conflicts

Connects to #1612

[pdurbin]

@raprasad @djbrooke for supporting the deletion of files via native API I just opened #3419.

[raprasad]

@pdurbin : adding an API for to add descriptions/tags to existing files is low-hanging fruit but am avoiding scope creep for now. (literally thought through the code line by line but wrote nothing and have now blanked it out:)

[raprasad]

fyi: found a bug with ADD.

REPLACE is retricted to a single file--even if a single '.zip' is "unzipped into multiple files.
ADD should not have this restriction.

Fix:

• Allow add of single file (e.g. zip) to result in multiple files in dataset
• Return appropriate JSON listing all files returned

[coveralls]

Coverage increased (+0.02%) to 8.618% when pulling 78f3267 on 1612-api-add-file into 8ad3431 on develop.

[coveralls]

Coverage increased (+0.02%) to 8.618% when pulling b2dafc6 on 1612-api-add-file into 8ad3431 on develop.

[coveralls]

Coverage increased (+0.01%) to 8.616% when pulling 078fbd4 on 1612-api-add-file into 8ad3431 on develop.

[pdurbin]

I'm wondering if it's possible to stick to the Java EE Standard (JAX-RS).

[pdurbin]

@scolapasta are we consciously switching from a JAVA EE standard (JAX-RS) to an specific implementation (Jersey)?

[raprasad]

Post code review checklist

• For file add/replace remove ability to add DataFile tags
  • Leave "logic" in place to use for in API: Allow changing label, description and tags of an existing file #3422 - changing label, description and tags of an existing file
    • For existing files, we can validate whether the file is tabular and can have a DataFile tag
• Use native JSON. e.g. Add new attributes to the native JSON, exclude dataset and dataverse information
  • Delete okResponseGsonObject method from AbstractApiBean and use the Java EE standard (javax.json) instead. There is a proposed fix by @pdurbin in pull request API consistency: OK, response only has "data", remove okResponseGsonObject #3433.
    • Add tags, categories, filesize, released, restricted to API 6cfc036
    • This is still a new method which take both a JsonObjectBuilder object and a String message
  • This inadvertently addresses Native API doesn't show Categories or File Tags (at least for "FITS file" and "Image file") #3067
• For add file API endpoint, we now have the dataset object--pass directly to AddReplaceHelper instead of the id
  • Redo business logic
  • Redo tests
• Tech debt: Update existing findDatasetOrDie method to use error messages in bundles
  • Rename existing bundle errors
  • Update add/replace business logic to use renamed bundle errors
  • Update API tests to use renamed bundle errors
• Move removeDuplicatesNullsEmptyStrings to a utility class
  • Which one?
• For OptionalFileParams, rename tags to categories in code/methods
  • Rename business logic methods
  • Update tests
  • There's an inconsistency where the UI/user facing logic has the word tags but internal code uses categories
    • Group decision to keep categories in the code
• AddReplaceHelper logic
  • Consolidate calls to UpdateDatasetCommand in step_070_run_update_dataset_command and step_080_run_update_dataset_command_for_replace
• OptionalFileParams.java. addCategoryByName already checks for existing categories, remove it from this class
• leftover code in API endpoint where testFileInputStream used instead of fileInputStream
• Use code coverage reports to see how much of the new code is being exercised by unit tests and attempt to raise the amount of code coverage of new code, if possible. See Set testing expectations in Developer Guide and document tools (Jacoco, Coveralls, REST Assured) #3431 for details on how to measure code coverage.
• Favor using the pattern of checking if a command can be executed rather than checking a specific permission. Here's an example from StatementManagerImpl.java for checking if a command can be executed: if (!permissionService.isUserAllowedOn(user, new GetDraftDatasetVersionCommand(dvReq, dataset), dataset))
• Rework addFileToDataset in Datasets.java to use command pattern and look more like the createPrivateUrl method in that class? Just like how there’s a CreatePrivateUrlCommand we could add a AddFileToDatasetCommand. This might help AddReplaceFileHelper.java get smaller and make the code generally more testable.
• Add REST Assured test for to add a tiny file (using the new native add, of course) to try to reproduce File Upload: Uploading a tiny text file causes a bufferunderflow log error but works. #1719.
• Add REST Assured test to see if the message seen when trying to upload a duplicate file matches what is shown in the GUI. Hopefully this means we will be able to put File Upload: Improve message when user tries to upload duplicate file. #2467 through QA.
• File add via native API must enforced the :MaxFileUploadSizeInBytes limit that is enforced by the GUI and by SWORD. This doc should be updated as well: http://guides.dataverse.org/en/4.5.1/installation/config.html#maxfileuploadsizeinbytes

---

Include in this ticket?

• Rename the ingestService.addFiles(..) method to ingestService.saveFiles(..)
  • retest: DatasetPage UI, EditDataFiles UI, and MediaResourceManagerImpl.java

Important Prov. note related #3238

• The add/replace API endpoints currently have a "pathway" to add additional JSON variables. e.g. If you're adding string, numeric, boolean or other JSON based data
• However, the API endpoints will need the ability to add a second file--a prov file.

[raprasad]

This is frozen. Dev has switched to https://github.com/IQSS/dataverse/tree/2290-file-replace