-
Notifications
You must be signed in to change notification settings - Fork 156
Verify that Briefcase and Collect use the same file extensions and mime types #33
Comments
Hi Yanokwa. I am a newbie looking to get involved with a good project. |
Hi @danielsteward! This is a great issue to start on because it mostly requires you to go looking through the code. Give it a try, and if you get stuck, ask in the #briefcase-code channel on http://slack.opendatakit.org! |
Welcome Daniel,
I also think this is good to look into. I have been reverse engineering a
collect and briefcase support without Aggregate using only the wire
protocol. One thing I know for certain is that MIME support is uneven
across these tools and doing normal things like attaching PNGs rather than
JPEGs has odd side effects.
Even if you only stopped after getting a good understanding of the issues
involved and wrote up good issues for the various defects it would be a
huge win.
Even better if you want to tackle fixing the behavior with pull requests as
well.
Thank you, and let us know if you need any assistance or moral support! :D
Brent
…On Thu, Jun 1, 2017 at 5:26 AM, Yaw Anokwa ***@***.***> wrote:
Hi @danielsteward <https://github.com/danielsteward>! This is a great
issue to start on because it mostly requires you to go looking through the
code. Give it a try, and if you get stuck, ask in the #briefcase-code
channel on http://slack.opendatakit.org!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#33 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAdPFcXunTQtvWwvcur1RuMnwjfwo4lsks5r_j1ngaJpZM4LKfS4>
.
|
Great. Let me look at it and I'm sure I'll have some queries.........and hopefully answers for you. |
Hi Team. I have the software working and have set everything up and have used it a little. I am now looking through the code and trying to replicate the problem that the issue was inspired by: #5. Anything springs to mind please let me know otherwise no need to answer this post. |
Hi Team. It's hard to replicate the original issue as Collect converts any image to a jpeg file type whatever the original. Is this right or am I making a mistake? |
@danielsteward I have noticed that behavior! To be honest with you, I'm not sure whether or not it's intentional. I have a vague memory of quickly looking into it and seeing that pngs were just renamed to have .jpg extensions but of course I didn't write it down anywhere. It would be a good thing to understand better if you're up for identifying where that happens in Collect, whether there are any comments around it and/or any helpful commit messages. You'll probably want to file an issue on the Collect side with your findings. |
@lognaturel's suggestion is a good one. And if you want to continue with Briefcase, that would be a good path to follow as well! It would seem that the file types that Briefcase uploads are here: https://github.com/opendatakit/briefcase/blob/HEAD/src/org/opendatakit/briefcase/util/AggregateUtils.java#L693, but given the sometimes unexpected behavior of ODK tools, it'd be good to confirm that the uploads actually happen for those file types. Once we know that, then we can compare with what we find on the Collect side and document both. |
Daniel, I think you are seeing what I experienced during my own testing. I used the Birds sample form to attach a PNG image I had stored on the device and was able to submit it (using Collect). However, it was submitted (in my case, to a custom endpoint) as a file with a jpg file extension. This is one, but perhaps not the only, oddities of current multimedia file handling. |
Right, thanks for those replies. I'll crack on........ |
Hi Team. Sorry for my slow speed here. Have a divorce on my hands and it's getting tricky ;<.
So I am now looking at the classes that use this parser which seems to be the servlets. |
The archaeology of the code is part of the challenge of this project, so take the time you need, @danielsteward! If it becomes a blocker we'll let you know. I just talked with @lognaturel and she said she believes that the problem is likely before the submission hits Aggregate. That is, when she was working on Google Drive support, she noticed that her Android device saves images as .png, but Collect renames it to .jpg and just sends that. I've searched from ".jpg" in the Collect code and it looks like that's is indeed the behavior. So I'd start there because that's a bug! File an issue first with that bug, then suggest an approach to fix it. |
Great, that sounds like a good first step. I'm nervous about interacting with Git and screwing things up but filing the bug in Collect and then sorting it sounds like a good first step. |
What is a blocker? |
Sorry about the terrible delay, @danielsteward. A blocker is something that prevents work (e.g., the next release) on Briefcase from proceeding. Blockers are often just bugs, but sometimes a work is blocked because of cookie licking. Lots of fun lingo! |
I don't know if this issue is still relevant. It had some activity from Dec'16 to Jul'17 and then it stopped. |
I think this is still relevant because we have three tools (Aggregate, Collect, Briefcase) that have a separate whitelist of extensions to mime type mappings. I'm confident that these lists will eventually fall out of sync and that seems fragile. Perhaps we can use a library for this? |
OK! Let's analyze this. The table we need to maintain is this:
These mappings must be the same in Collect, Briefcase, and Aggregate. I believe that adding a third-party library would let us replace the magic strings for the mime types with some static constants, but we would still need to ensure the mappings are the same, which can still go out of sync:
The insight here is that the problem that needs to be solved is the mapping, not the mime type strings, right? I think that we could either:
|
Just to bring this topic back from the dead. I think the initial reason is that Aggregate doesn't know how to display files if there are no MIME types, but I'm not sure why we need to keep a list of the various MIME types. That is, why can't we just use some library that has an extension to MIME mapping. On Collect, we can use https://developer.android.com/reference/android/webkit/MimeTypeMap. And Aggregate/Briefcase could use https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#probeContentType%28java.nio.file.Path%29 (which according to https://stackoverflow.com/a/8973468/152938 isn't perfect, but prob better than what we have). |
Thanks for the links, @yanokwa! Unfortunately, Java8 removed In Java8u181 (Oracle) we can match only xml and jpg, and nobody can assure that changing the JRE will maintain the same entries on the table. Users can extend the table by running our JAR with a |
When uploading a binary file, Briefcase and Collect use file extensions and mime types of a fixed list fo files. Confirm that this list is the same in Briefcase, Collect, and Aggregate. Aggregate's list should be considered the master list.
This issue was inspired by #5.
The text was updated successfully, but these errors were encountered: