Fix a bunch of thread violations (readium#490)

Co-authored-by: qnga <[email protected]>
marshal1919 · Apr 16, 2024 · 27d64b1 · 27d64b1
1 parent 9d18784
commit 27d64b1
Show file tree

Hide file tree

Showing 15 changed files with 66 additions and 63 deletions.
diff --git a/.idea/.gitignore b/.idea/.gitignore
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -55,6 +55,7 @@ junit = "4.13.2"
 
 kotlinx-coroutines = "1.7.3"
 kotlinx-coroutines-test = "1.7.3"
+kotlinx-datetime = "0.6.0-RC.2"
 kotlinx-serialization-json = "1.6.2"
 
 # Make sure to align with the Kotlin version.
@@ -139,6 +140,7 @@ kotlin-stdlib = { group = "org.jetbrains.kotlin", name = "kotlin-stdlib", versio
 
 kotlinx-coroutines-android = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-android", version.ref = "kotlinx-coroutines" }
 kotlinx-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "kotlinx-coroutines-test" }
+kotlinx-datetime = { group = "org.jetbrains.kotlinx", name = "kotlinx-datetime", version.ref = "kotlinx-datetime" }
 kotlinx-serialization-json = { group = "org.jetbrains.kotlinx", name = "kotlinx-serialization-json", version.ref = "kotlinx-serialization-json" }
 
 pdfium = { group = "com.github.barteksc", name = "pdfium-android", version.ref="pdfium" }

diff --git a/...pters/pdfium/document/src/main/java/org/readium/adapter/pdfium/document/PdfiumDocument.kt b/...pters/pdfium/document/src/main/java/org/readium/adapter/pdfium/document/PdfiumDocument.kt
@@ -17,6 +17,7 @@ import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
 import org.readium.r2.shared.InternalReadiumApi
 import org.readium.r2.shared.extensions.md5
+import org.readium.r2.shared.extensions.tryOrLog
 import org.readium.r2.shared.extensions.tryOrNull
 import org.readium.r2.shared.util.Try
 import org.readium.r2.shared.util.data.ReadError
@@ -69,7 +70,11 @@ public class PdfiumDocument(
         core.getTableOfContents(document).map { it.toOutlineNode() }
     }
 
-    override suspend fun close() {}
+    override suspend fun close() {
+        tryOrLog {
+            core.closeDocument(document)
+        }
+    }
 
     public companion object
 }

diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt
@@ -7,7 +7,6 @@
 package org.readium.r2.navigator
 
 import android.content.Context
-import android.content.SharedPreferences
 import android.graphics.PointF
 import android.graphics.Rect
 import android.graphics.RectF
@@ -112,7 +111,6 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV
     }
 
     var listener: Listener? = null
-    internal var preferences: SharedPreferences? = null
 
     var resourceUrl: AbsoluteUrl? = null
 

diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/image/ImageNavigatorFragment.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/image/ImageNavigatorFragment.kt
@@ -6,8 +6,6 @@
 
 package org.readium.r2.navigator.image
 
-import android.content.Context
-import android.content.SharedPreferences
 import android.graphics.PointF
 import android.os.Bundle
 import android.view.LayoutInflater
