Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overload method void uploadFile(String doi, File file); to void uploadFile(String doi, InputStream is, String filename); #13

Merged
merged 4 commits into from
Nov 17, 2022

Conversation

AleixMT
Copy link
Contributor

@AleixMT AleixMT commented Nov 15, 2022

Finally here is my PR to solve #12

@AleixMT
Copy link
Contributor Author

AleixMT commented Nov 15, 2022

I must say that I tried all the tests before and after my modifications and I found that 3 tests fail, before and after my modifications, so I deduced that these are not my fault. I tried against this dataverse instance

*
* @param doi Identifier of the dataset that we are sending the data to.
* @param file Stream of data to the contents of the file to upload.
* @param filename Name of the file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the input stream doesn't come from a file, but instead from a network or whatever, then the 'file' terminology isn't quite accurate. I think 'filename' is the name you want it to appear as in Dataverse, is that correct?

@@ -224,6 +225,23 @@ public void uploadFile (String doi, File file) {
}
}

@Override
public void uploadFile(String doi, InputStream file, String filename) {
FileUploader uploader = new FileUploader();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could uploadFile(string, file) call

uploadFile(doi, new FileInputStream(file), file.getName())

in order to reduce duplication?

Copy link
Contributor

@otter606 otter606 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'll try to add a basic Github action to run some tests and will get this merged. Thank you!

…d to previous changes in this PR: Changed name of variable and changed javadoc from uploadFile method, delegated implementation of one of the uploadFile implementation to the other to avoid duplication, split line of deposit for being too long
@richarda23 richarda23 merged commit 6374773 into IQSS:master Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants