From 461acf998da2d6f5b4eb1d6061cb7fa21ec5bf83 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 26 Jul 2026 13:24:08 +0200 Subject: [PATCH 1/5] Replace the hand-written JNI wrapper with odrcore's own bindings odrcore 5.7.0 ships JNI bindings: a java API under `app.opendocument.core` published to github packages as `app.opendocument:odr-core-java`, and the matching `libodr_jni.so` that the recipe builds behind its `with_jni` option. That makes `src/main/cpp/core_wrapper.cpp` redundant, so the whole `app/src/main/cpp` tree and the CMake build around it are gone. The java artifact carries no native code, so the .so still has to be built and shipped here: `with_jni=True` in conanfile.txt produces it per architecture, and conandeployer.py copies it into `build/conan// jniLibs/` together with the `libc++_shared.so` it links - nothing else pulls the c++ runtime into the apk now that the app compiles no native code of its own. CoreWrapper is gone with the C++ it wrapped. It existed to flatten native failures into integer error codes and mirror them back into exception types; the bindings raise typed OdrExceptions instead. Its remaining job was process-wide state, which moves into CoreLoader: the one-time core setup, the single http server that RawLoader also publishes text files on, and the open document that retranslate edits. LoaderService hands RawLoader the CoreLoader the same way it already does for OnlineLoader, and MetadataLoader calls Odr.mimetype directly. The tmpfile() interposition in tmpfile_hack.cpp goes away with it. It only ever worked because the app linked the core into a library it compiled itself, and libodr_jni.so is a prebuilt. Nothing in the android option set calls tmpfile() anymore either - only wvware did, and it is compiled out - so what is left is core resolving std::filesystem::temp_directory_path(), which falls back to a /tmp that does not exist on android. Setting TMPDIR before the first native call covers it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jT7GGUHvUnDvf8UMcQSLk --- app/CMakeLists.txt | 22 - app/build.gradle | 50 +-- app/conandeployer.py | 56 ++- app/conanfile.txt | 1 + app/proguard-rules.txt | 12 +- .../app/opendocument/droid/test/CoreTest.java | 244 ++++++----- app/src/main/cpp/core_wrapper.cpp | 366 ---------------- app/src/main/cpp/core_wrapper.hpp | 42 -- app/src/main/cpp/tmpfile_hack.cpp | 59 --- app/src/main/cpp/tmpfile_hack.hpp | 12 - .../droid/background/CoreLoader.java | 189 --------- .../droid/background/CoreLoader.kt | 389 ++++++++++++++++++ .../droid/background/CoreWrapper.java | 183 -------- .../droid/background/LoaderService.java | 2 +- .../droid/background/MetadataLoader.java | 4 +- .../droid/background/RawLoader.java | 48 +-- .../droid/background/RawLoaderTest.kt | 2 +- gradle/libs.versions.toml | 8 + settings.gradle | 31 ++ 19 files changed, 642 insertions(+), 1078 deletions(-) delete mode 100644 app/CMakeLists.txt delete mode 100644 app/src/main/cpp/core_wrapper.cpp delete mode 100644 app/src/main/cpp/core_wrapper.hpp delete mode 100644 app/src/main/cpp/tmpfile_hack.cpp delete mode 100644 app/src/main/cpp/tmpfile_hack.hpp delete mode 100644 app/src/main/java/app/opendocument/droid/background/CoreLoader.java create mode 100644 app/src/main/java/app/opendocument/droid/background/CoreLoader.kt delete mode 100644 app/src/main/java/app/opendocument/droid/background/CoreWrapper.java diff --git a/app/CMakeLists.txt b/app/CMakeLists.txt deleted file mode 100644 index 727761674a88..000000000000 --- a/app/CMakeLists.txt +++ /dev/null @@ -1,22 +0,0 @@ -cmake_minimum_required(VERSION 3.18.1) -project(odr-droid CXX) - -set(CMAKE_CXX_STANDARD 20) -set(CMAKE_CXX_STANDARD_REQUIRED ON) -set(CMAKE_CXX_EXTENSIONS OFF) - -find_package(odrcore REQUIRED) - -add_library(tmpfile_hack STATIC - src/main/cpp/tmpfile_hack.cpp) -target_include_directories(tmpfile_hack - PRIVATE src/main/cpp) -target_link_libraries(tmpfile_hack - PRIVATE log) - -add_library(odr-core SHARED - src/main/cpp/core_wrapper.cpp) -target_include_directories(odr-core - PRIVATE src/main/cpp) -target_link_libraries(odr-core - PRIVATE odrcore::odrcore tmpfile_hack log) diff --git a/app/build.gradle b/app/build.gradle index 8d97a06ea2b7..957c85de69e0 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -57,7 +57,8 @@ tasks.register('conanProfile', Copy) { tasks.named("conanInstall-" + architecture) { profile.set('build/conanprofile.txt') deployer.set('conandeployer.py') - deployerFolder.set(outputDirectory.get().asFile.toString() + "/assets/core") + // the deployer lays out both assets/core and jniLibs/ underneath this + deployerFolder.set(outputDirectory.get().asFile.toString()) dependsOn(tasks.named('conanProfile')) conanExecutable.set(conanExecutable_) } @@ -78,17 +79,6 @@ android { // pinned here to keep it from drifting with the namespace. testApplicationId = "at.tomtasche.reader.test" testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" - - externalNativeBuild { - cmake { - targets = ["odr-core"] - arguments( - "-DANDROID_STL=c++_shared", - "-DCMAKE_TOOLCHAIN_FILE=build/conan/android_toolchain.cmake", - "-DCMAKE_BUILD_TYPE=RelWithDebInfo", - ) - } - } } flavorDimensions = ["default"] @@ -152,13 +142,6 @@ android { targetCompatibility = JavaVersion.VERSION_17 } - - externalNativeBuild { - cmake { - version = "3.22.0+" - path = "CMakeLists.txt" - } - } lint { // lint errors are build failures. NewApi in particular is the check that // would have caught the java.nio.file usage below minSdk that shipped for @@ -178,15 +161,6 @@ android { // AGP 9 makes resValue opt-in; the flavors use it for DISABLE_TRACKING resValues = true } - packaging { - jniLibs { - // No need to pickFirst libc++_shared.so if all files are identical. - // They will be identical if NDK major version matches. - // NDK runtime problems may occur if NDK version mismatches, - // so comment this out, to get a compile error instead - // pickFirsts += ['**/libc++_shared.so'] - } - } // the java/kotlin package, R and BuildConfig. deliberately different from // applicationId above: this one is internal and free to rename, that one is not. // testNamespace is left to default to namespace + ".test". @@ -197,9 +171,20 @@ android { // TODO can and should this be architecture dependent? sourceSets.main.assets.srcDirs += "build/conan/armv8/assets" + + // libodr_jni.so (odrcore built with with_jni) plus the libc++_shared.so it links, + // both put there per architecture by conandeployer.py. these used to be produced by + // the app's own externalNativeBuild, which is gone now that the JNI layer comes + // from the core itself + sourceSets.main.jniLibs.srcDirs += + ["armv8", "armv7", "x86", "x86_64"].collect { "build/conan/${it}/jniLibs" } } dependencies { + // the java side of odrcore's JNI bindings. the matching libodr_jni.so is built by + // conan and deployed into jniLibs above, so this artifact is java classes only + implementation libs.odr.core.java + implementation libs.play.services.ads implementation libs.play.review implementation libs.user.messaging.platform @@ -223,12 +208,3 @@ dependencies { debugImplementation libs.espresso.idling.resource implementation libs.androidx.annotation } - -// Without removing .cxx dir on cleanup, double gradle clean is erroring out. -// Before removing this workaround, check if "./gradlew assembleDebug; ./gradlew clean; ./gradlew clean" works -tasks.named("clean") { - def dotCxxDir = layout.projectDirectory.dir(".cxx") - doFirst { - delete dotCxxDir - } -} diff --git a/app/conandeployer.py b/app/conandeployer.py index b91911af577d..1353b3706720 100644 --- a/app/conandeployer.py +++ b/app/conandeployer.py @@ -1,5 +1,18 @@ +import glob +import os import shutil +from conan.errors import ConanException + +# conan architecture -> android abi directory name expected under jniLibs, and the +# ndk toolchain triple the matching libc++_shared.so lives under +ANDROID_ABIS = { + "armv8": ("arm64-v8a", "aarch64-linux-android"), + "armv7": ("armeabi-v7a", "arm-linux-androideabi"), + "x86": ("x86", "i686-linux-android"), + "x86_64": ("x86_64", "x86_64-linux-android"), +} + def deploy(graph, output_folder: str, **kwargs): conanfile = graph.root.conanfile @@ -17,20 +30,55 @@ def deploy(graph, output_folder: str, **kwargs): copytree_kwargs = {"symlinks": symlinks, "dirs_exist_ok": True} + # assets/ and jniLibs/ are separate android source sets, so the deployer writes the + # whole build/conan/ tree rather than a single directory + assets_folder = f"{output_folder}/assets/core" + if "odrcore" in deps: dep = deps["odrcore"] - conanfile.output.info(f"Deploying odrcore to {output_folder}") + conanfile.output.info(f"Deploying odrcore assets to {assets_folder}") shutil.copytree( f"{dep.package_folder}/share", - f"{output_folder}/odrcore", + f"{assets_folder}/odrcore", + # share/java holds odr-core-java.jar, which the app takes from github + # packages instead - shipping it as an asset would only bloat the apk + ignore=shutil.ignore_patterns("java"), **copytree_kwargs, ) if "libmagic" in deps: dep = deps["libmagic"] - conanfile.output.info(f"Deploying libmagic to {output_folder}") + conanfile.output.info(f"Deploying libmagic assets to {assets_folder}") shutil.copytree( f"{dep.package_folder}/res", - f"{output_folder}/libmagic", + f"{assets_folder}/libmagic", **copytree_kwargs, ) + + if arch not in ANDROID_ABIS: + raise ConanException(f"No android abi known for arch {arch}") + abi, triple = ANDROID_ABIS[arch] + jni_libs_folder = f"{output_folder}/jniLibs/{abi}" + os.makedirs(jni_libs_folder, exist_ok=True) + + if "odrcore" in deps: + dep = deps["odrcore"] + # built by the recipe's with_jni option; the java side comes from the + # odr-core-java maven artifact of the very same odrcore version + source = f"{dep.package_folder}/lib/libodr_jni.so" + conanfile.output.info(f"Deploying libodr_jni.so to {jni_libs_folder}") + shutil.copy2(source, f"{jni_libs_folder}/libodr_jni.so") + + # libodr_jni.so links the shared c++ runtime (compiler.libcxx=c++_shared in + # conanprofile.txt). the app no longer builds any native code of its own, so + # nothing else would pull libc++_shared.so into the apk and the library would + # fail to load at runtime with "library libc++_shared.so not found" + ndk_path = conanfile.conf.get("tools.android:ndk_path") + if not ndk_path: + raise ConanException("tools.android:ndk_path is not set, cannot find libc++_shared.so") + pattern = f"{ndk_path}/toolchains/llvm/prebuilt/*/sysroot/usr/lib/{triple}/libc++_shared.so" + matches = glob.glob(pattern) + if not matches: + raise ConanException(f"No libc++_shared.so found at {pattern}") + conanfile.output.info(f"Deploying libc++_shared.so to {jni_libs_folder}") + shutil.copy2(matches[0], f"{jni_libs_folder}/libc++_shared.so") diff --git a/app/conanfile.txt b/app/conanfile.txt index 33ef371237be..2ed8239e7481 100644 --- a/app/conanfile.txt +++ b/app/conanfile.txt @@ -12,4 +12,5 @@ odrcore/*:with_wvWare=False odrcore/*:with_libmagic=True odrcore/*:with_http_server=True odrcore/*:with_cli=False +odrcore/*:with_jni=True odrcore/*:bundle_assets=False diff --git a/app/proguard-rules.txt b/app/proguard-rules.txt index d506f0664926..84abb48992c1 100644 --- a/app/proguard-rules.txt +++ b/app/proguard-rules.txt @@ -22,12 +22,12 @@ -keepnames class at.stefl.** { *; } -# the c++ side looks these up by name through FindClass/GetFieldID in -# src/main/cpp/core_wrapper.cpp, so r8 must not rename or strip them. keep these in sync -# with the java package - a stale rule here only breaks minified builds, where the fields -# get stripped and GetFieldID then returns null at runtime. --keep class app.opendocument.droid.background.CoreWrapper { *; } --keep class app.opendocument.droid.background.CoreWrapper$* { *; } +# libodr_jni.so resolves these by name: FindClass for HtmlConfig, HtmlPage, HtmlResource +# and the Html result holders, GetFieldID for every HtmlConfig field, and the native +# methods are bound by signature. it also constructs the OdrException subclasses when +# translating C++ exceptions. r8 must not rename or strip any of it - the failure only +# shows up in minified builds, at runtime, as a null field id or a NoSuchMethodError. +-keep class app.opendocument.core.** { *; } -keepattributes *Annotation* -keepattributes SourceFile,LineNumberTable diff --git a/app/src/androidTest/java/app/opendocument/droid/test/CoreTest.java b/app/src/androidTest/java/app/opendocument/droid/test/CoreTest.java index f23c887d2d7f..b34e75431e92 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/CoreTest.java +++ b/app/src/androidTest/java/app/opendocument/droid/test/CoreTest.java @@ -2,15 +2,22 @@ import android.content.Context; import android.content.res.AssetManager; +import android.os.Handler; +import android.os.Looper; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.LargeTest; import androidx.test.platform.app.InstrumentationRegistry; -import app.opendocument.droid.background.CoreWrapper; +import app.opendocument.core.OdrException; +import app.opendocument.droid.background.CoreLoader; +import app.opendocument.droid.background.FileLoader; +import app.opendocument.droid.nonfree.AnalyticsManager; +import app.opendocument.droid.nonfree.CrashManager; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.List; import org.junit.After; import org.junit.AfterClass; import org.junit.Assert; @@ -22,7 +29,8 @@ @LargeTest @RunWith(AndroidJUnit4.class) public class CoreTest { - private static Thread serverThread; + private static CoreLoader s_coreLoader; + private File m_testFile; private File m_passwordTestFile; private File m_spreadsheetTestFile; @@ -31,27 +39,24 @@ public class CoreTest { @BeforeClass public static void startServer() throws InterruptedException { Context appCtx = InstrumentationRegistry.getInstrumentation().getTargetContext(); - CoreWrapper.initialize(appCtx); - - // Create server cache directory - File serverCacheDir = new File(appCtx.getCacheDir(), "core/server"); - if (!serverCacheDir.isDirectory()) { - serverCacheDir.mkdirs(); - } - CoreWrapper.createServer(serverCacheDir.getAbsolutePath()); - // Start server in background thread - serverThread = - new Thread( - () -> { - try { - CoreWrapper.listenServer(29665); - } catch (Exception e) { - e.printStackTrace(); - } - }); - serverThread.setDaemon(true); - serverThread.start(); + // the loader owns the core lifecycle now, so the test drives a real one rather than a + // set of statics. nothing here goes through loadAsync, so both handlers can be the main + // looper and the listener is never called back + Handler handler = new Handler(Looper.getMainLooper()); + s_coreLoader = new CoreLoader(appCtx, true); + s_coreLoader.initialize( + new FileLoader.FileLoaderListener() { + @Override + public void onSuccess(FileLoader.Result result) {} + + @Override + public void onError(FileLoader.Result result, Throwable throwable) {} + }, + handler, + handler, + new AnalyticsManager(), + new CrashManager()); // Give server time to start Thread.sleep(1000); @@ -59,17 +64,12 @@ public static void startServer() throws InterruptedException { @AfterClass public static void stopServer() { - CoreWrapper.stopServer(); - if (serverThread != null) { - serverThread.interrupt(); + if (s_coreLoader != null) { + s_coreLoader.close(); + s_coreLoader = null; } } - @Before - public void initializeCore() { - // Server is already initialized in @BeforeClass - } - @Before public void extractTestFile() throws IOException { Context appCtx = InstrumentationRegistry.getInstrumentation().getTargetContext(); @@ -120,142 +120,128 @@ private static void copy(InputStream src, File dst) throws IOException { } } + private static File cacheDir() { + return InstrumentationRegistry.getInstrumentation().getTargetContext().getCacheDir(); + } + @Test public void test() { - File cacheDir = - InstrumentationRegistry.getInstrumentation().getTargetContext().getCacheDir(); - File outputPath = new File(cacheDir, "core_output"); - File cachePath = new File(cacheDir, "core_cache"); - - CoreWrapper.CoreOptions coreOptions = new CoreWrapper.CoreOptions(); - coreOptions.inputPath = m_testFile.getAbsolutePath(); - coreOptions.outputPath = outputPath.getPath(); - coreOptions.editable = true; - coreOptions.cachePath = cachePath.getPath(); - - CoreWrapper.CoreResult coreResult = CoreWrapper.hostFile("test", coreOptions); - Assert.assertEquals(0, coreResult.errorCode); - - File resultFile = new File(cacheDir, "result"); - coreOptions.outputPath = resultFile.getPath(); + File cachePath = new File(cacheDir(), "core_cache"); + + List views = + s_coreLoader.host( + "test", + m_testFile.getAbsolutePath(), + cachePath.getPath(), + null, + true, + false, + true); + Assert.assertFalse("hosting the ODT file should produce a view", views.isEmpty()); String htmlDiff = "{\"modifiedText\":{\"/child:1/child:0\":\"This is a simple testoooo document to" + " demonstrate the DocumentLoader example!\",\"/child:3/child:0\":\"This is a" + " simple testaaaa document to demonstrate the DocumentLoader example!\"}}"; - CoreWrapper.CoreResult result = CoreWrapper.backtranslate(coreOptions, htmlDiff); - Assert.assertEquals(0, result.errorCode); + File result = s_coreLoader.edit(htmlDiff, new File(cacheDir(), "result").getPath()); + Assert.assertTrue("the edited document should have been saved", result.isFile()); } @Test public void testDocxEdit() { - File cacheDir = - InstrumentationRegistry.getInstrumentation().getTargetContext().getCacheDir(); - File outputPath = new File(cacheDir, "core_output_docx"); - File cachePath = new File(cacheDir, "core_cache"); - - CoreWrapper.CoreOptions coreOptions = new CoreWrapper.CoreOptions(); - coreOptions.inputPath = m_docxTestFile.getAbsolutePath(); - coreOptions.outputPath = outputPath.getPath(); - coreOptions.editable = true; - coreOptions.cachePath = cachePath.getPath(); - - CoreWrapper.CoreResult coreResult = CoreWrapper.hostFile("docx-edit", coreOptions); - Assert.assertEquals(0, coreResult.errorCode); - - File resultFile = new File(cacheDir, "result_docx"); - coreOptions.outputPath = resultFile.getPath(); + File cachePath = new File(cacheDir(), "core_cache"); + + List views = + s_coreLoader.host( + "docx-edit", + m_docxTestFile.getAbsolutePath(), + cachePath.getPath(), + null, + true, + false, + true); + Assert.assertFalse("hosting the DOCX file should produce a view", views.isEmpty()); String htmlDiff = "{\"modifiedText\":{\"/child:16/child:0/child:0\":\"Outasdfsdafdline\",\"/child:24/child:0/child:0\":\"Colorasdfasdfasdfed" + " Line\",\"/child:6/child:0/child:0\":\"Text hello world!\"}}"; - CoreWrapper.CoreResult result = CoreWrapper.backtranslate(coreOptions, htmlDiff); - Assert.assertEquals(0, result.errorCode); + File result = s_coreLoader.edit(htmlDiff, new File(cacheDir(), "result_docx").getPath()); + Assert.assertTrue("the edited document should have been saved", result.isFile()); } @Test public void testPasswordProtectedDocumentWithoutPassword() { - File cacheDir = - InstrumentationRegistry.getInstrumentation().getTargetContext().getCacheDir(); - File outputDir = new File(cacheDir, "output_password_test"); - File cachePath = new File(cacheDir, "core_cache"); - - CoreWrapper.CoreOptions coreOptions = new CoreWrapper.CoreOptions(); - coreOptions.inputPath = m_passwordTestFile.getAbsolutePath(); - coreOptions.outputPath = outputDir.getPath(); - coreOptions.editable = false; - coreOptions.cachePath = cachePath.getPath(); - - CoreWrapper.CoreResult coreResult = - CoreWrapper.hostFile("password-test-no-pw", coreOptions); - Assert.assertEquals(-2, coreResult.errorCode); + File cachePath = new File(cacheDir(), "core_cache"); + + Assert.assertThrows( + OdrException.FileEncrypted.class, + () -> + s_coreLoader.host( + "password-test-no-pw", + m_passwordTestFile.getAbsolutePath(), + cachePath.getPath(), + null, + false, + false, + false)); } @Test public void testPasswordProtectedDocumentWithWrongPassword() { - File cacheDir = - InstrumentationRegistry.getInstrumentation().getTargetContext().getCacheDir(); - File outputDir = new File(cacheDir, "output_password_test"); - File cachePath = new File(cacheDir, "core_cache"); - - CoreWrapper.CoreOptions coreOptions = new CoreWrapper.CoreOptions(); - coreOptions.inputPath = m_passwordTestFile.getAbsolutePath(); - coreOptions.outputPath = outputDir.getPath(); - coreOptions.password = "wrongpassword"; - coreOptions.editable = false; - coreOptions.cachePath = cachePath.getPath(); - - CoreWrapper.CoreResult coreResult = - CoreWrapper.hostFile("password-test-wrong-pw", coreOptions); - Assert.assertEquals(-2, coreResult.errorCode); + File cachePath = new File(cacheDir(), "core_cache"); + + Assert.assertThrows( + OdrException.class, + () -> + s_coreLoader.host( + "password-test-wrong-pw", + m_passwordTestFile.getAbsolutePath(), + cachePath.getPath(), + "wrongpassword", + false, + false, + false)); } @Test public void testPasswordProtectedDocumentWithCorrectPassword() { - File cacheDir = - InstrumentationRegistry.getInstrumentation().getTargetContext().getCacheDir(); - File outputDir = new File(cacheDir, "output_password_test"); - File cachePath = new File(cacheDir, "core_cache"); - - CoreWrapper.CoreOptions coreOptions = new CoreWrapper.CoreOptions(); - coreOptions.inputPath = m_passwordTestFile.getAbsolutePath(); - coreOptions.outputPath = outputDir.getPath(); - coreOptions.password = "passwort"; - coreOptions.editable = false; - coreOptions.cachePath = cachePath.getPath(); - - CoreWrapper.CoreResult coreResult = - CoreWrapper.hostFile("password-test-correct-pw", coreOptions); - Assert.assertEquals(0, coreResult.errorCode); + File cachePath = new File(cacheDir(), "core_cache"); + + List views = + s_coreLoader.host( + "password-test-correct-pw", + m_passwordTestFile.getAbsolutePath(), + cachePath.getPath(), + "passwort", + false, + false, + false); + Assert.assertFalse("the decrypted document should produce a view", views.isEmpty()); } @Test public void testSpreadsheetSheetNames() { - File cacheDir = - InstrumentationRegistry.getInstrumentation().getTargetContext().getCacheDir(); - File outputPath = new File(cacheDir, "spreadsheet_output"); - File cachePath = new File(cacheDir, "spreadsheet_cache"); - - CoreWrapper.CoreOptions coreOptions = new CoreWrapper.CoreOptions(); - coreOptions.inputPath = m_spreadsheetTestFile.getAbsolutePath(); - coreOptions.outputPath = outputPath.getPath(); - coreOptions.editable = false; - coreOptions.cachePath = cachePath.getPath(); - - CoreWrapper.CoreResult coreResult = CoreWrapper.hostFile("spreadsheet-test", coreOptions); - Assert.assertEquals( - "CoreWrapper should successfully parse the ODS file", 0, coreResult.errorCode); + File cachePath = new File(cacheDir(), "spreadsheet_cache"); + + List views = + s_coreLoader.host( + "spreadsheet-test", + m_spreadsheetTestFile.getAbsolutePath(), + cachePath.getPath(), + null, + false, + false, + false); // Verify we have exactly 3 sheets - Assert.assertEquals("ODS file should contain 3 sheets", 3, coreResult.pageNames.size()); + Assert.assertEquals("ODS file should contain 3 sheets", 3, views.size()); // Verify sheet names match the actual sheet names from the ODS file + Assert.assertEquals("First sheet should be named 'hey'", "hey", views.get(0).getName()); + Assert.assertEquals("Second sheet should be named 'ho'", "ho", views.get(1).getName()); Assert.assertEquals( - "First sheet should be named 'hey'", "hey", coreResult.pageNames.get(0)); - Assert.assertEquals("Second sheet should be named 'ho'", "ho", coreResult.pageNames.get(1)); - Assert.assertEquals( - "Third sheet should be named 'Sheet3'", "Sheet3", coreResult.pageNames.get(2)); + "Third sheet should be named 'Sheet3'", "Sheet3", views.get(2).getName()); } } diff --git a/app/src/main/cpp/core_wrapper.cpp b/app/src/main/cpp/core_wrapper.cpp deleted file mode 100644 index 84c4aee40b30..000000000000 --- a/app/src/main/cpp/core_wrapper.cpp +++ /dev/null @@ -1,366 +0,0 @@ -#include "core_wrapper.hpp" - -#include "tmpfile_hack.hpp" - -#include -#include -#include -#include -#include -#include -#include -#include - -#include - -#include -#include -#include -#include - -namespace { - - std::string convertString(JNIEnv *env, jstring string) { - jboolean isCopy; - const char *cstring = env->GetStringUTFChars(string, &isCopy); - auto cppstring = std::string(cstring, env->GetStringUTFLength(string)); - env->ReleaseStringUTFChars(string, cstring); - return cppstring; - } - - std::string getStringField(JNIEnv *env, jclass clazz, jobject object, const char *name) { - jfieldID field = env->GetFieldID(clazz, name, "Ljava/lang/String;"); - auto string = (jstring) env->GetObjectField(object, field); - return convertString(env, string); - } - - std::string getStringField(JNIEnv *env, jobject object, const char *name) { - jclass clazz = env->GetObjectClass(object); - return getStringField(env, clazz, object, name); - } - - class AndroidLogger final : public odr::Logger { - public: - static int to_android_log_level(odr::LogLevel level) { - switch (level) { - case odr::LogLevel::verbose: - return ANDROID_LOG_VERBOSE; - case odr::LogLevel::debug: - return ANDROID_LOG_DEBUG; - case odr::LogLevel::info: - return ANDROID_LOG_INFO; - case odr::LogLevel::warning: - return ANDROID_LOG_WARN; - case odr::LogLevel::error: - return ANDROID_LOG_ERROR; - case odr::LogLevel::fatal: - return ANDROID_LOG_FATAL; - default: - return ANDROID_LOG_UNKNOWN; - } - } - - void flush() override {} - - [[nodiscard]] bool will_log(odr::LogLevel level) const override { - return true; - } - - protected: - void log_impl(Time time, odr::LogLevel level, const std::string &message, - const std::source_location &location) const override { - __android_log_print(to_android_log_level(level), "smn", "%s", message.c_str()); - } - - private: - - }; - -} - -std::optional s_document; - -JNIEXPORT jstring JNICALL -Java_app_opendocument_droid_background_CoreWrapper_versionStringNative(JNIEnv *env, jclass clazz) { - std::string versionString = "odrcore " + odr::identify(); - return env->NewStringUTF(versionString.c_str()); -} - -JNIEXPORT void JNICALL -Java_app_opendocument_droid_background_CoreWrapper_setGlobalParams(JNIEnv *env, jclass clazz, - jobject params) { - jclass paramsClass = env->GetObjectClass(params); - - std::string odrCoreDataPath = getStringField(env, paramsClass, params, "coreDataPath"); - std::string libmagicDatabasePath = getStringField(env, paramsClass, params, "libmagicDatabasePath"); - std::string customTmpfilePath = getStringField(env, paramsClass, params, "customTmpfilePath"); - - odr::GlobalParams::set_odr_core_data_path(odrCoreDataPath); - odr::GlobalParams::set_libmagic_database_path(libmagicDatabasePath); - - tmpfile_hack::set_tmpfile_directory(customTmpfilePath); -} - -JNIEXPORT jstring JNICALL -Java_app_opendocument_droid_background_CoreWrapper_mimetypeNative(JNIEnv *env, jclass clazz, jstring path) { - auto logger = std::make_shared(); - - std::string pathCpp = convertString(env, path); - std::string mimetypeCpp = std::string(odr::mimetype(pathCpp)); - jstring mimetype = env->NewStringUTF(mimetypeCpp.c_str()); - - return mimetype; -} - -JNIEXPORT jobject JNICALL -Java_app_opendocument_droid_background_CoreWrapper_backtranslateNative(JNIEnv *env, jclass clazz, - jobject options, - jstring htmlDiff) { - jclass resultClass = env->FindClass("app/opendocument/droid/background/CoreWrapper$CoreResult"); - jmethodID resultConstructor = env->GetMethodID(resultClass, "", "()V"); - jobject result = env->NewObject(resultClass, resultConstructor); - - jfieldID errorField = env->GetFieldID(resultClass, "errorCode", "I"); - - if (!s_document.has_value()) { - env->SetIntField(result, errorField, -1); - return result; - } - - try { - std::string outputPathPrefixCpp = getStringField(env, options, "outputPath"); - - jboolean isCopy; - const auto htmlDiffC = env->GetStringUTFChars(htmlDiff, &isCopy); - - const auto extension = odr::file_type_to_string(s_document->file_type()); - const auto outputPathCpp = outputPathPrefixCpp + "." + extension; - const char *outputPathC = outputPathCpp.c_str(); - jstring outputPath = env->NewStringUTF(outputPathC); - - jfieldID outputPathField = env->GetFieldID(resultClass, "outputPath", "Ljava/lang/String;"); - env->SetObjectField(result, outputPathField, outputPath); - - __android_log_print(ANDROID_LOG_DEBUG, "smn", "HTML diff: %s", htmlDiffC); - - try { - odr::html::edit(*s_document, htmlDiffC); - - env->ReleaseStringUTFChars(htmlDiff, htmlDiffC); - } catch (const std::exception &e) { - env->ReleaseStringUTFChars(htmlDiff, htmlDiffC); - - __android_log_print(ANDROID_LOG_ERROR, "smn", "Failed to edit document: %s", e.what()); - env->SetIntField(result, errorField, -6); - return result; - } catch (...) { - env->ReleaseStringUTFChars(htmlDiff, htmlDiffC); - - __android_log_print(ANDROID_LOG_ERROR, "smn", "Failed to edit document: unknown exception"); - env->SetIntField(result, errorField, -6); - return result; - } - - try { - s_document->save(outputPathCpp); - } catch (...) { - env->SetIntField(result, errorField, -7); - return result; - } - } catch (...) { - env->SetIntField(result, errorField, -3); - return result; - } - - env->SetIntField(result, errorField, 0); - return result; -} - -JNIEXPORT void JNICALL -Java_app_opendocument_droid_background_CoreWrapper_closeNative(JNIEnv *env, jclass clazz, - jobject options) { - s_document.reset(); -} - -std::optional s_server; - -JNIEXPORT void JNICALL -Java_app_opendocument_droid_background_CoreWrapper_createServerNative(JNIEnv *env, jclass clazz, - jstring cachePath) { - __android_log_print(ANDROID_LOG_INFO, "smn", "create server"); - - std::string cachePathCpp = convertString(env, cachePath); - - std::filesystem::create_directories(cachePathCpp); - - odr::HttpServer::Config config; - config.cache_path = cachePathCpp; - s_server = odr::HttpServer(config); -} - -JNIEXPORT jobject JNICALL -Java_app_opendocument_droid_background_CoreWrapper_hostFileNative(JNIEnv *env, jclass clazz, - jstring prefix, - jobject options) { - __android_log_print(ANDROID_LOG_INFO, "smn", "host file"); - - std::error_code ec; - auto logger = std::make_shared(); - - jclass resultClass = env->FindClass("app/opendocument/droid/background/CoreWrapper$CoreResult"); - jmethodID resultConstructor = env->GetMethodID(resultClass, "", "()V"); - jobject result = env->NewObject(resultClass, resultConstructor); - - jfieldID errorField = env->GetFieldID(resultClass, "errorCode", "I"); - - if (!s_server.has_value()) { - env->SetIntField(result, errorField, -1); - return result; - } - - try { - s_server->clear(); - - jclass optionsClass = env->GetObjectClass(options); - - std::optional passwordCpp; - jfieldID passwordField = env->GetFieldID(optionsClass, "password", "Ljava/lang/String;"); - auto password = (jstring) env->GetObjectField(options, passwordField); - if (password != nullptr) { - passwordCpp = convertString(env, password); - } - - jfieldID pagingField = env->GetFieldID(optionsClass, "paging", "Z"); - jboolean paging = env->GetBooleanField(options, pagingField); - - jfieldID editableField = env->GetFieldID(optionsClass, "editable", "Z"); - jboolean editable = env->GetBooleanField(options, editableField); - - std::string outputPathCpp = getStringField(env, optionsClass, options, "outputPath"); - std::string cachePathCpp = getStringField(env, optionsClass, options, "cachePath"); - - jclass listClass = env->FindClass("java/util/List"); - jmethodID addMethod = env->GetMethodID(listClass, "add", "(Ljava/lang/Object;)Z"); - - jfieldID pageNamesField = env->GetFieldID(resultClass, "pageNames", "Ljava/util/List;"); - auto pageNames = (jobject) env->GetObjectField(result, pageNamesField); - - jfieldID pagePathsField = env->GetFieldID(resultClass, "pagePaths", "Ljava/util/List;"); - auto pagePaths = (jobject) env->GetObjectField(result, pagePathsField); - - std::string inputPathCpp = getStringField(env, options, "inputPath"); - std::string prefixCpp = convertString(env, prefix); - - try { - odr::DecodePreference decodePreference; - decodePreference.engine_priority = {odr::DecoderEngine::odr}; - odr::DecodedFile file = odr::open(inputPathCpp, decodePreference, *logger); - - if (file.password_encrypted()) { - if (!passwordCpp.has_value()) { - env->SetIntField(result, errorField, -2); - return result; - } - try { - file = file.decrypt(passwordCpp.value()); - } catch (...) { - env->SetIntField(result, errorField, -2); - return result; - } - } - - __android_log_print(ANDROID_LOG_INFO, "smn", "type=%s", - file_type_to_string(file.file_type()).c_str()); - - // .doc-files are not real documents in core - if (file.is_document_file() && file.file_type() != odr::FileType::legacy_word_document) { - // TODO this will cause a second load - s_document = file.as_document_file().document(); - } - - odr::HtmlConfig htmlConfig; - htmlConfig.embed_images = false; - htmlConfig.embed_shipped_resources = true; - htmlConfig.relative_resource_paths = false; - htmlConfig.text_document_margin = paging; - htmlConfig.editable = editable; - - std::filesystem::remove_all(cachePathCpp, ec); - std::filesystem::create_directories(cachePathCpp); - odr::HtmlService service = odr::html::translate(file, cachePathCpp, htmlConfig, logger); - s_server->connect_service(service, prefixCpp); - odr::HtmlViews htmlViews = service.list_views(); - - // spreadsheets show one tab per sheet; every other format only shows - // the full "document" view without tabs (if the service provides one - - // e.g. plain text and image files only have a single differently named view) - bool isSpreadsheet = file.is_document_file() && - (file.as_document_file().document_type() == - odr::DocumentType::spreadsheet); - bool hasDocumentView = std::any_of( - htmlViews.begin(), htmlViews.end(), - [](const odr::HtmlView &view) { return view.name() == "document"; }); - - for (const auto &view: htmlViews) { - __android_log_print(ANDROID_LOG_INFO, "smn", "view name=%s path=%s", - view.name().c_str(), view.path().c_str()); - if (isSpreadsheet) { - if (view.name() == "document") { - continue; - } - } else if (hasDocumentView && (view.name() != "document")) { - continue; - } - - jstring pageName = env->NewStringUTF(view.name().c_str()); - env->CallBooleanMethod(pageNames, addMethod, pageName); - - std::string pagePathCpp = - "http://localhost:29665/file/" + prefixCpp + "/" + view.path(); - jstring pagePath = env->NewStringUTF(pagePathCpp.c_str()); - env->CallBooleanMethod(pagePaths, addMethod, pagePath); - } - } catch (const odr::UnknownFileType &e) { - __android_log_print(ANDROID_LOG_ERROR, "smn", "Unknown file type: %s", e.what()); - env->SetIntField(result, errorField, -5); - return result; - } catch (const odr::UnsupportedFileType &e) { - __android_log_print(ANDROID_LOG_ERROR, "smn", "Unsupported file type: %s", e.what()); - env->SetIntField(result, errorField, -5); - return result; - } catch (const std::exception &e) { - __android_log_print(ANDROID_LOG_ERROR, "smn", "Unhandled C++ exception: %s", e.what()); - env->SetIntField(result, errorField, -4); - return result; - } catch (...) { - __android_log_print(ANDROID_LOG_ERROR, "smn", - "Unhandled C++ exception without further information"); - env->SetIntField(result, errorField, -4); - return result; - } - } catch (...) { - env->SetIntField(result, errorField, -3); - return result; - } - - env->SetIntField(result, errorField, 0); - return result; -} - -JNIEXPORT void JNICALL -Java_app_opendocument_droid_background_CoreWrapper_listenServerNative(JNIEnv *env, jclass clazz, - jint port) { - __android_log_print(ANDROID_LOG_INFO, "smn", "listen ..."); - - s_server->listen("127.0.0.1", port); - - __android_log_print(ANDROID_LOG_INFO, "smn", "done listening"); -} - -JNIEXPORT void JNICALL -Java_app_opendocument_droid_background_CoreWrapper_stopServerNative(JNIEnv *env, jclass clazz) { - __android_log_print(ANDROID_LOG_INFO, "smn", "stop server"); - - s_server->stop(); - s_server.reset(); -} diff --git a/app/src/main/cpp/core_wrapper.hpp b/app/src/main/cpp/core_wrapper.hpp deleted file mode 100644 index e0b36b6b8205..000000000000 --- a/app/src/main/cpp/core_wrapper.hpp +++ /dev/null @@ -1,42 +0,0 @@ -#pragma once - -#include - -extern "C" { - -JNIEXPORT jstring JNICALL -Java_app_opendocument_droid_background_CoreWrapper_versionStringNative(JNIEnv *env, jclass clazz); - -JNIEXPORT void JNICALL -Java_app_opendocument_droid_background_CoreWrapper_setGlobalParams(JNIEnv *env, jclass clazz, - jobject params); - -JNIEXPORT jstring JNICALL -Java_app_opendocument_droid_background_CoreWrapper_mimetypeNative(JNIEnv *env, jclass clazz, - jstring path); - -JNIEXPORT jobject JNICALL -Java_app_opendocument_droid_background_CoreWrapper_backtranslateNative(JNIEnv *env, jclass clazz, - jobject options, - jstring htmlDiff); - -JNIEXPORT void JNICALL -Java_app_opendocument_droid_background_CoreWrapper_closeNative(JNIEnv *env, jclass clazz, - jobject options); - -JNIEXPORT void JNICALL -Java_app_opendocument_droid_background_CoreWrapper_createServerNative(JNIEnv *env, jclass clazz, - jstring outputPath); - -JNIEXPORT jobject JNICALL -Java_app_opendocument_droid_background_CoreWrapper_hostFileNative(JNIEnv *env, jclass clazz, - jstring prefix, jobject options); - -JNIEXPORT void JNICALL -Java_app_opendocument_droid_background_CoreWrapper_listenServerNative(JNIEnv *env, jclass clazz, - jint port); - -JNIEXPORT void JNICALL -Java_app_opendocument_droid_background_CoreWrapper_stopServerNative(JNIEnv *env, jclass clazz); - -} diff --git a/app/src/main/cpp/tmpfile_hack.cpp b/app/src/main/cpp/tmpfile_hack.cpp deleted file mode 100644 index b01e0a565640..000000000000 --- a/app/src/main/cpp/tmpfile_hack.cpp +++ /dev/null @@ -1,59 +0,0 @@ -#include "tmpfile_hack.hpp" - -#include -#include -#include - -#include - -#include -#include - -static constexpr std::string_view s_filename_template = "tmpfile-XXXXXX"; -static constexpr std::string_view s_default_directory = "/data/local/tmp"; - -static std::optional s_tmpfile_directory; - -std::string tmpfile_hack::get_tmpfile_directory() { - if (s_tmpfile_directory.has_value()) { - return s_tmpfile_directory.value(); - } - - if (const char *tmpfile_directory = std::getenv("TMPDIR"); tmpfile_directory != nullptr) { - return tmpfile_directory; - } - - return std::string(s_default_directory); -} - -void tmpfile_hack::set_tmpfile_directory(std::string_view tmpfile_dir) { - s_tmpfile_directory = std::string(tmpfile_dir); -} - -extern "C" { - -extern FILE *tmpfile() { - std::string tmpfile_path = - tmpfile_hack::get_tmpfile_directory() + "/" + std::string(s_filename_template); - - int descriptor = mkstemp(tmpfile_path.data()); - if (descriptor == -1) { - __android_log_print(ANDROID_LOG_ERROR, "tmpfile_hack", - "Failed to create temporary file: %s", tmpfile_path.c_str()); - return nullptr; - } - __android_log_print(ANDROID_LOG_VERBOSE, "tmpfile_hack", "Temporary file created: %s", - tmpfile_path.c_str()); - - FILE *handle = fdopen(descriptor, "w+b"); - unlink(tmpfile_path.c_str()); - - if (handle == nullptr) { - close(descriptor); - __android_log_print(ANDROID_LOG_ERROR, "tmpfile_hack", "Failed to open temporary file: %s", - tmpfile_path.c_str()); - } - return handle; -} - -} diff --git a/app/src/main/cpp/tmpfile_hack.hpp b/app/src/main/cpp/tmpfile_hack.hpp deleted file mode 100644 index cf15d3756569..000000000000 --- a/app/src/main/cpp/tmpfile_hack.hpp +++ /dev/null @@ -1,12 +0,0 @@ -#pragma once - -#include -#include - -namespace tmpfile_hack { - - std::string get_tmpfile_directory(); - - void set_tmpfile_directory(std::string_view tmpfile_dir); - -} diff --git a/app/src/main/java/app/opendocument/droid/background/CoreLoader.java b/app/src/main/java/app/opendocument/droid/background/CoreLoader.java deleted file mode 100644 index 69115a549a9e..000000000000 --- a/app/src/main/java/app/opendocument/droid/background/CoreLoader.java +++ /dev/null @@ -1,189 +0,0 @@ -package app.opendocument.droid.background; - -import android.content.Context; -import android.net.Uri; -import android.os.Handler; -import android.util.Log; -import app.opendocument.droid.nonfree.AnalyticsManager; -import app.opendocument.droid.nonfree.CrashManager; -import java.io.File; - -public class CoreLoader extends FileLoader { - - /** - * Whether odrcore renders text documents with page margins. This used to be read from the - * "use_paging" remote config key, but that resolved to false for every user since firebase - * remote config was removed - the ConfigManager left behind is a stub without a backing store. - * Kept as an explicit constant so the shipped behavior is visible instead of hidden behind a - * lookup that cannot return a value. - */ - private static final boolean USE_PAGING = false; - - private CoreWrapper.CoreOptions lastCoreOptions; - - private final boolean doOoxml; - - private Thread httpThread; - - public CoreLoader(Context context, boolean doOoxml) { - super(context, LoaderType.CORE); - - this.doOoxml = doOoxml; - } - - @Override - public void initialize( - FileLoaderListener listener, - Handler mainHandler, - Handler backgroundHandler, - AnalyticsManager analyticsManager, - CrashManager crashManager) { - // loads the native library and extracts the core assets. Kept out of the - // constructor so that constructing a CoreLoader stays side effect free - - // LoaderService calls initialize() right after new CoreLoader() anyway. - CoreWrapper.initialize(context); - - File serverCacheDir = new File(context.getCacheDir(), "core/server"); - if (!serverCacheDir.isDirectory() && !serverCacheDir.mkdirs()) { - Log.e( - "CoreLoader", - "Failed to create cache directory for CoreWrapper server: " - + serverCacheDir.getAbsolutePath()); - } - CoreWrapper.createServer(serverCacheDir.getAbsolutePath()); - - httpThread = - new Thread( - () -> { - try { - CoreWrapper.listenServer(29665); - } catch (Throwable e) { - crashManager.log(e); - } - }); - httpThread.start(); - - super.initialize(listener, mainHandler, backgroundHandler, analyticsManager, crashManager); - } - - @Override - public boolean isSupported(Options options) { - return options.fileType.startsWith("application/vnd.oasis.opendocument") - || options.fileType.startsWith("application/x-vnd.oasis.opendocument") - || options.fileType.startsWith("application/vnd.oasis.opendocument.text-master") - || options.fileType.startsWith("application/msword") - || (this.doOoxml - && (options.fileType.startsWith( - "application/vnd.openxmlformats-officedocument.wordprocessingml.document") - || options.fileType.startsWith( - "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") - // TODO: enable pptx too - // options.fileType.startsWith("application/vnd.openxmlformats-officedocument.presentationml.presentation"); - )); - } - - @Override - public void loadSync(Options options) { - final Result result = new Result(); - result.options = options; - result.loaderType = type; - - try { - translate(options, result); - - callOnSuccess(result); - } catch (Throwable e) { - if (e instanceof CoreWrapper.CoreEncryptedException) { - e = new EncryptedDocumentException(); - } - - callOnError(result, e); - } - } - - private void translate(Options options, Result result) throws Exception { - File cachedFile = AndroidFileCache.getCacheFile(context, options.cacheUri); - - File cacheDirectory = AndroidFileCache.getCacheDirectory(cachedFile); - - File coreOutputDirectory = new File(cacheDirectory, "core_output"); - File coreCacheDirectory = new File(cacheDirectory, "core_cache"); - - CoreWrapper.CoreOptions coreOptions = new CoreWrapper.CoreOptions(); - coreOptions.inputPath = cachedFile.getPath(); - coreOptions.outputPath = coreOutputDirectory.getPath(); - coreOptions.cachePath = coreCacheDirectory.getPath(); - coreOptions.password = options.password; - coreOptions.editable = options.translatable; - coreOptions.ooxml = doOoxml; - coreOptions.txt = false; - coreOptions.pdf = false; - - coreOptions.paging = USE_PAGING; - - lastCoreOptions = coreOptions; - - CoreWrapper.CoreResult coreResult = CoreWrapper.hostFile("odr", coreOptions); - - if (coreResult.exception != null) { - throw coreResult.exception; - } - - for (int i = 0; i < coreResult.pagePaths.size(); i++) { - result.partTitles.add(coreResult.pageNames.get(i)); - result.partUris.add(Uri.parse(coreResult.pagePaths.get(i))); - } - } - - @Override - public File retranslate(Options options, String htmlDiff) { - if (lastCoreOptions == null) { - // necessary if fragment was destroyed in the meanwhile - meaning the Loader is - // reinstantiated - - Result result = new Result(); - result.options = options; - - try { - translate(options, result); - } catch (Exception e) { - crashManager.log(e); - - return null; - } - } - - File inputFile = new File(lastCoreOptions.inputPath); - File inputCacheDirectory = AndroidFileCache.getCacheDirectory(inputFile); - File tempFilePrefix = new File(inputCacheDirectory, "retranslate"); - - lastCoreOptions.outputPath = tempFilePrefix.getPath(); - - try { - CoreWrapper.CoreResult result = CoreWrapper.backtranslate(lastCoreOptions, htmlDiff); - - return new File(result.outputPath); - } catch (Throwable e) { - crashManager.log(e); - - return null; - } - } - - @Override - public void close() { - super.close(); - - if (httpThread != null) { - CoreWrapper.stopServer(); - try { - httpThread.join(1000); - } catch (InterruptedException e) { - crashManager.log(e); - } - httpThread = null; - } - - CoreWrapper.close(); - } -} diff --git a/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt b/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt new file mode 100644 index 000000000000..95c8f9b59749 --- /dev/null +++ b/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt @@ -0,0 +1,389 @@ +package app.opendocument.droid.background + +import android.content.Context +import android.net.Uri +import android.os.Handler +import android.system.Os +import android.util.Log +import app.opendocument.core.DecodePreference +import app.opendocument.core.DecodedFile +import app.opendocument.core.DecoderEngine +import app.opendocument.core.Document +import app.opendocument.core.DocumentType +import app.opendocument.core.FileType +import app.opendocument.core.GlobalParams +import app.opendocument.core.Html +import app.opendocument.core.HtmlConfig +import app.opendocument.core.HtmlView +import app.opendocument.core.HttpServer +import app.opendocument.core.Odr +import app.opendocument.core.OdrException +import app.opendocument.droid.background.FileLoader.EncryptedDocumentException +import app.opendocument.droid.background.FileLoader.FileLoaderListener +import app.opendocument.droid.background.FileLoader.LoaderType +import app.opendocument.droid.background.FileLoader.Options +import app.opendocument.droid.background.FileLoader.Result +import app.opendocument.droid.nonfree.AnalyticsManager +import app.opendocument.droid.nonfree.CrashManager +import com.viliussutkus89.android.assetextractor.AssetExtractor +import java.io.File + +/** + * Loads documents through odrcore and publishes them on a local http server. + * + * This talks to odrcore's own JNI bindings (`app.opendocument.core`, shipped as the + * `app.opendocument:odr-core-java` artifact plus the `libodr_jni.so` that conan builds). It used to + * go through `CoreWrapper` and a hand-written JNI layer in `src/main/cpp/core_wrapper.cpp` that + * flattened every failure into an integer error code; the core reports typed [OdrException]s + * instead, so the codes and their mirror exception types are gone. + * + * The loader owns the process-wide core state: the one-time initialization, the single http server + * (which [RawLoader] also publishes text files on) and the currently open [Document] that + * [retranslate] edits. + */ +// the context is nullable like FileLoader's own field, which close() clears and the unit +// tests never set - isSupported() is pure and does not need one +class CoreLoader(context: Context?, private val doOoxml: Boolean) : + FileLoader(context, LoaderType.CORE) { + + private var server: HttpServer? = null + private var httpThread: Thread? = null + + private var document: Document? = null + private var lastInputPath: String? = null + private var lastCachePath: String? = null + + override fun initialize( + listener: FileLoaderListener, + mainHandler: Handler, + backgroundHandler: Handler, + analyticsManager: AnalyticsManager, + crashManager: CrashManager, + ) { + // loads the native library and extracts the core assets. Kept out of the constructor so + // that constructing a CoreLoader stays side effect free - LoaderService calls initialize() + // right after new CoreLoader() anyway. + initializeCore(context) + + val serverCacheDir = File(context.cacheDir, "core/server") + if (!serverCacheDir.isDirectory && !serverCacheDir.mkdirs()) { + Log.e(TAG, "Failed to create cache directory for the core server: $serverCacheDir") + } + + val config = HttpServer.Config() + config.cachePath = serverCacheDir.absolutePath + val server = HttpServer(config) + this.server = server + + httpThread = + Thread { + try { + server.listen(SERVER_BIND_ADDRESS, SERVER_PORT) + } catch (e: Throwable) { + crashManager.log(e) + } + } + .also { it.start() } + + super.initialize(listener, mainHandler, backgroundHandler, analyticsManager, crashManager) + } + + override fun isSupported(options: Options): Boolean { + val fileType = options.fileType + return fileType.startsWith("application/vnd.oasis.opendocument") || + fileType.startsWith("application/x-vnd.oasis.opendocument") || + fileType.startsWith("application/vnd.oasis.opendocument.text-master") || + fileType.startsWith("application/msword") || + (doOoxml && + (fileType.startsWith( + "application/vnd.openxmlformats-officedocument.wordprocessingml.document" + ) || + fileType.startsWith( + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" + ) + // TODO: enable pptx too + // fileType.startsWith("application/vnd.openxmlformats-officedocument.presentationml.presentation") + )) + } + + override fun loadSync(options: Options) { + val result = Result() + result.options = options + result.loaderType = type + + try { + translate(options, result) + + callOnSuccess(result) + } catch (e: OdrException.FileEncrypted) { + callOnError(result, EncryptedDocumentException()) + } catch (e: OdrException.WrongPassword) { + callOnError(result, EncryptedDocumentException()) + } catch (e: OdrException.DecryptionFailed) { + callOnError(result, EncryptedDocumentException()) + } catch (e: Throwable) { + callOnError(result, e) + } + } + + private fun translate(options: Options, result: Result) { + val cachedFile = + checkNotNull(AndroidFileCache.getCacheFile(context, options.cacheUri)) { + "not a cached file: " + options.cacheUri + } + val cacheDirectory = AndroidFileCache.getCacheDirectory(cachedFile) + + val coreCacheDirectory = File(cacheDirectory, "core_cache") + + lastInputPath = cachedFile.path + lastCachePath = coreCacheDirectory.path + + val views = + host( + prefix = "odr", + inputPath = cachedFile.path, + cachePath = coreCacheDirectory.path, + password = options.password, + editable = options.translatable, + paging = USE_PAGING, + keepDocument = true, + ) + + for (view in views) { + result.partTitles.add(view.name) + result.partUris.add(Uri.parse(view.url)) + } + } + + /** + * Opens [inputPath], translates it to html and publishes it on the shared http server under + * [prefix], replacing whatever was published before. + * + * [keepDocument] retains the decoded document for [retranslate]; [RawLoader] has nothing to + * edit and passes false. + */ + fun host( + prefix: String, + inputPath: String, + cachePath: String, + password: String? = null, + editable: Boolean = false, + paging: Boolean = false, + keepDocument: Boolean = false, + ): List { + val server = checkNotNull(server) { "core server is not running" } + + Log.i(TAG, "host file") + + server.clear() + + var file = open(inputPath) + + if (file.passwordEncrypted()) { + if (password == null) { + throw OdrException.FileEncrypted(inputPath) + } + file = file.decrypt(password) + } + + Log.i(TAG, "type=" + Odr.fileTypeToString(file.fileType())) + + if (keepDocument) { + closeDocument() + // .doc-files are not real documents in core + if (file.isDocumentFile && file.fileType() != FileType.LEGACY_WORD_DOCUMENT) { + // TODO this will cause a second load + document = file.asDocumentFile().document() + } + } + + val htmlConfig = HtmlConfig() + htmlConfig.embedImages = false + htmlConfig.embedShippedResources = true + htmlConfig.relativeResourcePaths = false + htmlConfig.textDocumentMargin = paging + htmlConfig.editable = editable + + val cacheDirectory = File(cachePath) + cacheDirectory.deleteRecursively() + cacheDirectory.mkdirs() + + val service = Html.translate(file, cachePath, htmlConfig) + server.connectService(service, prefix) + + return selectViews(file, service.listViews()).map { view -> + Log.i(TAG, "view name=" + view.name() + " path=" + view.path()) + HostedView( + view.name(), + "http://$SERVER_URL_HOST:$SERVER_PORT/file/$prefix/" + view.path(), + ) + } + } + + override fun retranslate(options: Options, htmlDiff: String): File? { + try { + if (document == null) { + // necessary if fragment was destroyed in the meanwhile - meaning the Loader is + // reinstantiated + translate(options, Result().also { it.options = options }) + } + + val inputFile = File(checkNotNull(lastInputPath)) + val inputCacheDirectory = AndroidFileCache.getCacheDirectory(inputFile) + + return edit(htmlDiff, File(inputCacheDirectory, "retranslate").path) + } catch (e: Throwable) { + crashManager.log(e) + + return null + } + } + + /** + * Applies [htmlDiff] to the document currently held open by [host] and saves it next to + * [outputPathPrefix], with the extension that matches the document's own file type. + */ + fun edit(htmlDiff: String, outputPathPrefix: String): File { + val document = checkNotNull(document) { "no editable document is open" } + + val extension = Odr.fileTypeToString(document.fileType()) + val outputFile = File("$outputPathPrefix.$extension") + + Log.d(TAG, "HTML diff: $htmlDiff") + + Html.edit(document, htmlDiff) + document.save(outputFile.path) + + return outputFile + } + + override fun close() { + super.close() + + val server = this.server + if (server != null) { + server.stop() + httpThread?.let { + try { + it.join(1000) + } catch (e: InterruptedException) { + crashManager.log(e) + } + } + httpThread = null + + server.close() + this.server = null + } + + closeDocument() + } + + private fun closeDocument() { + document?.close() + document = null + } + + /** A translated view of a document, ready to be opened in the WebView. */ + data class HostedView(val name: String, val url: String) + + companion object { + private const val TAG = "CoreLoader" + + /** The loopback address the server binds to. */ + private const val SERVER_BIND_ADDRESS = "127.0.0.1" + + /** + * The host the served pages are addressed by, deliberately not [SERVER_BIND_ADDRESS]: + * res/xml/network_security_config.xml permits cleartext http for the domain "localhost" + * only, so a page loaded from "http://127.0.0.1:..." never reaches the WebView. + */ + private const val SERVER_URL_HOST = "localhost" + + const val SERVER_PORT: Int = 29665 + + /** + * Whether odrcore renders text documents with page margins. This used to be read from the + * "use_paging" remote config key, but that resolved to false for every user since firebase + * remote config was removed - the ConfigManager left behind is a stub without a backing + * store. Kept as an explicit constant so the shipped behavior is visible instead of hidden + * behind a lookup that cannot return a value. + */ + private const val USE_PAGING = false + + private var coreInitialized = false + + /** + * One-time process wide setup of the core: data paths and the temporary directory. + * + * [MetadataLoader] calls [Odr.mimetype], which needs the libmagic database set up here, so + * this has to have run before any document is loaded. LoaderService constructs and + * initializes the CoreLoader while starting up, well before the first load request. + */ + @JvmStatic + @Synchronized + fun initializeCore(context: Context) { + if (coreInitialized) { + return + } + + // core resolves its temporary directory through std::filesystem::temp_directory_path(), + // which falls back to /tmp - a path that does not exist on android. this replaces the + // tmpfile() interposition that used to live in src/main/cpp/tmpfile_hack.cpp: that only + // worked while the app linked the core into a library it compiled itself, and + // libodr_jni.so is a prebuilt now. has to happen before the first native call, because + // core caches the directory on first use. + Os.setenv("TMPDIR", context.cacheDir.absolutePath, true) + + Log.i(TAG, "odrcore " + Odr.identify()) + + val assetsDirectory = File(context.filesDir, "assets") + val odrCoreDataDirectory = File(assetsDirectory, "odrcore") + val libmagicDataDirectory = File(assetsDirectory, "libmagic") + + val assetExtractor = AssetExtractor(context.assets) + assetExtractor.setOverwrite() + assetExtractor.extract(assetsDirectory, "core/odrcore") + assetExtractor.extract(assetsDirectory, "core/libmagic") + + GlobalParams.setOdrCoreDataPath(odrCoreDataDirectory.absolutePath) + GlobalParams.setLibmagicDatabasePath( + File(libmagicDataDirectory, "magic.mgc").absolutePath + ) + + coreInitialized = true + } + + /** + * Opens [path] restricted to core's own decoder engine. pdf2htmlEX and wvWare are compiled + * out of the android build (`with_pdf2htmlEX=False`, `with_wvWare=False`), so offering them + * would only produce "unsupported decoder engine" failures. + */ + private fun open(path: String): DecodedFile { + val preference = DecodePreference() + preference.enginePriority.add(DecoderEngine.ODR) + return Odr.open(path, preference) + } + + /** + * Spreadsheets show one tab per sheet; every other format only shows the full "document" + * view without tabs (if the service provides one - e.g. plain text and image files only + * have a single differently named view). + */ + private fun selectViews(file: DecodedFile, views: List): List { + val isSpreadsheet = + file.isDocumentFile && + file.asDocumentFile().documentType() == DocumentType.SPREADSHEET + if (isSpreadsheet) { + return views.filter { it.name() != "document" } + } + + val hasDocumentView = views.any { it.name() == "document" } + if (hasDocumentView) { + return views.filter { it.name() == "document" } + } + + return views + } + } +} diff --git a/app/src/main/java/app/opendocument/droid/background/CoreWrapper.java b/app/src/main/java/app/opendocument/droid/background/CoreWrapper.java deleted file mode 100644 index 4607f002a90d..000000000000 --- a/app/src/main/java/app/opendocument/droid/background/CoreWrapper.java +++ /dev/null @@ -1,183 +0,0 @@ -package app.opendocument.droid.background; - -import android.content.Context; -import android.util.Log; -import com.viliussutkus89.android.assetextractor.AssetExtractor; -import java.io.File; -import java.util.LinkedList; -import java.util.List; - -public class CoreWrapper { - static { - System.loadLibrary("odr-core"); - } - - public static class GlobalParams { - public String coreDataPath; - public String libmagicDatabasePath; - - public String customTmpfilePath; - } - - public static native void setGlobalParams(GlobalParams params); - - public static String versionString() { - return versionStringNative(); - } - - private static native String versionStringNative(); - - public static void initialize(Context context) { - Log.i("CoreWrapper", versionString()); - - File assetsDirectory = new File(context.getFilesDir(), "assets"); - File odrCoreDataDirectory = new File(assetsDirectory, "odrcore"); - File libmagicDataDirectory = new File(assetsDirectory, "libmagic"); - - AssetExtractor ae = new AssetExtractor(context.getAssets()); - ae.setOverwrite(); - ae.extract(assetsDirectory, "core/odrcore"); - ae.extract(assetsDirectory, "core/libmagic"); - - CoreWrapper.GlobalParams globalParams = new CoreWrapper.GlobalParams(); - globalParams.coreDataPath = odrCoreDataDirectory.getAbsolutePath(); - globalParams.libmagicDatabasePath = - new File(libmagicDataDirectory, "magic.mgc").getAbsolutePath(); - globalParams.customTmpfilePath = context.getCacheDir().getAbsolutePath(); - CoreWrapper.setGlobalParams(globalParams); - } - - public static String mimetype(String path) { - return mimetypeNative(path); - } - - private static native String mimetypeNative(String path); - - public static class CoreOptions { - public boolean ooxml; - public boolean txt; - // TODO: remove - public boolean pdf; - - public boolean editable; - - public boolean paging; - - public String password; - - public String inputPath; - public String outputPath; - public String cachePath; - } - - public static CoreResult backtranslate(CoreOptions options, String htmlDiff) { - CoreResult result = backtranslateNative(options, htmlDiff); - - switch (result.errorCode) { - case 0: - break; - case -3: - result.exception = new CoreUnknownErrorException(); - break; - case -6: - result.exception = new CoreCouldNotEditException(); - break; - case -7: - result.exception = new CoreCouldNotSaveException(); - break; - default: - result.exception = new CoreUnexpectedErrorCodeException(); - break; - } - - return result; - } - - private static native CoreResult backtranslateNative(CoreOptions options, String htmlDiff); - - public static void close() { - CoreOptions options = new CoreOptions(); - - closeNative(options); - } - - private static native void closeNative(CoreOptions options); - - public static void createServer(String cachePath) { - createServerNative(cachePath); - } - - private static native void createServerNative(String cachePath); - - public static CoreResult hostFile(String prefix, CoreOptions options) { - CoreResult result = hostFileNative(prefix, options); - - switch (result.errorCode) { - case 0: - break; - case -1: - result.exception = new CoreCouldNotOpenException(); - break; - case -2: - result.exception = new CoreEncryptedException(); - break; - case -3: - result.exception = new CoreUnknownErrorException(); - break; - case -4: - result.exception = new CoreCouldNotTranslateException(); - break; - case -5: - result.exception = new CoreUnexpectedFormatException(); - break; - default: - result.exception = new CoreUnexpectedErrorCodeException(); - break; - } - - return result; - } - - private static native CoreResult hostFileNative(String prefix, CoreOptions options); - - public static void listenServer(int port) { - listenServerNative(port); - } - - private static native void listenServerNative(int port); - - public static void stopServer() { - stopServerNative(); - } - - private static native void stopServerNative(); - - public static class CoreResult { - public int errorCode; - - public Exception exception; - - public List pageNames = new LinkedList<>(); - public List pagePaths = new LinkedList<>(); - - public String outputPath; - - public String extension; - } - - public static class CoreCouldNotOpenException extends RuntimeException {} - - public static class CoreEncryptedException extends RuntimeException {} - - public static class CoreCouldNotTranslateException extends RuntimeException {} - - public static class CoreUnexpectedFormatException extends RuntimeException {} - - public static class CoreUnexpectedErrorCodeException extends RuntimeException {} - - public static class CoreUnknownErrorException extends RuntimeException {} - - public static class CoreCouldNotEditException extends RuntimeException {} - - public static class CoreCouldNotSaveException extends RuntimeException {} -} diff --git a/app/src/main/java/app/opendocument/droid/background/LoaderService.java b/app/src/main/java/app/opendocument/droid/background/LoaderService.java index 8c02a0e26479..06009c6cf66d 100644 --- a/app/src/main/java/app/opendocument/droid/background/LoaderService.java +++ b/app/src/main/java/app/opendocument/droid/background/LoaderService.java @@ -58,7 +58,7 @@ public synchronized void onCreate() { coreLoader = new CoreLoader(context, true); coreLoader.initialize(this, mainHandler, backgroundHandler, analyticsManager, crashManager); - rawLoader = new RawLoader(context); + rawLoader = new RawLoader(context, coreLoader); rawLoader.initialize(this, mainHandler, backgroundHandler, analyticsManager, crashManager); onlineLoader = new OnlineLoader(context, coreLoader); diff --git a/app/src/main/java/app/opendocument/droid/background/MetadataLoader.java b/app/src/main/java/app/opendocument/droid/background/MetadataLoader.java index 928205217523..ece99020e6f2 100644 --- a/app/src/main/java/app/opendocument/droid/background/MetadataLoader.java +++ b/app/src/main/java/app/opendocument/droid/background/MetadataLoader.java @@ -5,6 +5,7 @@ import android.net.Uri; import android.provider.OpenableColumns; import android.webkit.MimeTypeMap; +import app.opendocument.core.Odr; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -108,7 +109,8 @@ public void loadSync(Options options) { String mimetype = null; try { - mimetype = CoreWrapper.mimetype(cachedFile.getAbsolutePath()); + // needs the libmagic database that CoreLoader.initializeCore() wires up + mimetype = Odr.mimetype(cachedFile.getAbsolutePath()); } catch (Throwable e) { crashManager.log(e); } diff --git a/app/src/main/java/app/opendocument/droid/background/RawLoader.java b/app/src/main/java/app/opendocument/droid/background/RawLoader.java index f66888e1a047..5d0b6e7ba62c 100644 --- a/app/src/main/java/app/opendocument/droid/background/RawLoader.java +++ b/app/src/main/java/app/opendocument/droid/background/RawLoader.java @@ -10,6 +10,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.List; public class RawLoader extends FileLoader { @@ -40,10 +41,14 @@ public class RawLoader extends FileLoader { "text/rtf" }; - private CoreWrapper lastCore; + // text files are rendered by the core and published on the http server that CoreLoader + // owns, so this loader needs the CoreLoader rather than a core of its own + private final CoreLoader coreLoader; - public RawLoader(Context context) { + public RawLoader(Context context, CoreLoader coreLoader) { super(context, LoaderType.RAW); + + this.coreLoader = coreLoader; } @Override @@ -159,30 +164,21 @@ public void loadSync(Options options) { .appendQueryParameter("ext", extension) .build(); } else if (fileType.startsWith("text/")) { - if (lastCore != null) { - lastCore.close(); - } - - CoreWrapper core = new CoreWrapper(); - try { - lastCore = core; - } catch (Throwable e) { - crashManager.log(e); - } - - CoreWrapper.CoreOptions coreOptions = new CoreWrapper.CoreOptions(); - coreOptions.inputPath = cacheFile.getPath(); - coreOptions.outputPath = cacheDirectory.getPath(); - coreOptions.ooxml = false; - coreOptions.txt = true; - coreOptions.pdf = false; - - CoreWrapper.CoreResult coreResult = CoreWrapper.hostFile("raw-text", coreOptions); - if (coreResult.exception != null) { - throw coreResult.exception; - } - - finalUri = Uri.parse(coreResult.pagePaths.get(0)); + // the previous cache path used to be left unset, which reached the core as a + // null path; give the translation a directory of its own next to the file + File coreCacheDirectory = new File(cacheDirectory, "core_cache"); + + List views = + coreLoader.host( + "raw-text", + cacheFile.getPath(), + coreCacheDirectory.getPath(), + null, + false, + false, + false); + + finalUri = Uri.parse(views.get(0).getUrl()); } else if (fileType.startsWith("application/zip")) { File htmlFile = new File(cacheDirectory, "zip.html"); InputStream htmlPrefixStream = context.getAssets().open("zip-prefix.html"); diff --git a/app/src/test/java/app/opendocument/droid/background/RawLoaderTest.kt b/app/src/test/java/app/opendocument/droid/background/RawLoaderTest.kt index 26828e9cc198..c682c7a8ad86 100644 --- a/app/src/test/java/app/opendocument/droid/background/RawLoaderTest.kt +++ b/app/src/test/java/app/opendocument/droid/background/RawLoaderTest.kt @@ -10,7 +10,7 @@ class RawLoaderTest { @Before fun setUp() { - rawLoader = RawLoader(null) + rawLoader = RawLoader(null, CoreLoader(null, true)) } private fun isSupported(fileType: String): Boolean { diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 255bbef79dc6..79311df82f2a 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -5,6 +5,12 @@ spotless = "8.8.0" googleJavaFormat = "1.35.0" ktfmt = "0.64" +# the java half of odrcore's JNI bindings. keep in sync with the odrcore version +# pinned in app/conanfile.txt: that conan build produces the matching libodr_jni.so, +# and the java classes and the native library are one ABI with no version negotiation +# (handles are raw longs, enums cross as ordinals). +odrCoreJava = "5.7.0" + androidxAnnotation = "1.10.0" androidxAppcompat = "1.7.1" androidxCore = "1.19.0" @@ -22,6 +28,8 @@ androidxTest = "1.7.0" androidxTestJunit = "1.3.0" [libraries] +odr-core-java = { module = "app.opendocument:odr-core-java", version.ref = "odrCoreJava" } + androidx-annotation = { module = "androidx.annotation:annotation", version.ref = "androidxAnnotation" } androidx-appcompat = { module = "androidx.appcompat:appcompat", version.ref = "androidxAppcompat" } androidx-core = { module = "androidx.core:core", version.ref = "androidxCore" } diff --git a/settings.gradle b/settings.gradle index d47ced340929..d0f710616974 100644 --- a/settings.gradle +++ b/settings.gradle @@ -6,10 +6,41 @@ pluginManagement { } } +// odr-core-java (the java half of odrcore's JNI bindings) is published to github +// packages, which requires authentication even for public packages. supply a github +// username and a token carrying the read:packages scope either as gradle properties +// (in ~/.gradle/gradle.properties, never in the repo) or as environment variables: +// +// odr.githubUser / GITHUB_ACTOR +// odr.githubToken / GITHUB_TOKEN +// +// in github actions both are present by default and the workflow only has to ask for +// the packages: read permission. without them the dependency fails to resolve with a +// 401 instead of silently falling back to something else. +def githubPackagesSetting = { String property, String environmentVariable -> + providers.gradleProperty(property) + .orElse(providers.environmentVariable(environmentVariable)) + .filter { !it.isEmpty() } + .getOrElse("") +} + dependencyResolutionManagement { repositories { google() mavenCentral() + maven { + name = "OpenDocumentCore" + url = uri("https://maven.pkg.github.com/opendocument-app/OpenDocument.core") + credentials { + username = githubPackagesSetting('odr.githubUser', 'GITHUB_ACTOR') + password = githubPackagesSetting('odr.githubToken', 'GITHUB_TOKEN') + } + // nothing else is published here, so keep this repository from answering + // for dependencies that belong to maven central or google + content { + includeModule("app.opendocument", "odr-core-java") + } + } } } From 750fc872d4d21744cb484c57968e4a90e0949d86 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 26 Jul 2026 13:44:13 +0200 Subject: [PATCH 2/5] Wire CI and the docs for the github packages dependency The bindings artifact lives on github packages, which authenticates every read even for public packages. Both workflows now ask for the packages: read permission and hand GITHUB_ACTOR/GITHUB_TOKEN to the gradle steps, including the one inside the emulator runner. README gets the personal access token a local build needs. CLAUDE.md described the native wrapper that no longer exists: the JNI interface, the cpp directory, the GetFieldID contract that forced @JvmField onto CoreWrapper's fields, and the claim that renaming the namespace means touching JNI symbols - none of which is true now that the bindings live in odrcore's own app.opendocument.core package. It gains the two constraints that replace them: the maven artifact and the conan pin are one version, and everything the bindings touch has to exist on android API 26. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jT7GGUHvUnDvf8UMcQSLk --- .github/workflows/build_test.yml | 9 ++++++++ .github/workflows/release.yml | 5 +++++ CLAUDE.md | 36 +++++++++++++++++++------------- README.md | 12 +++++++++++ 4 files changed, 48 insertions(+), 14 deletions(-) diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml index 44263979c609..60941b5f2357 100644 --- a/.github/workflows/build_test.yml +++ b/.github/workflows/build_test.yml @@ -25,6 +25,9 @@ permissions: contents: read # required to publish lint findings to the code scanning tab security-events: write + # odr-core-java is resolved from github packages, which needs a token even + # for public packages + packages: read env: ndk_version: 28.2.13676358 @@ -85,6 +88,9 @@ jobs: - name: gradle # spotlessCheck runs in the format workflow, which has no paths-ignore + env: + GITHUB_ACTOR: ${{ github.actor }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: ./gradlew assembleDebug testProDebugUnitTest lintProDebug lintLiteDebug --stacktrace # drop temporary download/source/build folders so only built packages get cached @@ -221,6 +227,9 @@ jobs: - name: Run tests uses: reactivecircus/android-emulator-runner@v2 + env: + GITHUB_ACTOR: ${{ github.actor }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: api-level: ${{ matrix.api-level }} arch: ${{ matrix.arch }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f6351c933208..0df73812786d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -44,6 +44,9 @@ concurrency: permissions: contents: read + # odr-core-java is resolved from github packages, which needs a token even + # for public packages + packages: read env: ndk_version: 28.2.13676358 @@ -131,6 +134,8 @@ jobs: ODR_KEYSTORE_PASSWORD: ${{ secrets.ODR_KEYSTORE_PASSWORD }} ODR_KEY_PASSWORD_PRO: ${{ secrets.ODR_KEY_PASSWORD_PRO }} ODR_KEY_PASSWORD_LITE: ${{ secrets.ODR_KEY_PASSWORD_LITE }} + GITHUB_ACTOR: ${{ github.actor }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | tasks="" case "${{ env.flavor }}" in diff --git a/CLAUDE.md b/CLAUDE.md index 167062dabdb5..8513149d1ac2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,8 +59,10 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co **Native Dependencies:** - Uses Conan package manager for C++ dependencies -- CMake build system for native C++ core library (`odr-core`) -- NDK version 28.2.13676358 required +- The app compiles no native code itself; conan builds odrcore (including its JNI + bindings) and the deployer drops the resulting `.so` files into `jniLibs` +- NDK version 28.2.13676358 required (for conan's cross build, and for the + `libc++_shared.so` that gets shipped alongside `libodr_jni.so`) - C++20 standard - `conan` is taken from PATH; override with `-Podr.conanExecutable=...` or `ODR_CONAN`. Note the conan gradle plugin does not track `app/conanprofile.txt` as a task input, so @@ -68,16 +70,26 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co settings - run the conan install by hand to pick the change up locally. **Core Library Integration:** -- Native C++ wrapper (`CoreWrapper.cpp`) provides JNI interface +- The JNI interface comes from odrcore itself: java classes under `app.opendocument.core` + from the `app.opendocument:odr-core-java` maven artifact, and `libodr_jni.so` built by + conan from the recipe's `with_jni` option. `CoreLoader` is the only thing wrapping it. +- The maven artifact ships java classes only, so its version must stay in lockstep with + the `odrcore/x.y.z` pin in `app/conanfile.txt` - handles cross as raw longs and enums as + ordinals, with no version negotiation +- It lives on github packages, which requires authentication even for public packages; + see `settings.gradle` for the `odr.githubUser`/`odr.githubToken` settings +- Anything the bindings use must exist on **android API 26**, which is far below what + their `--release 17` compiler accepts. That is a runtime-only failure, on device + (`java.lang.ref.Cleaner` and `List.of` both had to be fixed upstream for this reason) - Supports multiple architectures: armv8, armv7, x86, x86_64 -- Assets deployed to `assets/core` directory via custom Conan deployer +- Assets deployed to `assets/core` and native libraries to `jniLibs/` via the custom + Conan deployer (`app/conandeployer.py`) ### Key Directories - `app/src/main/java/app/opendocument/droid/background/` - Document processing services - `app/src/main/java/app/opendocument/droid/ui/` - UI components and activities - `app/src/main/java/app/opendocument/droid/nonfree/` - Analytics, billing, and ads -- `app/src/main/cpp/` - Native C++ JNI wrapper - `app/src/main/assets/` - HTML templates and fonts for document rendering ### Dependencies @@ -109,10 +121,9 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co the `.pro` suffix) differ on purpose - do not "fix" the mismatch: - `namespace` is only the java/kotlin package plus `R`/`BuildConfig`, and is free to - rename. Renaming it means updating the JNI symbol names and `FindClass` strings in - `app/src/main/cpp/core_wrapper.{cpp,hpp}` and the `CoreWrapper` keeps in - `proguard-rules.txt` in lockstep - none of that is caught at compile time, and a stale - proguard keep only fails in minified builds. + rename. Nothing native is bound to it anymore - the JNI symbols live in odrcore's own + `app.opendocument.core` package, which the app does not rename - so the keeps in + `proguard-rules.txt` are about that package, not this one. - `applicationId` is the identity on Play and F-Droid and can never change: Play has no rename path, so a new one is a new listing that existing installs never update to. Store-facing renaming goes through the listing title in `fastlane/metadata/`. @@ -130,10 +141,7 @@ the `.pro` suffix) differ on purpose - do not "fix" the mismatch: Mixed Java and Kotlin; Kotlin support is built into AGP 9, no kotlin plugin is applied. New code should be Kotlin. When converting an existing class, watch two things: -- `CoreWrapper` and its nested `CoreOptions`/`CoreResult`/`GlobalParams` are read from C++ - through `GetFieldID` in `app/src/main/cpp/core_wrapper.cpp` (see also Package names - above). Kotlin properties compile to - private fields with accessors, which breaks those lookups - every field needs `@JvmField`. - `proguard-rules.txt` keeps these classes for the same reason. - Java callers spell static helpers as `Foo.bar()`, so converted utility classes need `object` plus `@JvmStatic`, and public final fields need `@JvmField`. +- Kotlin classes with default arguments are called from the remaining java loaders, which + cannot see defaults - either pass every argument or add `@JvmOverloads`. diff --git a/README.md b/README.md index b194b4e89ddf..2632527dd8c3 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,18 @@ Please help to translate on the https://crowdin.com/project/opendocument changing $PATH. - `git submodule update --init --depth 1 conan-odr-index` - `python conan-odr-index/scripts/conan_export_all_packages.py` +- the java half of odrcore's JNI bindings (`app.opendocument:odr-core-java`) comes + from GitHub Packages, which needs authentication even though the package is + public. Create a personal access token with the `read:packages` scope and put it + in `~/.gradle/gradle.properties` (never in the repo): + + ```properties + odr.githubUser=your-github-username + odr.githubToken=ghp_... + ``` + + The `GITHUB_ACTOR` and `GITHUB_TOKEN` environment variables work too, which is + what CI uses. Without them the build fails to resolve the dependency with a 401. ## Release signing From 28a3e173be36820c1282fff653569db56dbcda39 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 26 Jul 2026 16:49:54 +0200 Subject: [PATCH 3/5] Bump odrcore to 5.7.1 for the android API 26 JNI fix odrcore's java bindings used java.lang.ref.Cleaner, which only exists from API 33, so every instrumented test below that failed to load the classes with NoClassDefFoundError while API 34 passed. v5.7.1 carries the upstream fix (opendocument-app/OpenDocument.core#621). The conan recipe, the odr-core-java maven artifact and the index submodule move together: the java classes and libodr_jni.so are one ABI with no version negotiation. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jT7GGUHvUnDvf8UMcQSLk --- app/conanfile.txt | 2 +- conan-odr-index | 2 +- gradle/libs.versions.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/conanfile.txt b/app/conanfile.txt index 2ed8239e7481..5ece20f1dd2d 100644 --- a/app/conanfile.txt +++ b/app/conanfile.txt @@ -1,5 +1,5 @@ [requires] -odrcore/5.7.0 +odrcore/5.7.1 [generators] CMakeToolchain diff --git a/conan-odr-index b/conan-odr-index index 010a87833ee9..a7930869a669 160000 --- a/conan-odr-index +++ b/conan-odr-index @@ -1 +1 @@ -Subproject commit 010a87833ee98f67163d07c459a7ed6cf088d940 +Subproject commit a7930869a669efa17ef23cba28c2756418be0ee6 diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 79311df82f2a..dc1e6a9d6fb4 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -9,7 +9,7 @@ ktfmt = "0.64" # pinned in app/conanfile.txt: that conan build produces the matching libodr_jni.so, # and the java classes and the native library are one ABI with no version negotiation # (handles are raw longs, enums cross as ordinals). -odrCoreJava = "5.7.0" +odrCoreJava = "5.7.1" androidxAnnotation = "1.10.0" androidxAppcompat = "1.7.1" From df64f2eca587cd3bcd7a195fd8a1168e1e6e6709 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 26 Jul 2026 17:17:31 +0200 Subject: [PATCH 4/5] Take odr-core-java from the conan package instead of github packages The jar was resolved from github packages, which needs authentication even though the package is public. F-Droid builds this app from source in a clean environment with no credentials, so the dependency would have failed there with a 401 and broken the F-Droid distribution. The odrcore conan package already ships the jar in share/java, right next to the libodr_jni.so it belongs to - the deployer was throwing it away. Deploy it to build/conan//libs instead and depend on it as a file. That also removes the version skew this had by construction: the java classes and the native library are one ABI with no version negotiation, and they were pinned separately in libs.versions.toml and conanfile.txt with only a comment holding them together. Both halves now come out of the same package. Verified with a clean build with GITHUB_ACTOR and GITHUB_TOKEN unset: assembleProDebug, testProDebugUnitTest, lintProDebug and spotlessCheck all pass, the 100 binding classes land in the apk dex, and the jar is still kept out of the assets. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jT7GGUHvUnDvf8UMcQSLk --- .github/workflows/build_test.yml | 9 ----- .github/workflows/release.yml | 5 --- CLAUDE.md | 18 +++++---- README.md | 16 ++------ app/build.gradle | 11 ++++-- app/conandeployer.py | 21 +++++++++-- .../droid/background/CoreLoader.kt | 10 ++--- gradle/libs.versions.toml | 12 ++---- settings.gradle | 37 +++---------------- 9 files changed, 54 insertions(+), 85 deletions(-) diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml index 60941b5f2357..44263979c609 100644 --- a/.github/workflows/build_test.yml +++ b/.github/workflows/build_test.yml @@ -25,9 +25,6 @@ permissions: contents: read # required to publish lint findings to the code scanning tab security-events: write - # odr-core-java is resolved from github packages, which needs a token even - # for public packages - packages: read env: ndk_version: 28.2.13676358 @@ -88,9 +85,6 @@ jobs: - name: gradle # spotlessCheck runs in the format workflow, which has no paths-ignore - env: - GITHUB_ACTOR: ${{ github.actor }} - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: ./gradlew assembleDebug testProDebugUnitTest lintProDebug lintLiteDebug --stacktrace # drop temporary download/source/build folders so only built packages get cached @@ -227,9 +221,6 @@ jobs: - name: Run tests uses: reactivecircus/android-emulator-runner@v2 - env: - GITHUB_ACTOR: ${{ github.actor }} - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: api-level: ${{ matrix.api-level }} arch: ${{ matrix.arch }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0df73812786d..f6351c933208 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -44,9 +44,6 @@ concurrency: permissions: contents: read - # odr-core-java is resolved from github packages, which needs a token even - # for public packages - packages: read env: ndk_version: 28.2.13676358 @@ -134,8 +131,6 @@ jobs: ODR_KEYSTORE_PASSWORD: ${{ secrets.ODR_KEYSTORE_PASSWORD }} ODR_KEY_PASSWORD_PRO: ${{ secrets.ODR_KEY_PASSWORD_PRO }} ODR_KEY_PASSWORD_LITE: ${{ secrets.ODR_KEY_PASSWORD_LITE }} - GITHUB_ACTOR: ${{ github.actor }} - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | tasks="" case "${{ env.flavor }}" in diff --git a/CLAUDE.md b/CLAUDE.md index 8513149d1ac2..57a8dabe2961 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,14 +70,16 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co settings - run the conan install by hand to pick the change up locally. **Core Library Integration:** -- The JNI interface comes from odrcore itself: java classes under `app.opendocument.core` - from the `app.opendocument:odr-core-java` maven artifact, and `libodr_jni.so` built by - conan from the recipe's `with_jni` option. `CoreLoader` is the only thing wrapping it. -- The maven artifact ships java classes only, so its version must stay in lockstep with - the `odrcore/x.y.z` pin in `app/conanfile.txt` - handles cross as raw longs and enums as - ordinals, with no version negotiation -- It lives on github packages, which requires authentication even for public packages; - see `settings.gradle` for the `odr.githubUser`/`odr.githubToken` settings +- The JNI interface comes from odrcore itself, both halves out of the one conan package + built with the recipe's `with_jni` option: java classes under `app.opendocument.core` + from `share/java/odr-core-java.jar`, and the matching `libodr_jni.so`. `CoreLoader` is + the only thing wrapping it. +- Taking both from the same package is deliberate and should stay that way - handles + cross as raw longs and enums as ordinals with no version negotiation, so a separately + versioned java artifact could drift from the `.so`. It also keeps the build free of + credentials, which f-droid and other clean source builders need. `conandeployer.py` + puts the jar in `build/conan//libs` and `app/build.gradle` depends on the armv8 + copy as a file, not through a repository. - Anything the bindings use must exist on **android API 26**, which is far below what their `--release 17` compiler accepts. That is a runtime-only failure, on device (`java.lang.ref.Cleaner` and `List.of` both had to be fixed upstream for this reason) diff --git a/README.md b/README.md index 2632527dd8c3..8a61f71a1f77 100644 --- a/README.md +++ b/README.md @@ -25,18 +25,10 @@ Please help to translate on the https://crowdin.com/project/opendocument changing $PATH. - `git submodule update --init --depth 1 conan-odr-index` - `python conan-odr-index/scripts/conan_export_all_packages.py` -- the java half of odrcore's JNI bindings (`app.opendocument:odr-core-java`) comes - from GitHub Packages, which needs authentication even though the package is - public. Create a personal access token with the `read:packages` scope and put it - in `~/.gradle/gradle.properties` (never in the repo): - - ```properties - odr.githubUser=your-github-username - odr.githubToken=ghp_... - ``` - - The `GITHUB_ACTOR` and `GITHUB_TOKEN` environment variables work too, which is - what CI uses. Without them the build fails to resolve the dependency with a 401. +- the java half of odrcore's JNI bindings (`odr-core-java.jar`) needs no setup: it + ships inside the odrcore conan package that also builds `libodr_jni.so`, and the + conan deployer puts it in `app/build/conan/armv8/libs`. No credentials are + involved anywhere in the build. ## Release signing diff --git a/app/build.gradle b/app/build.gradle index 957c85de69e0..bed88e8818b9 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -181,9 +181,14 @@ android { } dependencies { - // the java side of odrcore's JNI bindings. the matching libodr_jni.so is built by - // conan and deployed into jniLibs above, so this artifact is java classes only - implementation libs.odr.core.java + // the java side of odrcore's JNI bindings, taken from the same conan package that + // built the libodr_jni.so deployed into jniLibs above. the two are one ABI with no + // version negotiation, so sourcing both from one package is what keeps them in + // step. the jar is architecture independent - armv8 just happens to be the copy we + // read, like the assets source set above. + implementation files(layout.buildDirectory.file("conan/armv8/libs/odr-core-java.jar")) { + builtBy("conanInstall-armv8") + } implementation libs.play.services.ads implementation libs.play.review diff --git a/app/conandeployer.py b/app/conandeployer.py index 1353b3706720..4dbf20c080b9 100644 --- a/app/conandeployer.py +++ b/app/conandeployer.py @@ -40,12 +40,25 @@ def deploy(graph, output_folder: str, **kwargs): shutil.copytree( f"{dep.package_folder}/share", f"{assets_folder}/odrcore", - # share/java holds odr-core-java.jar, which the app takes from github - # packages instead - shipping it as an asset would only bloat the apk + # share/java holds odr-core-java.jar, which is deployed as a library + # below - shipping it as an asset too would only bloat the apk ignore=shutil.ignore_patterns("java"), **copytree_kwargs, ) + # the java half of the JNI bindings. taking it from the very package that + # built libodr_jni.so is what keeps the two halves in lockstep: they are one + # ABI with no version negotiation, and a maven artifact resolved by version + # could drift from the .so. it also keeps the build credential-free, which + # f-droid and other clean source builders need + libs_folder = f"{output_folder}/libs" + os.makedirs(libs_folder, exist_ok=True) + conanfile.output.info(f"Deploying odr-core-java.jar to {libs_folder}") + shutil.copy2( + f"{dep.package_folder}/share/java/odr-core-java.jar", + f"{libs_folder}/odr-core-java.jar", + ) + if "libmagic" in deps: dep = deps["libmagic"] conanfile.output.info(f"Deploying libmagic assets to {assets_folder}") @@ -63,8 +76,8 @@ def deploy(graph, output_folder: str, **kwargs): if "odrcore" in deps: dep = deps["odrcore"] - # built by the recipe's with_jni option; the java side comes from the - # odr-core-java maven artifact of the very same odrcore version + # built by the recipe's with_jni option, alongside the odr-core-java.jar + # deployed above - both halves come out of this one package source = f"{dep.package_folder}/lib/libodr_jni.so" conanfile.output.info(f"Deploying libodr_jni.so to {jni_libs_folder}") shutil.copy2(source, f"{jni_libs_folder}/libodr_jni.so") diff --git a/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt b/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt index 95c8f9b59749..1fdceb18fba4 100644 --- a/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt +++ b/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt @@ -31,11 +31,11 @@ import java.io.File /** * Loads documents through odrcore and publishes them on a local http server. * - * This talks to odrcore's own JNI bindings (`app.opendocument.core`, shipped as the - * `app.opendocument:odr-core-java` artifact plus the `libodr_jni.so` that conan builds). It used to - * go through `CoreWrapper` and a hand-written JNI layer in `src/main/cpp/core_wrapper.cpp` that - * flattened every failure into an integer error code; the core reports typed [OdrException]s - * instead, so the codes and their mirror exception types are gone. + * This talks to odrcore's own JNI bindings (`app.opendocument.core`, the `odr-core-java.jar` and + * the matching `libodr_jni.so`, both out of the odrcore conan package). It used to go through + * `CoreWrapper` and a hand-written JNI layer in `src/main/cpp/core_wrapper.cpp` that flattened + * every failure into an integer error code; the core reports typed [OdrException]s instead, so the + * codes and their mirror exception types are gone. * * The loader owns the process-wide core state: the one-time initialization, the single http server * (which [RawLoader] also publishes text files on) and the currently open [Document] that diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index dc1e6a9d6fb4..3d9b640d9d24 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -5,11 +5,9 @@ spotless = "8.8.0" googleJavaFormat = "1.35.0" ktfmt = "0.64" -# the java half of odrcore's JNI bindings. keep in sync with the odrcore version -# pinned in app/conanfile.txt: that conan build produces the matching libodr_jni.so, -# and the java classes and the native library are one ABI with no version negotiation -# (handles are raw longs, enums cross as ordinals). -odrCoreJava = "5.7.1" +# the java half of odrcore's JNI bindings is not versioned here: it comes out of the +# odrcore conan package pinned in app/conanfile.txt, which also builds the matching +# libodr_jni.so. see app/conandeployer.py. androidxAnnotation = "1.10.0" androidxAppcompat = "1.7.1" @@ -28,9 +26,7 @@ androidxTest = "1.7.0" androidxTestJunit = "1.3.0" [libraries] -odr-core-java = { module = "app.opendocument:odr-core-java", version.ref = "odrCoreJava" } - -androidx-annotation = { module = "androidx.annotation:annotation", version.ref = "androidxAnnotation" } +androidx-annotation ={ module = "androidx.annotation:annotation", version.ref = "androidxAnnotation" } androidx-appcompat = { module = "androidx.appcompat:appcompat", version.ref = "androidxAppcompat" } androidx-core = { module = "androidx.core:core", version.ref = "androidxCore" } androidx-webkit = { module = "androidx.webkit:webkit", version.ref = "androidxWebkit" } diff --git a/settings.gradle b/settings.gradle index d0f710616974..68b56403e45e 100644 --- a/settings.gradle +++ b/settings.gradle @@ -6,41 +6,16 @@ pluginManagement { } } -// odr-core-java (the java half of odrcore's JNI bindings) is published to github -// packages, which requires authentication even for public packages. supply a github -// username and a token carrying the read:packages scope either as gradle properties -// (in ~/.gradle/gradle.properties, never in the repo) or as environment variables: -// -// odr.githubUser / GITHUB_ACTOR -// odr.githubToken / GITHUB_TOKEN -// -// in github actions both are present by default and the workflow only has to ask for -// the packages: read permission. without them the dependency fails to resolve with a -// 401 instead of silently falling back to something else. -def githubPackagesSetting = { String property, String environmentVariable -> - providers.gradleProperty(property) - .orElse(providers.environmentVariable(environmentVariable)) - .filter { !it.isEmpty() } - .getOrElse("") -} - +// odr-core-java (the java half of odrcore's JNI bindings) deliberately does not come +// from a repository here: it is deployed out of the odrcore conan package that also +// builds libodr_jni.so (see app/conandeployer.py). that keeps the two halves of one +// ABI from drifting apart, and keeps the build resolvable without credentials - the +// github packages copy needs authentication even though it is public, which no clean +// source builder such as f-droid can supply. dependencyResolutionManagement { repositories { google() mavenCentral() - maven { - name = "OpenDocumentCore" - url = uri("https://maven.pkg.github.com/opendocument-app/OpenDocument.core") - credentials { - username = githubPackagesSetting('odr.githubUser', 'GITHUB_ACTOR') - password = githubPackagesSetting('odr.githubToken', 'GITHUB_TOKEN') - } - // nothing else is published here, so keep this repository from answering - // for dependencies that belong to maven central or google - content { - includeModule("app.opendocument", "odr-core-java") - } - } } } From 1129b7e34455fe4ddd02438bcde6164108758f3f Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 26 Jul 2026 17:35:30 +0200 Subject: [PATCH 5/5] Give the edit mode tests 30s to see contenteditable testODTEditMode and testDOCXEditMode are the flakiest tests in the suite: they fail on API 26 and 30 across unrelated branches, including a dependabot action bump, while the same commit passes elsewhere. The document loads fine in those runs - every other test in the job passes - the webview just needs longer than 10s to report contenteditable on a cold CI emulator. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0151dSFxsWcv4mP7tiy7jwyf --- .../app/opendocument/droid/test/MainActivityTests.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.java b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.java index bf14c3fe9fb6..d8934f73554b 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.java +++ b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.java @@ -66,6 +66,11 @@ public class MainActivityTests { private IdlingResource m_idlingResource; private static final Map s_testFiles = new ArrayMap<>(); + // entering edit mode has to round-trip through the webview before the dom reports + // contenteditable, and on a cold CI emulator that regularly took longer than the 10s + // this used to wait for - the edit mode tests were the flakiest thing in the suite. + private static final long EDIT_MODE_TIMEOUT_MS = 30000; + // Yes, this is ActivityTestRule instead of ActivityScenario, because ActivityScenario does not // actually work. // Issue ID may or may not be added later. @@ -354,7 +359,7 @@ public void testODTEditMode() throws InterruptedException { Assert.assertNotNull(pageView); Assert.assertTrue( "ODT should become editable after entering edit mode", - waitForEditableState(pageView, true, 10000)); + waitForEditableState(pageView, true, EDIT_MODE_TIMEOUT_MS)); } @Test @@ -370,7 +375,7 @@ public void testDOCXEditMode() throws InterruptedException { Assert.assertTrue( "DOCX should become editable after entering edit mode", - waitForEditableState(pageView, true, 10000)); + waitForEditableState(pageView, true, EDIT_MODE_TIMEOUT_MS)); } @Test