@@ -65,8 +63,6 @@ public class ImageNavigatorFragment private constructor(
     internal lateinit var positions: List<Locator>
     internal lateinit var resourcePager: R2ViewPager
 
-    internal lateinit var preferences: SharedPreferences
-
     internal lateinit var adapter: R2PagerAdapter
     private lateinit var currentActivity: FragmentActivity
 
@@ -102,10 +98,6 @@ public class ImageNavigatorFragment private constructor(
         _binding = ReadiumNavigatorViewpagerBinding.inflate(inflater, container, false)
         val view = binding.root
 
-        preferences = requireContext().getSharedPreferences(
-            "org.readium.r2.settings",
-            Context.MODE_PRIVATE
-        )
         resourcePager = binding.resourcePager
         resourcePager.publicationType = R2ViewPager.PublicationType.CBZ
 

diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2EpubPageFragment.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2EpubPageFragment.kt
@@ -10,8 +10,6 @@
 package org.readium.r2.navigator.pager
 
 import android.annotation.SuppressLint
-import android.content.Context
-import android.content.SharedPreferences
 import android.graphics.PointF
 import android.os.Bundle
 import android.util.DisplayMetrics
@@ -63,7 +61,6 @@ internal class R2EpubPageFragment : Fragment() {
         private set
 
     private lateinit var containerView: View
-    private lateinit var preferences: SharedPreferences
     private val viewModel: EpubNavigatorViewModel by viewModels(
         ownerProducer = { requireParentFragment() }
     )
@@ -137,10 +134,6 @@ internal class R2EpubPageFragment : Fragment() {
     ): View {
         _binding = ReadiumNavigatorViewpagerFragmentEpubBinding.inflate(inflater, container, false)
         containerView = binding.root
-        preferences = activity?.getSharedPreferences(
-            "org.readium.r2.settings",
-            Context.MODE_PRIVATE
-        )!!
 
         val webView = binding.webView
         this.webView = webView
@@ -158,7 +151,6 @@ internal class R2EpubPageFragment : Fragment() {
                 }
             }
         }
-        webView.preferences = preferences
 
         webView.settings.javaScriptEnabled = true
         webView.isVerticalScrollBarEnabled = false

diff --git a/readium/shared/build.gradle.kts b/readium/shared/build.gradle.kts
@@ -17,9 +17,9 @@ dependencies {
     implementation(libs.androidx.appcompat)
     implementation(libs.androidx.browser)
     implementation(libs.timber)
-    implementation(libs.joda.time)
     implementation(libs.kotlin.reflect)
     implementation(libs.kotlinx.coroutines.android)
+    implementation(libs.kotlinx.datetime)
     implementation(libs.kotlinx.serialization.json)
     implementation(libs.jsoup)
 

diff --git a/readium/shared/src/main/java/org/readium/r2/shared/extensions/Date.kt b/readium/shared/src/main/java/org/readium/r2/shared/extensions/Date.kt
@@ -9,11 +9,12 @@
 
 package org.readium.r2.shared.extensions
 
-import java.util.*
-import org.joda.time.DateTime
-import org.joda.time.DateTimeZone
+import java.util.Date
+import kotlinx.datetime.Instant
+import kotlinx.datetime.format
+import kotlinx.datetime.format.DateTimeComponents
 import org.readium.r2.shared.InternalReadiumApi
 
 @InternalReadiumApi
-public fun Date.toIso8601String(timeZone: TimeZone = TimeZone.getTimeZone("UTC")): String =
-    DateTime(this, DateTimeZone.forTimeZone(timeZone)).toString()
+public fun Date.toIso8601String(): String =
+    Instant.fromEpochMilliseconds(time).format(DateTimeComponents.Formats.ISO_DATE_TIME_OFFSET)
diff --git a/readium/shared/src/main/java/org/readium/r2/shared/extensions/String.kt b/readium/shared/src/main/java/org/readium/r2/shared/extensions/String.kt
@@ -12,30 +12,26 @@ package org.readium.r2.shared.extensions
 import android.net.Uri
 import java.net.URL
 import java.security.MessageDigest
-import java.util.*
-import org.joda.time.DateTime
-import org.joda.time.DateTimeZone
+import java.util.Date
+import kotlinx.datetime.Instant
+import kotlinx.datetime.LocalDate
+import kotlinx.datetime.LocalDateTime
+import kotlinx.datetime.TimeZone
+import kotlinx.datetime.atStartOfDayIn
+import kotlinx.datetime.toInstant
 import org.json.JSONException
 import org.json.JSONObject
 import org.readium.r2.shared.InternalReadiumApi
 
 @InternalReadiumApi
-public fun String.iso8601ToDate(): Date? =
-    try {
-        // We assume that a date without a time zone component is in UTC. To handle this properly,
-        // we need to set the default time zone of Joda to UTC, since by default it uses the local
-        // time zone. This ensures that apps see exactly the same dates (e.g. published) no matter
-        // where they are located.
-        // For the same reason, the output Date will be in UTC. Apps should convert it to the local
-        // time zone for display purposes, or keep it as UTC for storage.
-        val defaultTZ = DateTimeZone.getDefault()
-        DateTimeZone.setDefault(DateTimeZone.UTC)
-        val date = DateTime(this).toDateTime(DateTimeZone.UTC).toDate()
-        DateTimeZone.setDefault(defaultTZ)
-        date
-    } catch (e: Exception) {
-        null
-    }
+public fun String.iso8601ToDate(): Date? {
+    val instant = tryOrNull { Instant.parse(this) }
+        ?: tryOrNull { LocalDateTime.parse(this).toInstant(TimeZone.UTC) }
+        ?: tryOrNull { LocalDate.parse(this).atStartOfDayIn(TimeZone.UTC) }
+        ?: return null
+
+    return Date(instant.toEpochMilliseconds())
+}
 
 /**
  * If this string starts with the given [prefix], returns this string.

diff --git a/readium/shared/src/main/java/org/readium/r2/shared/util/file/FileResource.kt b/readium/shared/src/main/java/org/readium/r2/shared/util/file/FileResource.kt
@@ -99,21 +99,24 @@ public class FileResource(
     }
 
     override suspend fun length(): Try<Long, ReadError> =
-        metadataLength?.let { Try.success(it) }
-            ?: Try.failure(
-                ReadError.UnsupportedOperation(
-                    DebugError("Length not available for file at ${file.path}.")
+        withContext(Dispatchers.IO) {
+            metadataLength?.let { Try.success(it) }
+                ?: Try.failure(
+                    ReadError.UnsupportedOperation(
+                        DebugError("Length not available for file at ${file.path}.")
+                    )
                 )
-            )
+        }
 
-    private val metadataLength: Long? =
+    private val metadataLength: Long? by lazy {
         tryOrNull {
             if (file.isFile) {
                 file.length()
             } else {
                 null
             }
         }
+    }
 
     private inline fun <T> Try.Companion.catching(closure: () -> T): Try<T, ReadError> =
         try {

diff --git a/readium/shared/src/test/java/org/readium/r2/shared/extensions/StringTest.kt b/readium/shared/src/test/java/org/readium/r2/shared/extensions/StringTest.kt
@@ -6,12 +6,25 @@
 
 package org.readium.r2.shared.extensions
 
+import kotlin.test.assertEquals
 import kotlin.test.assertFalse
+import kotlin.test.assertNull
 import kotlin.test.assertTrue
 import org.junit.Test
 
 class StringTest {
 
+    @Test
+    fun `converts a ISO-8601 string to a Date`() {
+        assertNull("invalid".iso8601ToDate())
+
+        assertEquals(1712707200000, "2024-04-10".iso8601ToDate()?.time)
+        assertEquals(1712746680000, "2024-04-10T10:58".iso8601ToDate()?.time)
+        assertEquals(1712746724000, "2024-04-10T10:58:44".iso8601ToDate()?.time)
+        assertEquals(1712746724000, "2024-04-10T10:58:44Z".iso8601ToDate()?.time)
+        assertEquals(1712746724000, "2024-04-10T10:58:44.000Z".iso8601ToDate()?.time)
+    }
+
     @Test
     fun `checks if a string is made of printable ASCII characters`() {
         assertTrue("".isPrintableAscii())

diff --git a/readium/shared/src/test/java/org/readium/r2/shared/opds/AvailabilityTest.kt b/readium/shared/src/test/java/org/readium/r2/shared/opds/AvailabilityTest.kt
@@ -74,14 +74,14 @@ class AvailabilityTest {
             JSONObject(
                 """{
                 'state': 'available',
-                'since': '2001-02-01T13:36:27.000Z',
-                'until': '2001-02-01T13:36:27.000Z'
+                'since': '2001-02-01T13:36:27Z',
+                'until': '2001-02-01T13:36:27Z'
             }"""
             ),
             Availability(
                 state = Availability.State.AVAILABLE,
-                since = "2001-02-01T13:36:27.000Z".iso8601ToDate(),
-                until = "2001-02-01T13:36:27.000Z".iso8601ToDate()
+                since = "2001-02-01T13:36:27Z".iso8601ToDate(),
+                until = "2001-02-01T13:36:27Z".iso8601ToDate()
             ).toJSON()
         )
     }

diff --git a/readium/shared/src/test/java/org/readium/r2/shared/publication/MetadataTest.kt b/readium/shared/src/test/java/org/readium/r2/shared/publication/MetadataTest.kt
@@ -218,8 +218,8 @@ class MetadataTest {
                 ],
                 "title": {"en": "Title", "fr": "Titre"},
                 "subtitle": {"en": "Subtitle", "fr": "Sous-titre"},
-                "modified": "2001-01-01T12:36:27.000Z",
-                "published": "2001-01-02T12:36:27.000Z",
+                "modified": "2001-01-01T12:36:27Z",
+                "published": "2001-01-02T12:36:27Z",
                 "accessibility": {
                     "conformsTo": ["http://www.idpf.org/epub/a11y/accessibility-20170105.html#wcag-a"],
                     "accessMode": ["textual"],

diff --git a/test-app/src/main/java/org/readium/r2/testapp/Application.kt b/test-app/src/main/java/org/readium/r2/testapp/Application.kt
@@ -52,7 +52,7 @@ class Application : android.app.Application() {
 
     override fun onCreate() {
         if (DEBUG) {
-//            enableStrictMode()
+            enableStrictMode()
             Timber.plant(Timber.DebugTree())
         }
 

diff --git a/test-app/src/main/java/org/readium/r2/testapp/domain/CoverStorage.kt b/test-app/src/main/java/org/readium/r2/testapp/domain/CoverStorage.kt
@@ -18,14 +18,10 @@ import org.readium.r2.shared.util.http.fetchWithDecoder
 import org.readium.r2.testapp.utils.tryOrLog
 
 class CoverStorage(
-    appStorageDir: File,
+    private val appStorageDir: File,
     private val httpClient: HttpClient
 ) {
 
-    private val coverDir: File =
-        File(appStorageDir, "covers/")
-            .apply { if (!exists()) mkdirs() }
-
     suspend fun storeCover(publication: Publication, overrideUrl: AbsoluteUrl?): Try<File, Exception> {
         val coverBitmap: Bitmap? = overrideUrl?.fetchBitmap()
             ?: publication.cover()
@@ -59,12 +55,16 @@ class CoverStorage(
 
     private suspend fun storeCover(cover: Bitmap?): File =
         withContext(Dispatchers.IO) {
-            val coverImageFile = File(coverDir, "${UUID.randomUUID()}.png")
+            val coverImageFile = File(coverDir(), "${UUID.randomUUID()}.png")
             val resized = cover?.let { Bitmap.createScaledBitmap(it, 120, 200, true) }
             val fos = FileOutputStream(coverImageFile)
             resized?.compress(Bitmap.CompressFormat.PNG, 80, fos)
             fos.flush()
             fos.close()
             coverImageFile
         }
+
+    private fun coverDir(): File =
+        File(appStorageDir, "covers/")
+            .apply { if (!exists()) mkdirs() }
 }