Skip to content

feat!: collapse FileMeta into a single type - #626

Merged
andiwand merged 3 commits into
mainfrom
feat/flatten-file-meta
Jul 27, 2026
Merged

feat!: collapse FileMeta into a single type#626
andiwand merged 3 commits into
mainfrom
feat/flatten-file-meta

Conversation

@andiwand

@andiwand andiwand commented Jul 27, 2026

Copy link
Copy Markdown
Member

🤖 Generated with Claude Code

Clears the second of the two Breaking API changes (next major) items in
docs/design/README.md (the first landed in #625). With this the section is empty
and both entries are removed, so nothing deprecated or explicitly deferred remains
before the v6 tag.

Why

FileMeta carried an std::optional<DocumentMeta>, so every reader of a document
property went through two hops and an optional check, and every backend building
one had to remember to materialise the nested struct first. The indirection bought
nothing — DocumentMeta was never handed out independently except through
DocumentFile::document_meta(), which is just a second way of reading what
file_meta() already returns.

What

DocumentMeta's fields move up into FileMeta; the optional's presence check
becomes document_type != DocumentType::unknown.

Breaking changes

C++

  • odr::DocumentMeta is gone; its fields are members of odr::FileMeta.
  • FileMeta::document_meta is gone. document_type is a direct member, unknown
    when the file is not a document; the remaining document fields are only
    meaningful when it is set.
  • DocumentFile::document_meta() and the abstract::DocumentFile::document_meta()
    virtual are gone — use file_meta().
  • FileMeta's two constructors are gone; it is an aggregate.

JavaDocumentMeta removed, its fields are on FileMeta,
DocumentFile.documentMeta() removed.

PythonDocumentMeta removed, its fields are on FileMeta,
DocumentFile.document_meta() removed.

One JSON change (an earlier revision of this description wrongly said "unchanged")

meta_to_json is unchanged for every file with a known document type — it already
flattened both structs into one object.

It does change for an office container whose document type cannot be named.
The old backends assigned a default-constructed DocumentMeta before returning,
so an encrypted OOXML package emitted "documentType": "unknown". The flat struct
cannot distinguish "not a document" from "document of unknown type", so those files
now omit the document keys like any other non-document.

The reference-output run puts an exact number on the blast radius: 326 of 328
outputs matched
, and the two that changed are docx/encrypted.docx/meta.json and
xlsx/encrypted.xlsx/meta.json, each losing one line:

 {
-    "documentType": "unknown",
     "fileCategory": "unknown",
     "fileType": "unnamed",
     "isEncrypted": true
 }

That line said nothing its three neighbours did not already say, and a plain text
file never got a documentType key at all — so the new behaviour is also the more
consistent one. Reference outputs updated accordingly (no HTML moved, and
odr-private is untouched).

Thanks @chatgpt-codex-connector for catching this — it was a real behaviour change
hiding behind an "output unchanged" claim. The gate is now commented in
odr_meta_util.cpp so it reads as deliberate.

Note on the PDF backend

populate_document_meta was all-or-nothing: a parse throw left document_meta
unset rather than half-filled. Flattening would have silently lost that, since
fields are now written straight onto FileMeta, so the helper fills a copy and
commits it only on success. Behaviour is preserved and the invariant is now
spelled out in pdf/AGENTS.md.

Note on aggregate initialisation

The backends first used designated initializers naming only the three non-document
members. GCC rejects that under -Werror=missing-field-initializers even though
the remaining members have default member initializers, so they build the value
field by field instead — matching how odf_meta / ooxml_meta / oldms_file
already did it.

Test plan

  • Full C++ suite (excluding the slow HTML-output regression): 466 passed.
  • Full suite incl. HTML output, compared against the reference tree: 326/328
    matched before the reference bump, the 2 above after.
  • Python bindings: 41 passed, incl. a new assertion that a non-document (CSV)
    reports document_type == unknown with the document fields unset — the sentinel
    that replaces the optional.
  • JNI bindings: 31 passed, incl. the reworked FileMeta constructor signature.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5abc4472a7

ℹ️ 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".

Comment thread src/odr/internal/util/odr_meta_util.cpp
@andiwand
andiwand force-pushed the feat/logger-value-type branch from 46e03ee to 5ce4a5d Compare July 27, 2026 19:26
@andiwand
andiwand force-pushed the feat/flatten-file-meta branch from 2ee7a93 to 22a35ad Compare July 27, 2026 19:27
@andiwand
andiwand force-pushed the feat/logger-value-type branch from 5ce4a5d to 87f7cf3 Compare July 27, 2026 19:34
@andiwand
andiwand force-pushed the feat/flatten-file-meta branch from 22a35ad to 5c4fc64 Compare July 27, 2026 19:35
Base automatically changed from feat/logger-value-type to main July 27, 2026 19:47
andiwand added a commit to opendocument-app/OpenDocument.test.output that referenced this pull request Jul 27, 2026
`odr::FileMeta` no longer carries an optional nested `DocumentMeta`, so
"is a document" is now signalled by `document_type != unknown`. The
backends previously materialised a default-constructed `DocumentMeta`
before returning, which made an encrypted OOXML package report
`"documentType": "unknown"` — a value that said nothing the neighbouring
`fileType: unnamed` / `fileCategory: unknown` / `isEncrypted: true` did
not already say, and that a plain text file never got in the first place.

These two files are the only outputs affected (326 of 328 matched).

See opendocument-app/OpenDocument.core#626.
andiwand and others added 3 commits July 27, 2026 21:50
`FileMeta` carried an `std::optional<DocumentMeta>`, so every reader of a
document property went through two hops and an optional check, and every
backend building one had to remember to materialise the nested struct
first. The indirection bought nothing: `DocumentMeta` was never handed out
independently except through `DocumentFile::document_meta()`, which is
just a second way of reading what `file_meta()` already returns.

`DocumentMeta`'s fields move up into `FileMeta` and the optional's
presence check becomes `document_type != DocumentType::unknown`.
`FileMeta` is now a plain aggregate — the backends that spelled
`{file_type(), mimetype(), false, std::nullopt}` use designated
initializers instead.

The JSON from `meta_to_json` is unchanged: it already flattened both
structs into one object.

BREAKING CHANGE:
- `odr::DocumentMeta` is gone; its fields are members of `odr::FileMeta`.
- `FileMeta::document_meta` is gone. `document_type` is now a direct
  member, `unknown` when the file is not a document; the remaining
  document fields are only meaningful when it is set.
- `DocumentFile::document_meta()` and the
  `abstract::DocumentFile::document_meta()` virtual are gone — use
  `file_meta()`.
- `FileMeta`'s two constructors are gone; it is an aggregate.
- Java: `DocumentMeta` is gone, its fields are on `FileMeta`, and
  `DocumentFile.documentMeta()` is removed.
- Python: `DocumentMeta` is gone, its fields are on `FileMeta`, and
  `DocumentFile.document_meta()` is removed.

The PDF backend keeps its all-or-nothing metadata semantics: a malformed
structure leaves every document field unset rather than half-filled.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KbnNCsz6rkVKHW2MWL47m5
GCC rejects the designated initializers that name only the first three
members under `-Werror=missing-field-initializers`, even though the rest
carry default member initializers. Build the value field by field
instead, matching how `odf_meta` / `ooxml_meta` / `oldms_file` already do
it.

Also spell out why the JSON document block is gated on `document_type`:
an office container we cannot name a type for (an encrypted OOXML
package, an ODF file with an unrecognized mimetype) now omits those keys
instead of reporting `"documentType": "unknown"`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KbnNCsz6rkVKHW2MWL47m5
An encrypted OOXML package no longer reports `"documentType": "unknown"`,
since `document_type != unknown` is now the "is a document" signal and the
backends no longer materialise a default `DocumentMeta` on the way out.
The two affected outputs are the only ones that changed (326 of 328
matched); no HTML moved.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KbnNCsz6rkVKHW2MWL47m5
@andiwand
andiwand force-pushed the feat/flatten-file-meta branch from 5c4fc64 to 6300a72 Compare July 27, 2026 19:50
@andiwand
andiwand enabled auto-merge (squash) July 27, 2026 19:53
@andiwand
andiwand merged commit 08e77fa into main Jul 27, 2026
18 checks passed
@andiwand
andiwand deleted the feat/flatten-file-meta branch July 27, 2026 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant