feat!: collapse FileMeta into a single type

docs/design/README.md

@@ -34,11 +34,6 @@
 
 ## Future
 
-### Breaking API changes (next major)
-
-- Collapse `FileMeta` into a single type. The nested `DocumentMeta`
-  does not earn the extra indirection.
-
 ### Document index
 
 - replace iterators where it makes sense with an index

jni/CMakeLists.txt

@@ -58,7 +58,6 @@ add_jar(odr_java
         "java/app/opendocument/core/DirectionalString.java"
         "java/app/opendocument/core/Document.java"
         "java/app/opendocument/core/DocumentFile.java"
-        "java/app/opendocument/core/DocumentMeta.java"
         "java/app/opendocument/core/DocumentPath.java"
         "java/app/opendocument/core/DocumentType.java"
         "java/app/opendocument/core/Element.java"

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

@@ -22,10 +22,6 @@ public DocumentType documentType() {
     return DocumentType.fromNative(documentTypeNative(handle()));
   }
 
-  public DocumentMeta documentMeta() {
-    return documentMetaNative(handle());
-  }
-
   /** Returns a decrypted copy of this file. */
   @Override
   public DocumentFile decrypt(String password) {
@@ -44,8 +40,6 @@ public Document document() {
 
   private native int documentTypeNative(long handle);
 
-  private native DocumentMeta documentMetaNative(long handle);
-
   private native long decryptDocumentFileNative(long handle, String password);
 
   private native long documentNative(long handle);

jni/java/app/opendocument/core/FileMeta.java

@@ -5,13 +5,48 @@ public final class FileMeta {
   public final FileType type;
   public final String mimetype;
   public final boolean passwordEncrypted;
-  /** {@code null} when the file is not a document. */
-  public final DocumentMeta documentMeta;
 
-  FileMeta(int type, String mimetype, boolean passwordEncrypted, DocumentMeta documentMeta) {
+  /** {@code UNKNOWN} when the file is not a document; the fields below are then unset. */
+  public final DocumentType documentType;
+
+  /** Number of entries (pages/slides/sheets); {@code null} if unknown. */
+  public final Long entryCount;
+
+  public final String title;
+  public final String author;
+  public final String subject;
+  public final String keywords;
+  public final String creator;
+  public final String producer;
+  public final String creationDate;
+  public final String modificationDate;
+
+  FileMeta(
+      int type,
+      String mimetype,
+      boolean passwordEncrypted,
+      int documentType,
+      Long entryCount,
+      String title,
+      String author,
+      String subject,
+      String keywords,
+      String creator,
+      String producer,
+      String creationDate,
+      String modificationDate) {
     this.type = FileType.fromNative(type);
     this.mimetype = mimetype;
     this.passwordEncrypted = passwordEncrypted;
-    this.documentMeta = documentMeta;
+    this.documentType = DocumentType.fromNative(documentType);
+    this.entryCount = entryCount;
+    this.title = title;
+    this.author = author;
+    this.subject = subject;
+    this.keywords = keywords;
+    this.creator = creator;
+    this.producer = producer;
+    this.creationDate = creationDate;
+    this.modificationDate = modificationDate;
   }
 }

jni/src/jni_convert.hpp

@@ -33,7 +33,6 @@ jobject make_page_layout(JNIEnv *env, const odr::PageLayout &layout);
 jobject make_table_dimensions(JNIEnv *env,
                               const odr::TableDimensions &dimensions);
 jobject make_table_position(JNIEnv *env, const odr::TablePosition &position);
-jobject make_document_meta(JNIEnv *env, const odr::DocumentMeta &meta);
 jobject make_file_meta(JNIEnv *env, const odr::FileMeta &meta);
 
 jobject html_config_to_java(JNIEnv *env, const odr::HtmlConfig &config);

jni/src/jni_file.cpp

@@ -356,15 +356,6 @@ Java_app_opendocument_core_DocumentFile_documentTypeNative(JNIEnv *env, jobject,
   });
 }
 
-extern "C" JNIEXPORT jobject JNICALL
-Java_app_opendocument_core_DocumentFile_documentMetaNative(JNIEnv *env, jobject,
-                                                           jlong handle) {
-  return guarded(env, [&] {
-    return odr_jni::make_document_meta(
-        env, decoded(handle).as_document_file().document_meta());
-  });
-}
-
 extern "C" JNIEXPORT jlong JNICALL
 Java_app_opendocument_core_DocumentFile_decryptDocumentFileNative(
     JNIEnv *env, jobject, jlong handle, jstring password) {

jni/src/jni_style.cpp

@@ -251,12 +251,15 @@ jobject make_table_position(JNIEnv *env, const odr::TablePosition &position) {
                     static_cast<jint>(position.row));
 }
 
-jobject make_document_meta(JNIEnv *env, const odr::DocumentMeta &meta) {
+jobject make_file_meta(JNIEnv *env, const odr::FileMeta &meta) {
   return new_object(
-      env, "app/opendocument/core/DocumentMeta",
-      "(ILjava/lang/Long;Ljava/lang/String;Ljava/lang/String;"
+      env, "app/opendocument/core/FileMeta",
+      "(ILjava/lang/String;ZILjava/lang/Long;Ljava/lang/String;"
+      "Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;"
       "Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;"
-      "Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V",
+      "Ljava/lang/String;)V",
+      static_cast<jint>(meta.type), to_jstring(env, meta.mimetype),
+      static_cast<jboolean>(meta.password_encrypted),
       static_cast<jint>(meta.document_type), box_long(env, meta.entry_count),
       make_string_opt(env, meta.title), make_string_opt(env, meta.author),
       make_string_opt(env, meta.subject), make_string_opt(env, meta.keywords),
@@ -265,17 +268,6 @@ jobject make_document_meta(JNIEnv *env, const odr::DocumentMeta &meta) {
       make_string_opt(env, meta.modification_date));
 }
 
-jobject make_file_meta(JNIEnv *env, const odr::FileMeta &meta) {
-  jobject document_meta = meta.document_meta.has_value()
-                              ? make_document_meta(env, *meta.document_meta)
-                              : nullptr;
-  return new_object(
-      env, "app/opendocument/core/FileMeta",
-      "(ILjava/lang/String;ZLapp/opendocument/core/DocumentMeta;)V",
-      static_cast<jint>(meta.type), to_jstring(env, meta.mimetype),
-      static_cast<jboolean>(meta.password_encrypted), document_meta);
-}
-
 jobject html_config_to_java(JNIEnv *env, const odr::HtmlConfig &config) {
   jclass cls = env->FindClass("app/opendocument/core/HtmlConfig");
   if (cls == nullptr) {

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

@@ -25,8 +25,7 @@ void openOdt() throws IOException {
 
       DocumentFile documentFile = file.asDocumentFile();
       assertEquals(DocumentType.TEXT, documentFile.documentType());
-      DocumentMeta meta = documentFile.documentMeta();
-      assertEquals(DocumentType.TEXT, meta.documentType);
+      assertEquals(DocumentType.TEXT, documentFile.fileMeta().documentType);
     }
   }
 
@@ -66,7 +65,7 @@ void fileMeta() throws IOException {
       FileMeta meta = file.fileMeta();
       assertEquals(FileType.OPENDOCUMENT_TEXT, meta.type);
       assertFalse(meta.passwordEncrypted);
-      assertNotNull(meta.documentMeta);
+      assertEquals(DocumentType.TEXT, meta.documentType);
     }
   }
 

python/src/bind_file.cpp

@@ -93,28 +93,23 @@ void odr_python::bind_file(py::module_ &m) {
       .def_readwrite("engine_priority",
                      &odr::DecodePreference::engine_priority);
 
-  py::class_<odr::DocumentMeta>(m, "DocumentMeta")
-      .def(py::init<>())
-      .def_readwrite("document_type", &odr::DocumentMeta::document_type)
-      .def_readwrite("entry_count", &odr::DocumentMeta::entry_count)
-      .def_readwrite("title", &odr::DocumentMeta::title)
-      .def_readwrite("author", &odr::DocumentMeta::author)
-      .def_readwrite("subject", &odr::DocumentMeta::subject)
-      .def_readwrite("keywords", &odr::DocumentMeta::keywords)
-      .def_readwrite("creator", &odr::DocumentMeta::creator)
-      .def_readwrite("producer", &odr::DocumentMeta::producer)
-      .def_readwrite("creation_date", &odr::DocumentMeta::creation_date)
-      .def_readwrite("modification_date",
-                     &odr::DocumentMeta::modification_date);
-
   py::class_<odr::FileMeta>(m, "FileMeta")
       .def(py::init<>())
       .def_readwrite("type", &odr::FileMeta::type)
       .def_property_readonly(
           "mimetype",
           [](const odr::FileMeta &meta) { return std::string(meta.mimetype); })
       .def_readwrite("password_encrypted", &odr::FileMeta::password_encrypted)
-      .def_readwrite("document_meta", &odr::FileMeta::document_meta);
+      .def_readwrite("document_type", &odr::FileMeta::document_type)
+      .def_readwrite("entry_count", &odr::FileMeta::entry_count)
+      .def_readwrite("title", &odr::FileMeta::title)
+      .def_readwrite("author", &odr::FileMeta::author)
+      .def_readwrite("subject", &odr::FileMeta::subject)
+      .def_readwrite("keywords", &odr::FileMeta::keywords)
+      .def_readwrite("creator", &odr::FileMeta::creator)
+      .def_readwrite("producer", &odr::FileMeta::producer)
+      .def_readwrite("creation_date", &odr::FileMeta::creation_date)
+      .def_readwrite("modification_date", &odr::FileMeta::modification_date);
 
   py::class_<odr::File>(m, "File")
       .def(py::init<>())
@@ -183,7 +178,6 @@ void odr_python::bind_file(py::module_ &m) {
       .def_static("type_by_path", &odr::DocumentFile::type, py::arg("path"))
       .def_static("meta_by_path", &odr::DocumentFile::meta, py::arg("path"))
       .def("document_type", &odr::DocumentFile::document_type)
-      .def("document_meta", &odr::DocumentFile::document_meta)
       .def("decrypt", &odr::DocumentFile::decrypt, py::arg("password"))
       .def("document", &odr::DocumentFile::document);
 

python/tests/test_document.py

@@ -23,7 +23,7 @@ def test_open_odt(odt_path):
 
 
 def test_document_meta(odt_path):
-    meta = pyodr.open(str(odt_path)).as_document_file().document_meta()
+    meta = pyodr.open(str(odt_path)).as_document_file().file_meta()
     assert meta.document_type == pyodr.DocumentType.text
 
 

python/tests/test_file.py

@@ -58,6 +58,10 @@ def test_file_meta(csv_path):
     meta = file.file_meta()
     assert meta.type == pyodr.FileType.comma_separated_values
     assert not meta.password_encrypted
+    # not a document, so the document fields stay unset
+    assert meta.document_type == pyodr.DocumentType.unknown
+    assert meta.entry_count is None
+    assert meta.title is None
 
 
 def test_open_zip_archive(odt_path):

src/odr/file.cpp

@@ -15,20 +15,6 @@
 
 namespace odr {
 
-DocumentMeta::DocumentMeta() = default;
-
-DocumentMeta::DocumentMeta(const DocumentType document_type,
-                           const std::optional<std::uint32_t> entry_count)
-    : document_type{document_type}, entry_count{entry_count} {}
-
-FileMeta::FileMeta() = default;
-
-FileMeta::FileMeta(const FileType type, const std::string_view mimetype,
-                   const bool password_encrypted,
-                   const std::optional<DocumentMeta> document_meta)
-    : type{type}, mimetype{mimetype}, password_encrypted{password_encrypted},
-      document_meta{document_meta} {}
-
 File::File() = default;
 
 File::File(std::shared_ptr<internal::abstract::File> impl)
@@ -270,10 +256,6 @@ DocumentType DocumentFile::document_type() const {
   return m_impl->document_type();
 }
 
-DocumentMeta DocumentFile::document_meta() const {
-  return m_impl->document_meta();
-}
-
 DocumentFile DocumentFile::decrypt(const std::string &password) const {
   return DecodedFile::decrypt(password).as_document_file();
 }

src/odr/file.hpp

@@ -134,11 +134,14 @@ enum class DocumentType {
   drawing,
 };
 
-/// @brief Meta information about a document.
-struct DocumentMeta final {
-  DocumentMeta();
-  DocumentMeta(DocumentType document_type,
-               std::optional<std::uint32_t> entry_count);
+/// @brief Meta information about a file.
+///
+/// The document fields are only meaningful when @ref document_type is set;
+/// `document_type == DocumentType::unknown` means the file is not a document.
+struct FileMeta final {
+  FileType type{FileType::unknown};
+  std::string_view mimetype;
+  bool password_encrypted{false};
 
   DocumentType document_type{DocumentType::unknown};
   std::optional<std::uint32_t> entry_count;
@@ -155,18 +158,6 @@ struct DocumentMeta final {
   std::optional<std::string> modification_date;
 };
 
-/// @brief Meta information about a file.
-struct FileMeta final {
-  FileMeta();
-  FileMeta(FileType type, std::string_view mimetype, bool password_encrypted,
-           std::optional<DocumentMeta> document_meta);
-
-  FileType type{FileType::unknown};
-  std::string_view mimetype;
-  bool password_encrypted{false};
-  std::optional<DocumentMeta> document_meta;
-};
-
 /// @brief Represents a file.
 class File final {
 public:
@@ -291,7 +282,6 @@ class DocumentFile final : public DecodedFile {
                         const Logger &logger = Logger::null());
 
   [[nodiscard]] DocumentType document_type() const;
-  [[nodiscard]] DocumentMeta document_meta() const;
 
   [[nodiscard]] DocumentFile decrypt(const std::string &password) const;
 

src/odr/internal/abstract/file.hpp

@@ -88,7 +88,6 @@ class DocumentFile : public DecodedFile {
   }
 
   [[nodiscard]] virtual DocumentType document_type() const = 0;
-  [[nodiscard]] virtual DocumentMeta document_meta() const = 0;
 
   [[nodiscard]] virtual std::shared_ptr<Document> document() const = 0;
 };

src/odr/internal/cfb/cfb_file.cpp

@@ -25,7 +25,10 @@ std::string_view CfbFile::mimetype() const noexcept {
 }
 
 FileMeta CfbFile::file_meta() const noexcept {
-  return {file_type(), mimetype(), false, std::nullopt};
+  FileMeta result;
+  result.type = file_type();
+  result.mimetype = mimetype();
+  return result;
 }
 
 bool CfbFile::is_decodable() const noexcept { return true; }

src/odr/internal/csv/csv_file.cpp

@@ -25,7 +25,10 @@ FileType CsvFile::file_type() const noexcept {
 std::string_view CsvFile::mimetype() const noexcept { return "text/csv"; }
 
 FileMeta CsvFile::file_meta() const noexcept {
-  return {file_type(), mimetype(), false, std::nullopt};
+  FileMeta result;
+  result.type = file_type();
+  result.mimetype = mimetype();
+  return result;
 }
 
 bool CsvFile::is_decodable() const noexcept { return false; }