refactor(api)!: one way to open a file

CHANGELOG.md

@@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- **Breaking**: `odr::open` is the only way to decode a file. The `DecodedFile`
+  and `DocumentFile` constructors and `DocumentFile::from_disk`/`::from_memory`
+  are gone, in every binding; narrow with `open(...).as_document_file()`.
+
 - **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()`.

apple/src/ODRFile.mm

@@ -313,8 +313,7 @@ + (instancetype)decodedFileWithHandle:(odr::DecodedFile)handle {
 
 + (nullable instancetype)decodePath:(NSString *)path error:(NSError **)error {
   return guarded(error, [&]() -> ODRDecodedFile * {
-    return [ODRDecodedFile
-        decodedFileWithHandle:odr::DecodedFile(to_string(path))];
+    return [ODRDecodedFile decodedFileWithHandle:odr::open(to_string(path))];
   });
 }
 
@@ -323,9 +322,8 @@ + (nullable instancetype)decodePath:(NSString *)path
                               error:(NSError **)error {
   return guarded(error, [&]() -> ODRDecodedFile * {
     return [ODRDecodedFile
-        decodedFileWithHandle:odr::DecodedFile(
-                                  to_string(path),
-                                  static_cast<odr::FileType>(type))];
+        decodedFileWithHandle:odr::open(to_string(path),
+                                        static_cast<odr::FileType>(type))];
   });
 }
 
@@ -340,13 +338,13 @@ + (nullable instancetype)decodePath:(NSString *)path
     }
     native.file_type_priority = to_file_types(preference.fileTypePriority);
     return [ODRDecodedFile
-        decodedFileWithHandle:odr::DecodedFile(to_string(path), native)];
+        decodedFileWithHandle:odr::open(to_string(path), native)];
   });
 }
 
 + (nullable instancetype)decodeFile:(ODRFile *)file error:(NSError **)error {
   return guarded(error, [&]() -> ODRDecodedFile * {
-    return [ODRDecodedFile decodedFileWithHandle:odr::DecodedFile(file.handle)];
+    return [ODRDecodedFile decodedFileWithHandle:odr::open(file.handle)];
   });
 }
 
@@ -355,7 +353,7 @@ + (nullable instancetype)decodePath:(NSString *)path
                               error:(NSError **)error {
   return guarded(error, [&]() -> ODRDecodedFile * {
     return [ODRDecodedFile
-        decodedFileWithHandle:odr::DecodedFile(to_string(path), logger.handle)];
+        decodedFileWithHandle:odr::open(to_string(path), logger.handle)];
   });
 }
 
@@ -365,9 +363,9 @@ + (nullable instancetype)decodePath:(NSString *)path
                               error:(NSError **)error {
   return guarded(error, [&]() -> ODRDecodedFile * {
     return [ODRDecodedFile
-        decodedFileWithHandle:odr::DecodedFile(to_string(path),
-                                               static_cast<odr::FileType>(type),
-                                               logger.handle)];
+        decodedFileWithHandle:odr::open(to_string(path),
+                                        static_cast<odr::FileType>(type),
+                                        logger.handle)];
   });
 }
 
@@ -376,7 +374,7 @@ + (nullable instancetype)decodeFile:(ODRFile *)file
                               error:(NSError **)error {
   return guarded(error, [&]() -> ODRDecodedFile * {
     return [ODRDecodedFile
-        decodedFileWithHandle:odr::DecodedFile(file.handle, logger.handle)];
+        decodedFileWithHandle:odr::open(file.handle, logger.handle)];
   });
 }
 

cli/src/back_translate.cpp

@@ -1,6 +1,7 @@
 #include <odr/document.hpp>
 #include <odr/file.hpp>
 #include <odr/html.hpp>
+#include <odr/odr.hpp>
 
 #include <odr/internal/util/file_util.hpp>
 
@@ -23,7 +24,7 @@ int main(const int argc, char **argv) {
     const std::string diff_path{argv[2]};
     const std::string output{argv[3]};
 
-    const DocumentFile document_file{input};
+    const DocumentFile document_file = open(input).as_document_file();
 
     if (document_file.password_encrypted()) {
       ODR_FATAL(logger, "encrypted documents are not supported");

cli/src/meta.cpp

@@ -1,4 +1,5 @@
 #include <odr/file.hpp>
+#include <odr/odr.hpp>
 
 #include <odr/internal/util/odr_meta_util.hpp>
 
@@ -25,7 +26,7 @@ int main(const int argc, char **argv) {
       password = argv[2];
     }
 
-    DocumentFile document_file{input};
+    DocumentFile document_file = open(input).as_document_file();
 
     if (document_file.password_encrypted()) {
       if (!password) {

cli/src/server.cpp

@@ -4,6 +4,7 @@
 #include <odr/filesystem.hpp>
 #include <odr/html.hpp>
 #include <odr/http_server.hpp>
+#include <odr/odr.hpp>
 
 #include <cstdint>
 #include <iostream>
@@ -30,7 +31,7 @@ int main(const int argc, char **argv) {
     DecodePreference decode_preference;
     decode_preference.as_file_type = FileType::zip;
 
-    DecodedFile decoded_file{input, decode_preference, logger};
+    DecodedFile decoded_file = open(input, decode_preference, logger);
 
     if (decoded_file.password_encrypted()) {
       if (!password) {

cli/src/translate.cpp

@@ -1,6 +1,7 @@
 #include <odr/exceptions.hpp>
 #include <odr/file.hpp>
 #include <odr/html.hpp>
+#include <odr/odr.hpp>
 
 #include <filesystem>
 #include <iostream>
@@ -26,7 +27,7 @@ int main(const int argc, char **argv) {
       password = argv[3];
     }
 
-    DecodedFile decoded_file{input};
+    DecodedFile decoded_file = open(input);
 
     if (decoded_file.password_encrypted()) {
       if (!password) {

jni/java/app/opendocument/core/DecodedFile.java

@@ -1,9 +1,9 @@
 package app.opendocument.core;
 
 /**
- * A decoded file. Mirrors {@code odr::DecodedFile}. Obtain via
- * {@link Odr#open} or the constructors; {@code as*} accessors return typed
- * views and throw when the file is not of that kind (check {@code is*} first).
+ * A decoded file. Mirrors {@code odr::DecodedFile}. Obtain via {@link Odr#open};
+ * {@code as*} accessors return typed views and throw when the file is not of
+ * that kind (check {@code is*} first).
  */
 public class DecodedFile extends NativeResource {
   static {
@@ -14,18 +14,6 @@ public class DecodedFile extends NativeResource {
     super(handle, null, DecodedFile::destroy);
   }
 
-  public DecodedFile(String path) {
-    this(create(path));
-  }
-
-  public DecodedFile(String path, FileType as) {
-    this(createAs(path, as.toNative()));
-  }
-
-  public DecodedFile(File file) {
-    this(file.decode());
-  }
-
   public File file() {
     return new File(fileNative(handle()));
   }
@@ -116,10 +104,6 @@ public FontFile asFontFile() {
     return new FontFile(asFontFileNative(handle()));
   }
 
-  private static native long create(String path);
-
-  private static native long createAs(String path, int as);
-
   static native void destroy(long handle);
 
   private native long fileNative(long handle);

jni/java/app/opendocument/core/DocumentFile.java

@@ -6,10 +6,6 @@ public final class DocumentFile extends DecodedFile {
     super(handle);
   }
 
-  public DocumentFile(String path) {
-    this(create(path));
-  }
-
   public DocumentType documentType() {
     return DocumentType.fromNative(documentTypeNative(handle()));
   }
@@ -33,8 +29,6 @@ public Document document() {
     return new Document(documentNative(handle()));
   }
 
-  private static native long create(String path);
-
   private native int documentTypeNative(long handle);
 
   private native long decryptDocumentFileNative(long handle, String password);

jni/java/app/opendocument/core/Odr.java

@@ -100,6 +100,15 @@ public static List<FileType> listFileTypes(String path) {
   /** Determines the MIME type of a file. */
   public static native String mimetype(String path);
 
+  /** Decodes an already-opened file. */
+  public static DecodedFile open(File file) {
+    try {
+      return new DecodedFile(file.decode());
+    } finally {
+      file.keepAlive();
+    }
+  }
+
   /** Opens and decodes a file. */
   public static DecodedFile open(String path) {
     return new DecodedFile(openNative(path));

jni/src/jni_file.cpp

@@ -90,7 +90,7 @@ extern "C" JNIEXPORT void JNICALL Java_app_opendocument_core_File_copyNative(
 extern "C" JNIEXPORT jlong JNICALL Java_app_opendocument_core_File_decodeNative(
     JNIEnv *env, jobject, jlong handle) {
   return guarded(env, [&] {
-    return make_handle(odr::DecodedFile(*from_handle<odr::File>(handle)));
+    return make_handle(odr::open(*from_handle<odr::File>(handle)));
   });
 }
 
@@ -99,22 +99,6 @@ extern "C" JNIEXPORT jlong JNICALL Java_app_opendocument_core_File_decodeNative(
 // Handles hold a heap `odr::DecodedFile` (the typed C++ subclasses are sliced
 // away); typed accessors re-derive the typed view per call via `as_*`.
 
-extern "C" JNIEXPORT jlong JNICALL
-Java_app_opendocument_core_DecodedFile_create(JNIEnv *env, jclass,
-                                              jstring path) {
-  return guarded(
-      env, [&] { return make_handle(odr::DecodedFile(to_string(env, path))); });
-}
-
-extern "C" JNIEXPORT jlong JNICALL
-Java_app_opendocument_core_DecodedFile_createAs(JNIEnv *env, jclass,
-                                                jstring path, jint as) {
-  return guarded(env, [&] {
-    return make_handle(
-        odr::DecodedFile(to_string(env, path), static_cast<odr::FileType>(as)));
-  });
-}
-
 extern "C" JNIEXPORT void JNICALL
 Java_app_opendocument_core_DecodedFile_destroy(JNIEnv *env, jclass,
                                                jlong handle) {
@@ -338,15 +322,6 @@ Java_app_opendocument_core_ArchiveFile_archiveNative(JNIEnv *env, jobject,
 
 // app.opendocument.core.DocumentFile
 
-extern "C" JNIEXPORT jlong JNICALL
-Java_app_opendocument_core_DocumentFile_create(JNIEnv *env, jclass,
-                                               jstring path) {
-  return guarded(env, [&] {
-    return make_handle(
-        odr::DecodedFile(odr::DocumentFile(to_string(env, path))));
-  });
-}
-
 extern "C" JNIEXPORT jint JNICALL
 Java_app_opendocument_core_DocumentFile_documentTypeNative(JNIEnv *env, jobject,
                                                            jlong handle) {

jni/tests/app/opendocument/core/FileTest.java

@@ -95,7 +95,7 @@ void archiveEntryName() throws IOException {
   void decodeAnOpenFile() throws IOException {
     Path odt = TestFiles.odtFile(tempDir);
     try (File file = new File(odt.toString());
-        DecodedFile decoded = new DecodedFile(file)) {
+        DecodedFile decoded = Odr.open(file)) {
       assertEquals(FileType.OPENDOCUMENT_TEXT, decoded.fileType());
     }
   }

python/src/bind_file.cpp

@@ -185,21 +185,8 @@ void odr_python::bind_file(py::module_ &m) {
           "Read the whole file into bytes.")
       .def("copy", &odr::File::copy, py::arg("path"));
 
+  // no constructor: `pyodr.open(...)` decodes, this is what it hands back
   py::class_<odr::DecodedFile>(m, "DecodedFile")
-      .def(py::init<const odr::File &, const odr::Logger &>(), py::arg("file"),
-           py::arg("logger") = odr::Logger::null())
-      .def(py::init<const odr::File &, odr::FileType, const odr::Logger &>(),
-           py::arg("file"), py::arg("as_type"),
-           py::arg("logger") = odr::Logger::null())
-      .def(py::init<const odr::File &, const odr::DecodePreference &,
-                    const odr::Logger &>(),
-           py::arg("file"), py::arg("preference"),
-           py::arg("logger") = odr::Logger::null())
-      .def(py::init<const std::string &, const odr::Logger &>(),
-           py::arg("path"), py::arg("logger") = odr::Logger::null())
-      .def(py::init<const std::string &, odr::FileType, const odr::Logger &>(),
-           py::arg("path"), py::arg("as_type"),
-           py::arg("logger") = odr::Logger::null())
       .def("file", &odr::DecodedFile::file)
       .def("file_type", &odr::DecodedFile::file_type)
       .def("file_category", &odr::DecodedFile::file_category)
@@ -246,25 +233,8 @@ void odr_python::bind_file(py::module_ &m) {
   py::class_<odr::ArchiveFile, odr::DecodedFile>(m, "ArchiveFile")
       .def("archive", &odr::ArchiveFile::archive);
 
+  // no constructor either: `pyodr.open(...).as_document_file()` narrows
   py::class_<odr::DocumentFile, odr::DecodedFile>(m, "DocumentFile")
-      .def(py::init<const odr::File &, const odr::Logger &>(), py::arg("file"),
-           py::arg("logger") = odr::Logger::null())
-      .def(py::init<const std::string &>(), py::arg("path"))
-      .def_static("from_disk", &odr::DocumentFile::from_disk, py::arg("path"),
-                  py::arg("logger") = odr::Logger::null(),
-                  py::call_guard<py::gil_scoped_release>(),
-                  "Decode the document file at `path` on disk.")
-      .def_static(
-          "from_memory",
-          [](const py::bytes &data, const odr::Logger &logger) {
-            // the bytes have to be copied out under the GIL; only the decode
-            // that follows is long-running
-            std::string bytes(data);
-            const py::gil_scoped_release release;
-            return odr::DocumentFile::from_memory(std::move(bytes), logger);
-          },
-          py::arg("data"), py::arg("logger") = odr::Logger::null(),
-          "Decode a document file held in memory; `data` is its bytes.")
       .def("document_type", &odr::DocumentFile::document_type)
       .def("decrypt", &odr::DocumentFile::decrypt, py::arg("password"),
            py::call_guard<py::gil_scoped_release>())