feat: expose the supported-format tables and a per-file-type capability query - #634
Merged
Conversation
…ty query
Consumers had to rebuild odrcore's format knowledge by hand — OpenDocument.droid
keeps three separate format tables plus an instrumented test to hold them in
sync. The lookups in `odr.hpp` were one-directional and incomplete, and there
was no way at all to ask what the library can do with a format *before* holding
a file, which is exactly when an app has to decide (intent filters, picker MIME
lists).
Everything lived as six independent if-chains over `FileType` in `odr.cpp`,
which had already drifted apart: `markdown` had a MIME type but no extension,
the `wpd` MIME round trip was broken (`odr_test` documented it), `rtf`/`svm`/
`cfb`/`ooxml_encrypted` had no MIME type at all, there was no
`extension_by_file_type`, and whole alias families were missing.
Collapse all of it into one `constexpr` table in the new
`internal/file_type_table.{hpp,cpp}` — one row per `FileType` holding the name,
every extension, every MIME type, the category, the document type and the
capabilities. Every public lookup is now a thin forward into it; `odr.cpp`
shrinks from 441 to 164 lines.
On top of that:
- `all_file_types`, `file_extensions_by_file_type`, `file_extension_by_file_type`
and `mimetypes_by_file_type` enumerate what a file type accepts, so a consumer
never needs its own copy of these tables.
- `FileTypeCapabilities` + `capabilities_by_file_type` answer, per format,
whether we can detect, open, decrypt, render, edit, save or encrypt it. It is
explicitly a *declared upper bound*; `DecodedFile::capabilities()` refines it
for the file in hand, and `Document::is_editable`/`is_savable` remain the
precise answer.
- The missing aliases: `docm`/`dotx`/`dotm`, `pptm`/`potx`/`ppsx`/`ppsm`,
`xlsm`/`xltx`/`xlsb`, `dot`/`pot`/`pps`/`xlt`, `md`, `otm`, `ttc`, the ODF
`x-vnd`/`-template`/`-master`/`-flat-xml` families and the OOXML
`.template`/`.slideshow`/`macroEnabled.12` families.
A hand-maintained table can lie, so the invariant is a test rather than
discipline: `FileTypeCapabilities.declaration_matches_the_engines` walks the
corpus and asserts that `DecodedFile::capabilities()` is a subset of what the
table declares, and that `Document::is_editable()`/`is_savable(false|true)`
*equal* the declared `edit`/`save`/`encrypt`. Further tests assert that every
`FileType` has exactly one row (derived from the enum, not the table), that no
extension or MIME type is claimed twice, and that every alias round-trips.
`html_output_test` now derives its "cannot open" skip from the table instead of
a hardcoded list.
Behaviour changes:
- `mimetype_by_file_type` throws only for `FileType::unknown` now. wpd, rtf,
cfb, ooxml-encrypted and svm gained canonical types; svm's is taken from
`svm_file.cpp` so the static answer matches what the file object reports.
- `file_category_by_file_type(pdf)` reported `unknown` while
`abstract::PdfFile::file_category()` has always said `document` — it is
`document` now, with `document_type` `text`, matching what `pdf_file.cpp`
already puts in `file_meta`. `office_open_xml_encrypted` likewise becomes
`document` and is named `ooxml_encrypted` instead of `unnamed`.
- `internal::ImageFile::mimetype()` returned `""`; it answers from the table.
`xlsb` is deliberately classified as a workbook (that is its MIME family) even
though there is no decoder for the binary package — a `detect != open` case,
which is what the capability struct exists to express.
Reference outputs are regenerated for the two changed metadata fields; every
HTML output is byte-identical.
Closes #632.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012c8P8ueTw7pfNtCAzzVA8Z
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46f9d4ad05
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Two review findings on #634. `xlsb` rode along in the `office_open_xml_workbook` row, so `file_type_by_file_extension("xlsb")` inherited that row's `open` / `translate_html` — a caller configuring a file picker from the new tables would offer xlsb files that then fail to open. The claim that "the capability struct expresses this" was wrong: capabilities are keyed on `FileType`, so a type shared with xlsx cannot say anything different. Give it `FileType:: excel_binary_workbook` instead, still classified as a spreadsheet document but with an all-false capability row. `odr.hpp` only forward-declared `FileTypeCapabilities`, which made `odr::capabilities_by_file_type(type)` fail with an incomplete return type unless the caller also knew to include `<odr/file.hpp>`. Include `file.hpp` from `odr.hpp` — every lookup there is *about* those types, and two of them (`FileTypeCapabilities`, `DecodedFile`) are returned by value — and drop the now-redundant forward declarations. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012c8P8ueTw7pfNtCAzzVA8Z
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
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.
🤖 Generated with Claude Code
Closes #632.
Why
Consumers have to rebuild odrcore's format knowledge by hand — OpenDocument.droid keeps three separate format tables plus an instrumented test to hold them in sync. The lookups in
odr.hppare one-directional and incomplete, and there is no way at all to ask what the library can do with a format before holding a file, which is exactly when an app has to decide (intent filters, picker MIME lists, "open with" entries).All of it lived as six independent if-chains over
FileTypeinodr.cpp, which had already drifted apart:markdownhad a MIME type but no extension.wpdMIME round trip was broken — libmagic reportsapplication/vnd.wordperfect,file_type_by_mimetypecould not map it back, andodr_test::types_wpddocumented the gap.rtf,svm,cfbandoffice_open_xml_encryptedhad no MIME type at all.extension_by_file_type;file_type_to_stringaccidentally doubled as one.docm,dotx,xlsb,ppsx, the macro-enabled and template MIME types,application/x-vnd.oasis.*).What
One central table.
src/odr/internal/file_type_table.{hpp,cpp}— aconstexprarray, one row perFileType, holding the name, every extension, every MIME type (canonical first), the category, the document type and the capabilities. Every public lookup is now a thin forward into it;odr.cppshrinks from 441 to 164 lines.Enumeration.
all_file_types,file_extensions_by_file_type,file_extension_by_file_type,mimetypes_by_file_type— so a consumer never needs its own copy of these tables.Capabilities.
FileTypeCapabilities+capabilities_by_file_typeanswer, per format, whether we can detect / open / decrypt / render / edit / save / encrypt it:It is explicitly a declared upper bound — the question you have to answer before you hold a file.
DecodedFile::capabilities()refines it for the file in hand, andDocument::is_editable/is_savablestay the precise answer.Aliases.
docm/dotx/dotm,pptm/potx/potm/ppsx/ppsm,xlsm/xltx/xltm,dot/pot/pps/xlt/xlm,md/markdown,otm,text,ttc,xlsb; the ODFx-vnd/-template/-master/-flat-xmlfamilies, the OOXML.template/.slideshow/macroEnabled.12families, plustext/rtf,application/x-pdf,image/x-ms-bmpand friends.Keeping it honest
A hand-maintained table can lie, so the invariant is a test rather than discipline.
FileTypeCapabilities.declaration_matches_the_engineswalks the corpus and asserts thatDecodedFile::capabilities()is a subset of what the table declares, and thatDocument::is_editable()/is_savable(false|true)equal the declarededit/save/encrypt. Over-declaring edit support for a format that cannot edit now fails CI.Alongside it: every
FileTypehas exactly one row (derived from the enum, not the table, so a missing row is caught), no extension or MIME type is claimed twice, every alias round-trips, the canonical alias isfront(), and onlyunknownlacks a MIME type.html_output_testnow derives its "cannot open" skip fromcapabilities_by_file_type(...).openinstead of a hardcoded list.Behaviour changes
mimetype_by_file_typethrows only forFileType::unknownnow. wpd, rtf, cfb, ooxml-encrypted and svm gained canonical types; svm's is taken fromsvm_file.cppso the static answer matches what the file object reports.file_category_by_file_type(pdf)reportedunknownwhileabstract::PdfFile::file_category()has always saiddocument. It isdocumentnow, withdocument_typetext— matching whatpdf_file.cppalready puts infile_meta.office_open_xml_encryptedlikewise becomesdocument, and is namedooxml_encryptedinstead ofunnamed.internal::ImageFile::mimetype()returned""; it answers from the table.xlsbgets its ownFileType::excel_binary_workbookrather than riding along withxlsx: the package is OOXML but the workbook parts are binary, so there is no decoder. Since capabilities are keyed onFileType, sharing the xlsx row would have made it inheritopen/translate_htmland led exactly the pickers this PR is meant to serve into offering files that cannot be opened. Its own type carries an all-false capability row.Bindings
Both layers expose the new API: JNI gets
FileTypeCapabilities.java, five natives onOdrandDecodedFile.capabilities(); pyodr gets the matching free functions plusFileTypeCapabilitiesandDecodedFile.capabilities.Reference output
Regenerated for the two changed metadata fields (
fileCategoryon every PDF and the two encrypted OOXML packages;fileTypeon those two). Every HTML output is byte-identical, so nohtml-tidypass was involved.Testing
odr_testsuite green. The category change surfaced five PDFs whosedocument_typeis unset because the meta parse is all-or-nothing (seepdf/AGENTS.md); the comparison is guarded for that.pytest python/tests: 43/43.javac --release 17 -Xlint:allover all ofjni/java, and the touchedjni_*.cppagainst JDK headers). The JUnit suite itself was not run locally — CI covers it.