Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 44 additions & 16 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ what it replaced, and the second round of tapping is always the expensive one.

**Document Processing Pipeline:**
- `CoreLoader` - Primary document processor using the native C++ ODR core library
- `RawLoader` - Plain text and other raw file processor
- `RawLoader` - The three files odrcore does not render for us: csv, svg and xml
- `OnlineLoader` - Remote document fetcher
- `MetadataLoader` - Document metadata extractor

Expand Down Expand Up @@ -159,9 +159,17 @@ the `.pro` suffix) differ on purpose - do not "fix" the mismatch:
### Supported file types come from odrcore, and a test keeps the manifest in step

`SupportedDocumentTypes` used to be a hand written table of mime *prefixes*. Since odrcore
6.1 it is derived: `Odr.allFileTypes()` filtered by `Odr.capabilitiesByFileType(...)
.translateHtml` and `Odr.fileCategoryByFileType(...) == DOCUMENT` is what `CoreLoader`
renders, and `Odr.mimetypesByFileType` / `Odr.fileExtensionsByFileType` expand each of those
6.1 it is derived, and it answers two separate questions:

- **what `CoreLoader` renders** (`CORE_FILE_TYPES`): `Odr.allFileTypes()` filtered by
`Odr.capabilitiesByFileType(...).translateHtml`, minus csv. Since 6.2 that covers text,
images, zip and cfb, fonts, audio and video - which is why `RawLoader` lost its viewers.
- **what the app claims** (`CLAIMED_FILE_TYPES`): the same filter narrowed to
`Odr.fileCategoryByFileType(...) == DOCUMENT`, plus text, csv and zip. The manifest mirrors
this. Keep it narrow - the app plays an mp3 handed to it but does not want it in the share
sheet.

`Odr.mimetypesByFileType` / `Odr.fileExtensionsByFileType` expand each of those
into every spelling the core accepts - the templates, the macro-enabled variants, the
`application/x-vnd.oasis...` family and the `-flat-xml` ones. Do not put a list of mime
prefixes back; the whole point is that the app cannot claim a format the core does not have,
Expand All @@ -170,9 +178,8 @@ or miss one it does. A prefix match is also what made the app claim `.xlsb`
and then fail to open it - the core gives it a file type of its own with an empty capability
row now.

