Forms always show as updated when manifest's meda files are missing hash prefix

[seadowg]

A test was added to describe this behavior in BadServerTest as part of #4911:

@Test
public void whenMediaFileHasMissingPrefix_showsAsUpdated() {
    testDependencies.server.removeMediaFileHashPrefix();
    testDependencies.server.addForm("One Question", "one_question", "1", "one-question.xml", Arrays.asList("fruits.csv"));

    rule.withProject(testDependencies.server.getURL())
            .copyForm("one-question.xml", Arrays.asList("fruits.csv"), testDependencies.server.getHostName())
            .clickGetBlankForm()
            .assertText(R.string.newer_version_of_a_form_info);
}

We should discuss what behaviour we actually want in this case (and for null hashes etc). As part of whatever direction we go in, we should backfill unit tests for OpenRosaResponseParserImpl to both document all these kinds of edge cases and make sure we have coverage of them.

[seadowg]

cc @grzesiek2010 @lognaturel

[grzesiek2010]

I think if there is null, empty or invalid hash (without prefix) we should ignore such a file. It's not our fault so there is no reason to be overprotective.

[seadowg]

I think if there is null, empty or invalid hash (without prefix) we should ignore such a file. It's not our fault so there is no reason to be overprotective.

Definitely agree we should ignore the file. I guess I'm wondering if we should refuse to download the form and show an error? Right now, in cases where the manifest has invalid items we download the form "successfully" (and report that to the user) which could leave them in a weird state where the form behaves strangely. It seems inconsistent seeing as we will show an error (and back out of downloading the form) if downloading a media file fails.

[grzesiek2010]

if we should refuse to download the form

download a form for the first time or its updated version because it's a difference. If you download a form for the first time you don't really need that hash in media files so maybe it would be strange to block this case even though it's not our fault.

Maybe we should consider adding warnings (not only errors) like for example https://getodk.org/xlsform/ does during converting a form. So in case like that we could display a warning that there was something wrong with hashes but not block downloading.

[seadowg]

download a form for the first time or its updated version because it's a difference. If you download a form for the first time you don't really need that hash in media files so maybe it would be strange to block this case even though it's not our fault.

Agreed! Right now we wouldn't download media files with null hashes the first time or on update (or signal an update based on them updating). Just to summarise, at the moment:

• Manifest including media file with null filename, hash or download url: ignored, form downloaded/updated without media files
• Manifest including media file with invalid hash: treated as valid, form downloaded with media files and updates end up wonky due to hash comparisons
• Manifest fails to download: whole form fails to download
• Manifest is not valid XML: whole form fails to download

I feel like an invalid hash should be equivalent to null hash. The extra mile would be to provide an error/warning, but it feels like that might be too much work to bother with for a case only caused by a bad server. I'm thinking a better catch-all might be to just fail downloads for all these cases (like we do with failed manifest downloads or invalid XML).

[seadowg]

The problems this causes will be fairly well mitigated by #5440 and #5439. We're also looking to relax the Open Rosa spec around media file hashes to allow for cases where an MD5 file hash might not be something the server can generate.