From 8b8f12780d3864b6333fe611f322163826adf177 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 14:05:30 +0200 Subject: [PATCH 1/2] refactor(api)!: one way to ask what a file is Twelve entry points answered the question. Four were the free functions; four more were DecodedFile statics the free functions forwarded to, so the same call had two spellings and the bindings picked different ones - apple went through the statics, jni through the free function. The bodies move to the free functions and the statics go. The other four were DocumentFile::type and ::meta, which decoded the entire file to read a single field and threw NoDocumentFile for anything that was not a document. open(...).file_type() and .file_meta() say the same thing without the narrowing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016hxDa2rev11eLUEJZJ5nmz --- CHANGELOG.md | 4 +++ apple/src/ODRFile.mm | 7 ++-- .../app/opendocument/core/DocumentFile.java | 12 ------- jni/src/jni_file.cpp | 17 --------- jni/tests/app/opendocument/core/FileTest.java | 6 ++-- python/src/bind_file.cpp | 18 ---------- python/tests/test_file.py | 16 +++------ src/odr/file.cpp | 35 ------------------- src/odr/file.hpp | 15 -------- src/odr/odr.cpp | 10 +++--- test/src/file_test.cpp | 12 +++---- test/src/internal/csv/csv_file_test.cpp | 3 +- .../internal/markdown/markdown_file_test.cpp | 2 +- test/src/internal/odf/odf_flat_file_test.cpp | 4 +-- test/src/internal/xml/xml_file_test.cpp | 2 +- 15 files changed, 32 insertions(+), 131 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0544b5e8f..ca88a2c93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- **Breaking**: `DecodedFile::list_file_types`/`::mimetype` and + `DocumentFile::type`/`::meta` are gone, in every binding. Use the free + `odr::list_file_types`/`odr::mimetype`, or `odr::open(...).file_type()`. + - **Breaking**: `html::translate` takes only a `DecodedFile`, `Document`, `Filesystem` or `Archive` now; drop the cache path, in every binding. Open a file as `FileType::text_file` to render it as a numbered line list. diff --git a/apple/src/ODRFile.mm b/apple/src/ODRFile.mm index 134beb9d0..68c145012 100644 --- a/apple/src/ODRFile.mm +++ b/apple/src/ODRFile.mm @@ -384,21 +384,20 @@ + (nullable instancetype)decodeFile:(ODRFile *)file logger:(ODRLogger *)logger error:(NSError **)error { return guarded(error, [&]() -> NSArray * { - return to_nsarray( - odr::DecodedFile::list_file_types(to_string(path), logger.handle)); + return to_nsarray(odr::list_file_types(to_string(path), logger.handle)); }); } + (nullable NSArray *)listFileTypesAtPath:(NSString *)path error:(NSError **)error { return guarded(error, [&]() -> NSArray * { - return to_nsarray(odr::DecodedFile::list_file_types(to_string(path))); + return to_nsarray(odr::list_file_types(to_string(path))); }); } + (nullable NSString *)mimetypeAtPath:(NSString *)path error:(NSError **)error { return guarded(error, [&]() -> NSString * { - return to_nsstring(odr::DecodedFile::mimetype(to_string(path))); + return to_nsstring(odr::mimetype(to_string(path))); }); } diff --git a/jni/java/app/opendocument/core/DocumentFile.java b/jni/java/app/opendocument/core/DocumentFile.java index f850fbfb5..2add96060 100644 --- a/jni/java/app/opendocument/core/DocumentFile.java +++ b/jni/java/app/opendocument/core/DocumentFile.java @@ -10,14 +10,6 @@ public DocumentFile(String path) { this(create(path)); } - public static FileType typeByPath(String path) { - return FileType.fromNative(typeByPathNative(path)); - } - - public static FileMeta metaByPath(String path) { - return metaByPathNative(path); - } - public DocumentType documentType() { return DocumentType.fromNative(documentTypeNative(handle())); } @@ -43,10 +35,6 @@ public Document document() { private static native long create(String path); - private static native int typeByPathNative(String path); - - private static native FileMeta metaByPathNative(String path); - private native int documentTypeNative(long handle); private native long decryptDocumentFileNative(long handle, String password); diff --git a/jni/src/jni_file.cpp b/jni/src/jni_file.cpp index 35687622c..5db8a9700 100644 --- a/jni/src/jni_file.cpp +++ b/jni/src/jni_file.cpp @@ -347,23 +347,6 @@ Java_app_opendocument_core_DocumentFile_create(JNIEnv *env, jclass, }); } -extern "C" JNIEXPORT jint JNICALL -Java_app_opendocument_core_DocumentFile_typeByPathNative(JNIEnv *env, jclass, - jstring path) { - return guarded(env, [&] { - return static_cast(odr::DocumentFile::type(to_string(env, path))); - }); -} - -extern "C" JNIEXPORT jobject JNICALL -Java_app_opendocument_core_DocumentFile_metaByPathNative(JNIEnv *env, jclass, - jstring path) { - return guarded(env, [&] { - return odr_jni::make_file_meta( - env, odr::DocumentFile::meta(to_string(env, path))); - }); -} - extern "C" JNIEXPORT jint JNICALL Java_app_opendocument_core_DocumentFile_documentTypeNative(JNIEnv *env, jobject, jlong handle) { diff --git a/jni/tests/app/opendocument/core/FileTest.java b/jni/tests/app/opendocument/core/FileTest.java index e6eecbde0..c3ea2832a 100644 --- a/jni/tests/app/opendocument/core/FileTest.java +++ b/jni/tests/app/opendocument/core/FileTest.java @@ -127,8 +127,8 @@ void listFileTypes() throws IOException { @Test void documentFileByPath() throws IOException { Path odt = TestFiles.odtFile(tempDir); - assertEquals(FileType.OPENDOCUMENT_TEXT, DocumentFile.typeByPath(odt.toString())); - FileMeta meta = DocumentFile.metaByPath(odt.toString()); - assertEquals(FileType.OPENDOCUMENT_TEXT, meta.type); + DecodedFile file = Odr.open(odt.toString()); + assertEquals(FileType.OPENDOCUMENT_TEXT, file.fileType()); + assertEquals(FileType.OPENDOCUMENT_TEXT, file.fileMeta().type); } } diff --git a/python/src/bind_file.cpp b/python/src/bind_file.cpp index 1ee6a53a3..9679d1fd5 100644 --- a/python/src/bind_file.cpp +++ b/python/src/bind_file.cpp @@ -265,24 +265,6 @@ void odr_python::bind_file(py::module_ &m) { }, py::arg("data"), py::arg("logger") = odr::Logger::null(), "Decode a document file held in memory; `data` is its bytes.") - // `type`/`meta` are overloaded on `File` and path, so the address of - // either is ambiguous; name the signature. - .def_static( - "type_by_file", - py::overload_cast(&odr::DocumentFile::type), - py::arg("file")) - .def_static( - "type_by_path", - py::overload_cast(&odr::DocumentFile::type), - py::arg("path")) - .def_static( - "meta_by_file", - py::overload_cast(&odr::DocumentFile::meta), - py::arg("file")) - .def_static( - "meta_by_path", - py::overload_cast(&odr::DocumentFile::meta), - py::arg("path")) .def("document_type", &odr::DocumentFile::document_type) .def("decrypt", &odr::DocumentFile::decrypt, py::arg("password"), py::call_guard()) diff --git a/python/tests/test_file.py b/python/tests/test_file.py index 36c369135..37a190b74 100644 --- a/python/tests/test_file.py +++ b/python/tests/test_file.py @@ -163,12 +163,10 @@ def test_decoded_file_from_file(odt_path): def test_document_file_from_file(odt_path): file = pyodr.File.from_memory(odt_path.read_bytes()) - assert pyodr.DocumentFile.type_by_file(file) == pyodr.FileType.opendocument_text - assert ( - pyodr.DocumentFile.meta_by_file(file).type == pyodr.FileType.opendocument_text - ) - document_file = pyodr.DocumentFile(file) + assert document_file.file_type() == pyodr.FileType.opendocument_text + assert document_file.file_meta().type == pyodr.FileType.opendocument_text + assert document_file.document_type() == pyodr.DocumentType.text @@ -213,10 +211,4 @@ def test_file_and_path_entry_points_agree(odt_path): assert pyodr.mimetype(file) == pyodr.mimetype(path) assert pyodr.list_file_types(file) == pyodr.list_file_types(path) assert pyodr.open(file).file_type() == pyodr.open(path).file_type() - assert pyodr.DocumentFile.type_by_file(file) == pyodr.DocumentFile.type_by_path( - path - ) - assert ( - pyodr.DocumentFile.meta_by_file(file).type - == pyodr.DocumentFile.meta_by_path(path).type - ) + assert pyodr.DocumentFile(file).file_type() == pyodr.DocumentFile(path).file_type() diff --git a/src/odr/file.cpp b/src/odr/file.cpp index d42ec4d8a..eb2db84e9 100644 --- a/src/odr/file.cpp +++ b/src/odr/file.cpp @@ -87,25 +87,6 @@ void File::copy(const std::string &path) const { std::shared_ptr File::impl() const { return m_impl; } -std::vector DecodedFile::list_file_types(const File &file, - const Logger &logger) { - return internal::open_strategy::list_file_types(file.impl(), logger); -} - -std::vector DecodedFile::list_file_types(const std::string &path, - const Logger &logger) { - return list_file_types(File::from_disk(path), logger); -} - -std::string_view DecodedFile::mimetype(const File &file, const Logger &logger) { - return internal::magic::mimetype(file.impl(), logger); -} - -std::string_view DecodedFile::mimetype(const std::string &path, - const Logger &logger) { - return mimetype(File::from_disk(path), logger); -} - DecodedFile::DecodedFile(std::shared_ptr impl) : m_impl{std::move(impl)} { if (m_impl == nullptr) { @@ -373,22 +354,6 @@ DocumentFile DocumentFile::from_memory(std::string data, const Logger &logger) { return DocumentFile(File::from_memory(std::move(data)), logger); } -FileType DocumentFile::type(const File &file) { - return DocumentFile(file).file_type(); -} - -FileType DocumentFile::type(const std::string &path) { - return type(File::from_disk(path)); -} - -FileMeta DocumentFile::meta(const File &file) { - return DocumentFile(file).file_meta(); -} - -FileMeta DocumentFile::meta(const std::string &path) { - return meta(File::from_disk(path)); -} - DocumentFile::DocumentFile( std::shared_ptr impl) : DecodedFile(impl), m_impl{std::move(impl)} {} diff --git a/src/odr/file.hpp b/src/odr/file.hpp index a095c55dc..f248494fb 100644 --- a/src/odr/file.hpp +++ b/src/odr/file.hpp @@ -352,16 +352,6 @@ class File final { /// @brief Represents a decoded file. class DecodedFile { public: - [[nodiscard]] static std::vector - list_file_types(const File &file, const Logger &logger = Logger::null()); - [[nodiscard]] static std::vector - list_file_types(const std::string &path, - const Logger &logger = Logger::null()); - [[nodiscard]] static std::string_view - mimetype(const File &file, const Logger &logger = Logger::null()); - [[nodiscard]] static std::string_view - mimetype(const std::string &path, const Logger &logger = Logger::null()); - explicit DecodedFile(std::shared_ptr impl); explicit DecodedFile(const File &file, const Logger &logger = Logger::null()); DecodedFile(const File &file, FileType as, @@ -525,11 +515,6 @@ class DocumentFile final : public DecodedFile { [[nodiscard]] static DocumentFile from_memory(std::string data, const Logger &logger = Logger::null()); - static FileType type(const File &file); - static FileType type(const std::string &path); - static FileMeta meta(const File &file); - static FileMeta meta(const std::string &path); - explicit DocumentFile(std::shared_ptr); explicit DocumentFile(const File &file, const Logger &logger = Logger::null()); diff --git a/src/odr/odr.cpp b/src/odr/odr.cpp index 247d80ab5..5d14ae71c 100644 --- a/src/odr/odr.cpp +++ b/src/odr/odr.cpp @@ -6,6 +6,8 @@ #include #include #include +#include +#include #include #include @@ -184,20 +186,20 @@ bool odr::text_encoding_is_decodable(const TextEncoding encoding) noexcept { std::vector odr::list_file_types(const File &file, const Logger &logger) { - return DecodedFile::list_file_types(file, logger); + return internal::open_strategy::list_file_types(file.impl(), logger); } std::vector odr::list_file_types(const std::string &path, const Logger &logger) { - return DecodedFile::list_file_types(path, logger); + return list_file_types(File::from_disk(path), logger); } std::string_view odr::mimetype(const File &file, const Logger &logger) { - return DecodedFile::mimetype(file, logger); + return internal::magic::mimetype(file.impl(), logger); } std::string_view odr::mimetype(const std::string &path, const Logger &logger) { - return DecodedFile::mimetype(path, logger); + return mimetype(File::from_disk(path), logger); } odr::DecodedFile odr::open(const File &file, const Logger &logger) { diff --git a/test/src/file_test.cpp b/test/src/file_test.cpp index 3a40d1f63..d3997bc7e 100644 --- a/test/src/file_test.cpp +++ b/test/src/file_test.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -162,12 +163,11 @@ TEST(File, from_memory_decodes_the_same_as_from_disk) { EXPECT_EQ(from_memory.file_meta().document_type, from_disk.file_meta().document_type); - EXPECT_EQ(DecodedFile::list_file_types( - File::from_memory(internal::util::file::read(path))), - DecodedFile::list_file_types(path)); - EXPECT_EQ(DecodedFile::mimetype( - File::from_memory(internal::util::file::read(path))), - DecodedFile::mimetype(path)); + EXPECT_EQ( + list_file_types(File::from_memory(internal::util::file::read(path))), + list_file_types(path)); + EXPECT_EQ(mimetype(File::from_memory(internal::util::file::read(path))), + mimetype(path)); } /// `MemoryFile` used to report itself as `disk`, and `memory_data()` handed diff --git a/test/src/internal/csv/csv_file_test.cpp b/test/src/internal/csv/csv_file_test.cpp index 0262efc51..b4e686a1d 100644 --- a/test/src/internal/csv/csv_file_test.cpp +++ b/test/src/internal/csv/csv_file_test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -66,7 +67,7 @@ TEST(CsvFile, csv) { std::make_shared(file.impl()))); // and the probe in `open_strategy` reaches the same conclusion - EXPECT_THAT(DecodedFile::list_file_types(file), + EXPECT_THAT(list_file_types(file), testing::Contains(FileType::comma_separated_values)); } diff --git a/test/src/internal/markdown/markdown_file_test.cpp b/test/src/internal/markdown/markdown_file_test.cpp index 76db898a7..3d6c79caf 100644 --- a/test/src/internal/markdown/markdown_file_test.cpp +++ b/test/src/internal/markdown/markdown_file_test.cpp @@ -120,7 +120,7 @@ TEST(MarkdownFile, translating_the_decoded_file_yields_the_document) { TEST(MarkdownFile, it_is_not_detected_by_content) { const File file = File::from_memory("# hello\n\nsome *markdown*\n"); - EXPECT_THAT(DecodedFile::list_file_types(file), + EXPECT_THAT(list_file_types(file), testing::Not(testing::Contains(FileType::markdown))); EXPECT_EQ(DecodedFile(file).file_type(), FileType::text_file); } diff --git a/test/src/internal/odf/odf_flat_file_test.cpp b/test/src/internal/odf/odf_flat_file_test.cpp index d5ae50376..b03f4b4dc 100644 --- a/test/src/internal/odf/odf_flat_file_test.cpp +++ b/test/src/internal/odf/odf_flat_file_test.cpp @@ -394,8 +394,8 @@ TEST(FlatOpenDocumentFile, the_statistics_give_the_entry_count) { /// A flat document is well formed xml too, so both readings are reported. TEST(FlatOpenDocumentFile, it_is_listed_next_to_the_source_view) { - const std::vector types = DecodedFile::list_file_types( - File::from_memory(flat_text("Hello"))); + const std::vector types = + list_file_types(File::from_memory(flat_text("Hello"))); EXPECT_NE(std::ranges::find(types, FileType::xml), std::end(types)); EXPECT_NE(std::ranges::find(types, FileType::opendocument_text), diff --git a/test/src/internal/xml/xml_file_test.cpp b/test/src/internal/xml/xml_file_test.cpp index 63bf343e8..dfb152242 100644 --- a/test/src/internal/xml/xml_file_test.cpp +++ b/test/src/internal/xml/xml_file_test.cpp @@ -53,7 +53,7 @@ TEST(XmlFile, an_xml_file_opens_as_xml) { EXPECT_FALSE(file.is_document_file()); EXPECT_TRUE(file.capabilities().translate_html); - EXPECT_THAT(DecodedFile::list_file_types(File::from_memory("")), + EXPECT_THAT(list_file_types(File::from_memory("")), testing::Contains(FileType::xml)); } From 0e2a00cfa4c7cceec86d2b3b8214d17840380195 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 14:24:36 +0200 Subject: [PATCH 2/2] fixup: the wasm binding asked DecodedFile too detect() reached for the statics rather than the free functions, and nothing local builds wasm, so it went unnoticed until a syntax check against the emscripten toolchain. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016hxDa2rev11eLUEJZJ5nmz --- wasm/src/wasm_file.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wasm/src/wasm_file.cpp b/wasm/src/wasm_file.cpp index 77bee6161..ae7963a26 100644 --- a/wasm/src/wasm_file.cpp +++ b/wasm/src/wasm_file.cpp @@ -37,13 +37,13 @@ emscripten::val detect(const std::string &bytes, std::string name) { const Logger &logger = default_logger(); emscripten::val types = emscripten::val::array(); - for (const FileType type : DecodedFile::list_file_types(file, logger)) { + for (const FileType type : odr::list_file_types(file, logger)) { types.call("push", static_cast(type)); } emscripten::val result = emscripten::val::object(); result.set("fileTypes", types); - result.set("mimeType", std::string(DecodedFile::mimetype(file, logger))); + result.set("mimeType", std::string(odr::mimetype(file, logger))); return ok(result); }); }