tsv ingest fix for directupload#7124
Merged
kcondon merged 8 commits intoIQSS:developfrom Aug 12, 2020
Merged
Conversation
sekmiller
reviewed
Aug 7, 2020
| logger.fine("Determined type: " + finalType); | ||
| } else { | ||
| //Remote file, trust supplier | ||
| finalType = suppliedContentType; |
Contributor
There was a problem hiding this comment.
Do we care if the suppliedContentType is blank or null?
Member
Author
There was a problem hiding this comment.
That should already be being caught (e.g. in the EditDatafilesPage.handleExternalUpload method) but worth a safety check here probably. And we shouldn't replace the default with a null/blank response if determineFileTypeByExtension returns null/blank.
Contributor
There was a problem hiding this comment.
If you make that change, I'll put this into QA. Thanks!
sekmiller
approved these changes
Aug 7, 2020
…_with_directupload
as in normal upload. This will replace generic types as in this issue as well as replace text/tab-separated-value mimetypes so that tsv files get ingested and will stop the browser supplied mimetypes for r, stata, etc. in the same way that normal upload does.
qqmyers
commented
Aug 12, 2020
|
|
||
|
|
||
| private static File saveInputStreamInTempFile(InputStream inputStream, Long fileSizeLimit) | ||
| private static boolean useRecognizedType(String suppliedContentType, String recognizedType) { |
Member
Author
There was a problem hiding this comment.
just moved and made to return a boolean, no logic changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it: enables ingest of .tsv files (and potentially other types) with direct upload when the browser does not supply a mimetype. The changes here invoke code to determine the mimetype from the extension and then update the mapping of tsv files to the currently required text/tsv mimetype.
Which issue(s) this PR closes:
Closes #7122
Special notes for your reviewer:
Suggestions on how to test this: setup for direct upload and try uploading a small tsv - it should ingest to produce a .tab file. Also turn on fine logging for the FileUtil class and look for "Determined type: text/tsv". - If you see this, it indicates that you are in the test case where your browser doesn't supply a mimetype for .tsv files. If you don't see this, you may need to remove any mapping for the .tsv extension from your test browser/machine (or I can slack/demo against the test server from my browser that doesn't provide a mimetype).
Does this PR introduce a user interface change? If mockups are available, please link/include them here: no
Is there a release notes update needed for this change?: no
Additional documentation: