diff --git a/safebox/src/androidTest/java/com/harrytmthy/safebox/SafeBoxTest.kt b/safebox/src/androidTest/java/com/harrytmthy/safebox/SafeBoxTest.kt index 2bde020..42ba8ae 100644 --- a/safebox/src/androidTest/java/com/harrytmthy/safebox/SafeBoxTest.kt +++ b/safebox/src/androidTest/java/com/harrytmthy/safebox/SafeBoxTest.kt @@ -20,9 +20,6 @@ import android.content.Context import android.content.SharedPreferences import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 -import com.harrytmthy.safebox.SafeBox.Companion.DEFAULT_KEY_ALIAS -import com.harrytmthy.safebox.SafeBox.Companion.DEFAULT_VALUE_KEYSTORE_ALIAS -import com.harrytmthy.safebox.state.SafeBoxState import com.harrytmthy.safebox.state.SafeBoxStateListener import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers @@ -30,14 +27,9 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.UnconfinedTestDispatcher import org.junit.After import org.junit.runner.RunWith -import java.io.File -import java.security.KeyStore -import java.util.concurrent.CopyOnWriteArrayList -import java.util.concurrent.atomic.AtomicBoolean import kotlin.test.Test import kotlin.test.assertContentEquals import kotlin.test.assertEquals -import kotlin.test.assertFalse import kotlin.test.assertNull import kotlin.test.assertTrue @@ -56,15 +48,6 @@ class SafeBoxTest { safeBox.edit() .clear() .commit() - safeBox.close() - - KeyStore.getInstance("AndroidKeyStore").apply { - load(null) - deleteEntry(DEFAULT_VALUE_KEYSTORE_ALIAS) - } - - File(context.noBackupFilesDir, "$fileName.bin").delete() - File(context.noBackupFilesDir, "$DEFAULT_KEY_ALIAS.bin").delete() } @Test @@ -179,68 +162,6 @@ class SafeBoxTest { assertContentEquals(expectedValueChanges, changedValues) } - @Test - fun closeWhenIdle_shouldWaitUntilWritesAreDoneBeforeClosing() { - val observedStates = CopyOnWriteArrayList() - val closed = AtomicBoolean(false) - safeBox = createSafeBox( - ioDispatcher = Dispatchers.IO, - stateListener = SafeBoxStateListener { state -> - observedStates.add(state) - closed.set(state == SafeBoxState.CLOSED) - }, - ) - repeat(5) { - safeBox.edit() - .putString("key", "value") - .apply() - } - safeBox.closeWhenIdle() - repeat(5) { - safeBox.edit() - .putString("key", "value") - .apply() - } - while (!closed.get()) { - Thread.sleep(3) - } - val expectedOnSlowInit = listOf( - SafeBoxState.STARTING, - SafeBoxState.WRITING, - SafeBoxState.IDLE, - SafeBoxState.CLOSED, - ) - val expectedOnFastInit = listOf( - SafeBoxState.STARTING, - SafeBoxState.IDLE, // finished STARTING before launching any write operation - SafeBoxState.WRITING, - SafeBoxState.IDLE, - SafeBoxState.CLOSED, - ) - assertTrue(observedStates == expectedOnSlowInit || observedStates == expectedOnFastInit) - } - - @Test - fun putString_shouldDoNothingAfterClosing() { - val hasEmissionAfterClose = AtomicBoolean(false) - val closed = AtomicBoolean(false) - safeBox = createSafeBox( - stateListener = SafeBoxStateListener { state -> - if (closed.get()) { - hasEmissionAfterClose.set(true) - } - closed.set(state == SafeBoxState.CLOSED) - }, - ) - - safeBox.closeWhenIdle() - safeBox.edit() - .putString("key", "value") - .commit() - - assertFalse(hasEmissionAfterClose.get()) - } - private fun createSafeBox( ioDispatcher: CoroutineDispatcher = UnconfinedTestDispatcher(), stateListener: SafeBoxStateListener? = null, diff --git a/safebox/src/androidTest/java/com/harrytmthy/safebox/migration/SafeBoxMigrationHelperTest.kt b/safebox/src/androidTest/java/com/harrytmthy/safebox/migration/SafeBoxMigrationHelperTest.kt index fc967f3..ea5e7a7 100644 --- a/safebox/src/androidTest/java/com/harrytmthy/safebox/migration/SafeBoxMigrationHelperTest.kt +++ b/safebox/src/androidTest/java/com/harrytmthy/safebox/migration/SafeBoxMigrationHelperTest.kt @@ -64,7 +64,6 @@ class SafeBoxMigrationHelperTest { esp.edit() .clear() .commit() - safeBox.close() context.deleteSharedPreferences(fileEsp) context.deleteFile("$fileSafeBox.bin") } diff --git a/safebox/src/androidTest/java/com/harrytmthy/safebox/storage/SafeBoxBlobStoreTest.kt b/safebox/src/androidTest/java/com/harrytmthy/safebox/storage/SafeBoxBlobStoreTest.kt index 18831af..1fa1b18 100644 --- a/safebox/src/androidTest/java/com/harrytmthy/safebox/storage/SafeBoxBlobStoreTest.kt +++ b/safebox/src/androidTest/java/com/harrytmthy/safebox/storage/SafeBoxBlobStoreTest.kt @@ -43,7 +43,6 @@ class SafeBoxBlobStoreTest { @After fun teardown() { - blobStore.close() File(context.noBackupFilesDir, "$fileName.bin").delete() } diff --git a/safebox/src/main/java/com/harrytmthy/safebox/SafeBox.kt b/safebox/src/main/java/com/harrytmthy/safebox/SafeBox.kt index b3b2daa..a583abe 100644 --- a/safebox/src/main/java/com/harrytmthy/safebox/SafeBox.kt +++ b/safebox/src/main/java/com/harrytmthy/safebox/SafeBox.kt @@ -32,7 +32,6 @@ import com.harrytmthy.safebox.extensions.safeBoxScope import com.harrytmthy.safebox.extensions.toBytes import com.harrytmthy.safebox.extensions.toEncodedByteArray import com.harrytmthy.safebox.keystore.SecureRandomKeyProvider -import com.harrytmthy.safebox.registry.SafeBoxBlobFileRegistry import com.harrytmthy.safebox.state.SafeBoxStateListener import com.harrytmthy.safebox.state.SafeBoxStateManager import com.harrytmthy.safebox.storage.Bytes @@ -199,9 +198,9 @@ public class SafeBox private constructor( } /** + * **Deprecated:** SafeBox no longer supports instance closing. + * * Immediately closes the underlying file channel and releases resources. - * Also unregisters the file from [SafeBoxBlobFileRegistry], allowing a new SafeBox - * instance to be created with the same filename. * * ⚠️ Once closed, this instance becomes *permanently unusable*. Any further access will fail. * @@ -209,17 +208,15 @@ public class SafeBox private constructor( * * Closing during an active write can result in data corruption or incomplete persistence. */ + @Deprecated(message = "This method is now a no-op, as SafeBox is always active and reusable.") public fun close() { - SafeBoxBlobFileRegistry.unregister(blobStore.getFileName()) - blobStore.close() - keyCipherProvider.destroyKey() - valueCipherProvider.destroyKey() + // no-op } /** + * **Deprecated:** SafeBox no longer supports instance closing. + * * Closes the underlying file channel only after all pending writes have completed. - * Also unregisters the file from [SafeBoxBlobFileRegistry], allowing a new SafeBox - * instance to be created with the same filename. * * ⚠️ Once closed, this instance becomes *permanently unusable*. Any further access will fail. * @@ -228,8 +225,9 @@ public class SafeBox private constructor( * Internally, this launches a coroutine on [safeBoxScope] to wait until the SafeBox * becomes idle before releasing resources. */ + @Deprecated(message = "This method is now a no-op, as SafeBox is always active and reusable.") public fun closeWhenIdle() { - stateManager.closeWhenIdle(::close) + // no-op } override fun getAll(): Map { @@ -376,6 +374,9 @@ public class SafeBox private constructor( @VisibleForTesting internal const val DEFAULT_VALUE_KEYSTORE_ALIAS = "SafeBoxValue" + @VisibleForTesting + internal val instances = ConcurrentHashMap() + /** * Creates a [SafeBox] instance with secure defaults: * - Keys are deterministically encrypted using [ChaCha20CipherProvider]. @@ -411,7 +412,10 @@ public class SafeBox private constructor( ioDispatcher: CoroutineDispatcher = Dispatchers.IO, stateListener: SafeBoxStateListener? = null, ): SafeBox { - SafeBoxBlobFileRegistry.register(fileName) + instances[fileName]?.let { safeBox -> + stateListener?.let(safeBox.stateManager::setStateListener) + return safeBox + } val aesGcmCipherProvider = AesGcmCipherProvider.create( alias = valueKeyStoreAlias, aad = additionalAuthenticatedData, @@ -428,6 +432,7 @@ public class SafeBox private constructor( val stateManager = SafeBoxStateManager(fileName, stateListener, ioDispatcher) val blobStore = SafeBoxBlobStore.create(context, fileName) return SafeBox(blobStore, keyCipherProvider, valueCipherProvider, stateManager) + .also { instances[fileName] = it } } /** @@ -461,10 +466,14 @@ public class SafeBox private constructor( ioDispatcher: CoroutineDispatcher = Dispatchers.IO, stateListener: SafeBoxStateListener? = null, ): SafeBox { - SafeBoxBlobFileRegistry.register(fileName) + instances[fileName]?.let { safeBox -> + stateListener?.let(safeBox.stateManager::setStateListener) + return safeBox + } val stateManager = SafeBoxStateManager(fileName, stateListener, ioDispatcher) val blobStore = SafeBoxBlobStore.create(context, fileName) return SafeBox(blobStore, keyCipherProvider, valueCipherProvider, stateManager) + .also { instances[fileName] = it } } } } diff --git a/safebox/src/main/java/com/harrytmthy/safebox/registry/SafeBoxBlobFileRegistry.kt b/safebox/src/main/java/com/harrytmthy/safebox/registry/SafeBoxBlobFileRegistry.kt deleted file mode 100644 index 476b73b..0000000 --- a/safebox/src/main/java/com/harrytmthy/safebox/registry/SafeBoxBlobFileRegistry.kt +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright 2025 Harry Timothy Tumalewa - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.harrytmthy.safebox.registry - -import com.harrytmthy.safebox.SafeBox -import com.harrytmthy.safebox.state.SafeBoxGlobalStateObserver -import java.util.Collections -import java.util.concurrent.ConcurrentHashMap - -/** - * An internal registry that tracks active SafeBox blob files. - * - * Prevents multiple [SafeBox] instances from accessing the same file simultaneously. - * This ensures thread safety and prevents corruption due to concurrent `FileChannel` access. - * - * This registry is internal-only and not intended for external observation. - * Please use [SafeBoxGlobalStateObserver] to listen for state changes. - */ -internal object SafeBoxBlobFileRegistry { - - private val registry: MutableSet = Collections.newSetFromMap(ConcurrentHashMap()) - - /** - * Registers the given file name as currently in use. - * - * @param fileName The file name associated with a SafeBox instance. - * @throws IllegalStateException if the file is already registered. - */ - @Throws(IllegalStateException::class) - fun register(fileName: String) { - if (registry.contains(fileName)) { - error("SafeBox with file name '$fileName' is already in use. Please close it first.") - } - registry.add(fileName) - } - - /** - * Unregisters the given file name, allowing the creation of a new SafeBox instances with - * an existing file name. - * - * @param fileName The file name to unregister. - */ - fun unregister(fileName: String) { - registry.remove(fileName) - } -} diff --git a/safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxState.kt b/safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxState.kt index b27f2e5..6f23539 100644 --- a/safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxState.kt +++ b/safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxState.kt @@ -53,5 +53,6 @@ public enum class SafeBoxState { * Indicates that SafeBox has been closed and is no longer usable. * To access the same file again, a new SafeBox instance must be created. */ + @Deprecated(message = "This state is never emitted, as SafeBox is always active and reusable.") CLOSED, } diff --git a/safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxStateListener.kt b/safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxStateListener.kt index 1302d04..a74fd93 100644 --- a/safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxStateListener.kt +++ b/safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxStateListener.kt @@ -20,7 +20,7 @@ package com.harrytmthy.safebox.state * A listener interface for observing [SafeBoxState] changes tied to a specific SafeBox file. * * This is typically used in non-singleton SafeBox use cases (e.g. ViewModel-scoped), - * where consumers need to track if the instance is writing, idle, or closed. + * where consumers need to track if the instance is starting, writing, or idle. * * @see SafeBoxState */ diff --git a/safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxStateManager.kt b/safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxStateManager.kt index 293c35f..9dafcce 100644 --- a/safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxStateManager.kt +++ b/safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxStateManager.kt @@ -18,7 +18,6 @@ package com.harrytmthy.safebox.state import com.harrytmthy.safebox.SafeBox import com.harrytmthy.safebox.extensions.safeBoxScope -import com.harrytmthy.safebox.state.SafeBoxState.CLOSED import com.harrytmthy.safebox.state.SafeBoxState.IDLE import com.harrytmthy.safebox.state.SafeBoxState.STARTING import com.harrytmthy.safebox.state.SafeBoxState.WRITING @@ -27,7 +26,6 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking -import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicInteger import java.util.concurrent.atomic.AtomicReference @@ -39,9 +37,8 @@ import java.util.concurrent.atomic.AtomicReference * * Key behaviors: * - Tracks concurrent writes using an atomic counter. - * - Waits for the blob store's initial read before permitting writes or close. + * - Waits for the blob store's initial read before permitting writes. * - Guarantees transition to [IDLE] after all writes complete. - * - Supports safe, deferred closing via [closeWhenIdle], ensuring no writes are in progress. * * @param fileName The unique file identifier associated with this SafeBox instance. * @param stateListener Optional listener for observing state transitions on this instance. @@ -49,7 +46,7 @@ import java.util.concurrent.atomic.AtomicReference */ internal class SafeBoxStateManager( private val fileName: String, - private val stateListener: SafeBoxStateListener?, + private var stateListener: SafeBoxStateListener?, private val ioDispatcher: CoroutineDispatcher, ) { @@ -59,7 +56,9 @@ internal class SafeBoxStateManager( private val writeCompleted = AtomicReference>() - private val closed = AtomicBoolean(false) + fun setStateListener(stateListener: SafeBoxStateListener?) { + this.stateListener = stateListener + } inline fun launchWithStartingState(crossinline block: suspend () -> Unit) { updateState(STARTING) @@ -75,9 +74,6 @@ internal class SafeBoxStateManager( } inline fun launchCommitWithWritingState(crossinline block: suspend () -> Boolean): Boolean { - if (closed.get()) { - return false - } if (concurrentWriteCount.incrementAndGet() == 1) { writeCompleted.set(CompletableDeferred()) if (initialReadCompleted.isCompleted) { @@ -96,9 +92,6 @@ internal class SafeBoxStateManager( exceptionHandler: CoroutineExceptionHandler, crossinline block: suspend () -> Unit, ) { - if (closed.get()) { - return - } if (concurrentWriteCount.incrementAndGet() == 1) { writeCompleted.set(CompletableDeferred()) if (initialReadCompleted.isCompleted) { @@ -113,22 +106,6 @@ internal class SafeBoxStateManager( } } - inline fun closeWhenIdle(crossinline block: () -> Unit) { - if (concurrentWriteCount.get() == 0 && initialReadCompleted.isCompleted) { - closed.set(true) - block() - updateState(CLOSED) - return - } - safeBoxScope.launch(ioDispatcher) { - initialReadCompleted.await() - writeCompleted.get()?.await() - closed.set(true) - block() - updateState(CLOSED) - } - } - private fun updateState(newState: SafeBoxState) { stateListener?.onStateChanged(newState) SafeBoxGlobalStateObserver.updateState(fileName, newState) diff --git a/safebox/src/main/java/com/harrytmthy/safebox/storage/SafeBoxBlobStore.kt b/safebox/src/main/java/com/harrytmthy/safebox/storage/SafeBoxBlobStore.kt index 2bb2c63..d28ccc6 100644 --- a/safebox/src/main/java/com/harrytmthy/safebox/storage/SafeBoxBlobStore.kt +++ b/safebox/src/main/java/com/harrytmthy/safebox/storage/SafeBoxBlobStore.kt @@ -170,14 +170,6 @@ internal class SafeBoxBlobStore private constructor(private val file: File) { */ internal fun getFileName(): String = file.nameWithoutExtension - /** - * Closes the underlying file channel and releases associated resources. - * Must be called when SafeBoxBlobStore is no longer in use to prevent memory leaks. - */ - internal fun close() { - channel.close() - } - private fun writeAtOffset( encryptedKey: Bytes, encryptedValue: ByteArray, diff --git a/safebox/src/test/java/com/harrytmthy/safebox/registry/SafeBoxBlobFileRegistryTest.kt b/safebox/src/test/java/com/harrytmthy/safebox/registry/SafeBoxBlobFileRegistryTest.kt deleted file mode 100644 index f0d9d0f..0000000 --- a/safebox/src/test/java/com/harrytmthy/safebox/registry/SafeBoxBlobFileRegistryTest.kt +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright 2025 Harry Timothy Tumalewa - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.harrytmthy.safebox.registry - -import kotlin.test.Test -import kotlin.test.assertFailsWith - -class SafeBoxBlobFileRegistryTest { - - @Test - fun `register and unregister should succeed`() { - val fileName = "test_file" - - SafeBoxBlobFileRegistry.register(fileName) - SafeBoxBlobFileRegistry.unregister(fileName) - SafeBoxBlobFileRegistry.register(fileName) - } - - @Test - fun `register twice should throw IllegalStateException`() { - val fileName = "duplicate_file" - - SafeBoxBlobFileRegistry.register(fileName) - - assertFailsWith { SafeBoxBlobFileRegistry.register(fileName) } - } - - @Test - fun `unregister non-registered file should succeed`() { - val fileName = "nonexistent_file" - - SafeBoxBlobFileRegistry.unregister(fileName) - } -}