Two declarations are left. The app's own choice of which of the core's formats go to
`CoreLoader` and which to `RawLoader` - text, csv, zip and `image/` are named in
`SupportedDocumentTypes`, because that is an app decision the core knows nothing about. And
Two declarations are left. The app's own choice of what to *claim*, named in
`SupportedDocumentTypes` as `CLAIMED_FILE_TYPES` because the core knows nothing about it. And
the `STRICT_CATCH` `activity-alias` in `AndroidManifest.xml`, which cannot be collapsed into
the first because XML cannot read any of this. Its three intent-filters are *generated* from
the same table (an intent-filter matches a mime type exactly, so all 40 spellings and 41
Expand All @@ -182,18 +189,39 @@ that `SupportedDocumentTypes` and the package manager give the same answer, so a
upstream and forgotten in the manifest fails CI rather than shipping.

The tables live in `libodr_jni`, so anything that reads them needs a device. That is why
`CoreLoaderTest`, `SupportedDocumentTypesTest` and `OnlineLoaderTest` are instrumented tests
and not JVM ones - none of them opens a file, they just cannot ask the table from a plain
JVM. What the core decides *after* the file is in the cache is unchanged: `MetadataLoader`
runs libmagic over the copy, and `CoreLoader.isDocumentEditable` asks the opened document.
`CoreLoaderTest`, `SupportedDocumentTypesTest`, `RawLoaderTest` and `OnlineLoaderTest` are
instrumented tests and not JVM ones - none of them opens a file, they just cannot ask the
table from a plain JVM. What the core decides *after* the file is in the cache is unchanged:
`MetadataLoader` runs libmagic over the copy, and `CoreLoader.isDocumentEditable` asks the
opened document.

One consequence of claiming every spelling: `MetadataLoader` puts whatever mime type it ended
up with through `SupportedDocumentTypes.canonicalMimeType`, so the loaders behind it see one
per format. `CoreLoader` matches the whole set and does not care, but `RawLoader` routes by
mime type *prefix* - a provider volunteering `application/x-zip-compressed` or
`application/csv` would be offered the app and then told the file is unsupported.
`SupportedFormatsTest.everythingTheAppClaimsIsLoadedBySomebody` is what holds that: every mime
type the app claims has to reach a loader that takes it.
per format. Both loaders match whole sets rather than prefixes now, but a provider
volunteering `application/x-zip-compressed` or `application/csv` still has to reach one of
them. `SupportedFormatsTest.everythingTheAppClaimsIsLoadedBySomebody` is what holds that:
every mime type the app claims has to reach a loader that takes it.

One catch when reading the table directly, as `isDocument` does: the core matches mime types
*exactly* and spells some with capitals (`macroEnabled`), so do not `lowercase()` first. The
lookups against our own sets are the other way round - `mimeTypesOf` lowercases what it stores.

### `RawLoader` is asked before `CoreLoader`, not after it

It used to be the fallback at the end of the chain, reached only when the core threw. Once
odrcore learned to render text, images, zip and media the core stopped throwing, and the
PhotoSwipe, Plyr, JSZip and csv-to-html-table viewers in `assets/` became unreachable without
anyone noticing. The first four are gone; the csv one was worth keeping.

So `LoaderService.onSuccess` asks `rawLoader.isSupported` *first*.
`SupportedDocumentTypes.isRenderedByRaw` is the whole list: csv (the only case where the
order matters, since `CORE_FILE_TYPES` excludes it), plus svg and xml, which have no odrcore
file type at all. A `RawLoader` failure falls through to the core; everything the core cannot
open goes to the upload offer rather than being renamed and handed to the WebView on spec.

Routing by name is guarded - see `nameSays`. The core identifies by content, so a `report.csv`
holding an odt is an odt, and only a file it did not recognize (or called plain text) may be
routed by its extension.

### Editability comes from the core, never from a mime type

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,33 @@ class CoreLoaderTest {
assertTrue(isSupported("application/x-pdf"))
}

/** What odrcore 6.2 renders beyond documents, each of which used to need a viewer in assets. */
@Test
fun whatTheCoreRendersBesidesDocumentsIsSupported() {
assertTrue(isSupported("text/plain"))
assertTrue(isSupported("image/png"))
// the image types 6.2 added
assertTrue(isSupported("image/webp"))
assertTrue(isSupported("image/heic"))
assertTrue(isSupported("image/avif"))
assertTrue(isSupported("application/zip"))
// rendered when handed one, never claimed in the share sheet
assertTrue(isSupported("audio/mpeg"))
assertTrue(isSupported("video/mp4"))
}

/** The one format left to RawLoader although the core would take it - its table viewer. */
@Test
fun whatRawLoaderShowsIsNotClaimed() {
// the core does render these, but RawLoader is what gives them their player or viewer
assertFalse(isSupported("text/plain"))
assertFalse(isSupported("text/csv"))
assertFalse(isSupported("image/png"))
assertFalse(isSupported("application/zip"))
assertFalse(isSupported("application/csv"))
}

/** No file type in the core at all, so nothing here could name them either. */
@Test
fun whatTheCoreHasNoFileTypeForIsNotClaimed() {
assertFalse(isSupported("image/svg+xml"))
assertFalse(isSupported("application/xml"))
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import org.junit.Test
import org.junit.runner.RunWith

/**
* Instrumented rather than a JVM test since odrcore 6.1: [OnlineLoader.isConvertible] falls through
* to [CoreLoader.isSupported], which reads the core's format table out of `libodr_jni` now instead
* of matching mime prefixes in kotlin. Nothing here uploads anything or touches the network.
* Instrumented since odrcore 6.1: [OnlineLoader.isConvertible] falls through to
* [SupportedDocumentTypes.isDocument], which reads the core's table in `libodr_jni`. Nothing here
* uploads anything or touches the network.
*/
@SmallTest
@RunWith(AndroidJUnit4::class)
Expand All @@ -20,7 +20,7 @@ class OnlineLoaderTest {

@Before
fun setUp() {
onlineLoader = OnlineLoader(null, CoreLoader(null))
onlineLoader = OnlineLoader(null)
}

private fun options(fileType: String): FileLoader.Options {
Expand Down Expand Up @@ -85,15 +85,20 @@ class OnlineLoaderTest {
"application/vnd.openxmlformats-officedocument.presentationml.presentation"
)
)
// delegated to CoreLoader
// whatever else the core files as a document
Assert.assertTrue(isConvertible("application/vnd.oasis.opendocument.text"))
// including what it cannot open - the converter runs libreoffice, not odrcore
Assert.assertTrue(isConvertible("application/vnd.ms-excel.sheet.binary.macroEnabled.12"))
}

/** The categories a converter cannot help with. */
@Test
fun handsEverythingElseToAThirdPartyViewer() {
Assert.assertFalse(isConvertible("text/plain"))
Assert.assertFalse(isConvertible("image/png"))
Assert.assertFalse(isConvertible("application/zip"))
Assert.assertFalse(isConvertible("audio/mpeg"))
Assert.assertFalse(isConvertible("video/mp4"))
Assert.assertFalse(isConvertible("application/vnd.apple.pages"))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
package app.opendocument.droid.background

import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import androidx.test.platform.app.InstrumentationRegistry
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.BeforeClass
import org.junit.Test
import org.junit.runner.RunWith

/**
* The three things [RawLoader] still has a viewer for, and the many it handed back to the core.
*
* Instrumented because the csv spellings come from odrcore's table in `libodr_jni`.
*/
@SmallTest
@RunWith(AndroidJUnit4::class)
class RawLoaderTest {

private lateinit var rawLoader: RawLoader

@Before
fun setUp() {
// no context: isSupported() is pure, and constructing a loader has no side effects
rawLoader = RawLoader(null)
}

private fun isSupported(fileType: String?, filename: String? = null): Boolean {
val options = FileLoader.Options()
options.fileType = fileType
options.filename = filename

return rawLoader.isSupported(options)
}

/**
* How svg and xml actually arrive: the core identifies by content and an svg *is* text, so
* `Odr.mimetype` says `text/plain` and only the name knows better.
*/
@Test
fun whatTheCoreMisreadsAsTextIsRoutedByItsName() {
assertTrue(isSupported("text/plain", "drawing.svg"))
assertTrue(isSupported("text/plain", "feed.xml"))
assertTrue(isSupported("text/plain", "rows.csv"))
// and a text file is still a text file
assertFalse(isSupported("text/plain", "readme.txt"))
assertFalse(isSupported("text/plain", "notes"))
}

/**
* The other half: the bytes win whenever the core recognized them. An odt called `report.csv`
* would otherwise reach the table viewer, succeed, and leave no fallback.
*/
@Test
fun aMisnamedDocumentIsStillADocument() {
assertFalse(isSupported("application/vnd.oasis.opendocument.text", "report.csv"))
assertFalse(isSupported("application/pdf", "report.csv"))
assertFalse(isSupported("application/zip", "archive.svg"))
assertFalse(isSupported("image/png", "scan.xml"))
// but nothing detected at all still leaves the name as the only thing to go on
assertTrue(isSupported("application/octet-stream", "rows.csv"))
assertTrue(isSupported(null, "drawing.svg"))
}

/** The core renders a csv line by line; `text-prefix.html` builds a table out of one. */
@Test
fun csvIsSupportedInEverySpelling() {
assertTrue(isSupported("text/csv"))
assertTrue(isSupported("application/csv"))
assertTrue(isSupported("text/comma-separated-values"))
}

/** Neither has a file type in odrcore, so nothing else would show them at all. */
@Test
fun whatTheCoreHasNoFileTypeForIsSupported() {
assertTrue(isSupported("image/svg+xml"))
assertTrue(isSupported("application/xml"))
assertTrue(isSupported("text/xml"))
}

/**
* Each had a viewer here until odrcore 6.2 learned to render it. Keeps them from coming back.
*/
@Test
fun whatTheCoreRendersIsLeftToIt() {
assertFalse(isSupported("text/plain"))
assertFalse(isSupported("image/png"))
assertFalse(isSupported("image/webp"))
assertFalse(isSupported("application/zip"))
assertFalse(isSupported("audio/mpeg"))
assertFalse(isSupported("video/mp4"))
assertFalse(isSupported("application/json"))
}

/** No rename-and-hope for these - the upload offer beats a blank page. */
@Test
fun whatNobodyCanShowIsNotSupported() {
assertFalse(isSupported("application/pdf"))
assertFalse(isSupported("application/vnd.oasis.opendocument.text"))
assertFalse(isSupported("application/octet-stream"))
assertFalse(isSupported("text/vcard"))
assertFalse(isSupported("text/rtf"))
assertFalse(isSupported(null))
}

companion object {

// @JvmStatic because junit requires @BeforeClass to be static
@JvmStatic
@BeforeClass
fun prepare() {
// the csv mime types come from a table in libodr_jni
CoreLoader.initializeCore(InstrumentationRegistry.getInstrumentation().targetContext)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,13 @@ class SupportedFormatsTest {
* Everything the app offers itself for reaches a loader that takes it.
*
* Claiming a mime type nobody loads is the "cannot open" the user gets on a file they picked us
* for, and it is what reading the core's whole table risks: [CoreLoader] matches every spelling
* of a format, but [RawLoader] goes by mime type prefix and so only ever sees the canonical one
* that [MetadataLoader] resolves to - `application/csv` and `multipart/x-zip` are claimed and
* would otherwise be dropped between the two.
* for. [CoreLoader] takes almost all of it now; the exception is csv, which [RawLoader] keeps
* for its table viewer and which [LoaderService] therefore routes before the core.
*/
@Test
fun everythingTheAppClaimsIsLoadedBySomebody() {
val coreLoader = CoreLoader(null)
val rawLoader = RawLoader(null, coreLoader)
val rawLoader = RawLoader(null)

for (mimeType in SupportedDocumentTypes.MIME_TYPES) {
val options = FileLoader.Options()
Expand Down
21 changes: 12 additions & 9 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,14 @@
tools:ignore="AppLinkUrlError">
<!--
STRICT_CATCH: what the app actually opens - opendocument, ooxml, the legacy
binary doc/ppt/xls and pdf through the core, plus text, csv, images and zip
through RawLoader. The three filters below are generated from odrcore's own
format table and must not be edited by hand alone; SupportedDocumentTypes
reads that same table at runtime, and SupportedFormatsTest fails when the two
stop agreeing.
binary doc/ppt/xls and pdf, plus text, images, zip and csv. All of it goes to
the core except csv, which RawLoader keeps for its table viewer. The three
filters below are generated from odrcore's own format table and must not be
edited by hand alone; SupportedDocumentTypes reads that same table at runtime,
and SupportedFormatsTest fails when the two stop agreeing.

Narrower than what the core can render on purpose: it also turns fonts, audio
and video into a page, none of which belong in a document viewer's share sheet.
-->
<intent-filter>
<action android:name="android.intent.action.VIEW" />
Expand Down Expand Up @@ -302,10 +305,10 @@
-->
<data android:mimeType="application/octet-stream" />
<!--
raw types handled by RawLoader: text/plain rather than text/*, which
would re-match the vcard contacts of #477, and image/* rather than the
five image formats odrcore names, because the WebView shows webp and
heic just as well.
the non-document types worth opening a viewer for: text/plain rather than
text/*, which would re-match the vcard contacts of #477, and image/*
rather than the image formats odrcore names, because it has no file type
for svg - which RawLoader still shows.
-->
<data android:mimeType="text/plain" />
<data android:mimeType="text/csv" />
Expand Down
43 changes: 0 additions & 43 deletions app/src/main/assets/audio.html

This file was deleted.

43 changes: 0 additions & 43 deletions app/src/main/assets/video.html

This file was deleted.

Loading
Loading