Skip to content

Commit 0fa89dd

Browse files
harrytmthyharrytmthy
committed
feat: Enforce singleton instance per file
- SafeBox.create(...) now returns the same instance for a given fileName - Deprecated close() and closeWhenIdle() with no-op bodies - Removed SafeBoxBlobFileRegistry as it's no longer needed
1 parent da82150 commit 0fa89dd

File tree

6 files changed

+27
-227
lines changed

6 files changed

+27
-227
lines changed

‎safebox/src/androidTest/java/com/harrytmthy/safebox/SafeBoxTest.kt‎

Lines changed: 0 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,16 @@ import android.content.Context
2020
import android.content.SharedPreferences
2121
import androidx.test.core.app.ApplicationProvider
2222
import androidx.test.ext.junit.runners.AndroidJUnit4
23-
import com.harrytmthy.safebox.SafeBox.Companion.DEFAULT_KEY_ALIAS
24-
import com.harrytmthy.safebox.SafeBox.Companion.DEFAULT_VALUE_KEYSTORE_ALIAS
25-
import com.harrytmthy.safebox.state.SafeBoxState
2623
import com.harrytmthy.safebox.state.SafeBoxStateListener
2724
import kotlinx.coroutines.CoroutineDispatcher
2825
import kotlinx.coroutines.Dispatchers
2926
import kotlinx.coroutines.ExperimentalCoroutinesApi
3027
import kotlinx.coroutines.test.UnconfinedTestDispatcher
3128
import org.junit.After
3229
import org.junit.runner.RunWith
33-
import java.io.File
34-
import java.security.KeyStore
35-
import java.util.concurrent.CopyOnWriteArrayList
36-
import java.util.concurrent.atomic.AtomicBoolean
3730
import kotlin.test.Test
3831
import kotlin.test.assertContentEquals
3932
import kotlin.test.assertEquals
40-
import kotlin.test.assertFalse
4133
import kotlin.test.assertNull
4234
import kotlin.test.assertTrue
4335

@@ -56,15 +48,6 @@ class SafeBoxTest {
5648
safeBox.edit()
5749
.clear()
5850
.commit()
59-
safeBox.close()
60-
61-
KeyStore.getInstance("AndroidKeyStore").apply {
62-
load(null)
63-
deleteEntry(DEFAULT_VALUE_KEYSTORE_ALIAS)
64-
}
65-
66-
File(context.noBackupFilesDir, "$fileName.bin").delete()
67-
File(context.noBackupFilesDir, "$DEFAULT_KEY_ALIAS.bin").delete()
6851
}
6952

7053
@Test
@@ -179,68 +162,6 @@ class SafeBoxTest {
179162
assertContentEquals(expectedValueChanges, changedValues)
180163
}
181164

182-
@Test
183-
fun closeWhenIdle_shouldWaitUntilWritesAreDoneBeforeClosing() {
184-
val observedStates = CopyOnWriteArrayList<SafeBoxState>()
185-
val closed = AtomicBoolean(false)
186-
safeBox = createSafeBox(
187-
ioDispatcher = Dispatchers.IO,
188-
stateListener = SafeBoxStateListener { state ->
189-
observedStates.add(state)
190-
closed.set(state == SafeBoxState.CLOSED)
191-
},
192-
)
193-
repeat(5) {
194-
safeBox.edit()
195-
.putString("key", "value")
196-
.apply()
197-
}
198-
safeBox.closeWhenIdle()
199-
repeat(5) {
200-
safeBox.edit()
201-
.putString("key", "value")
202-
.apply()
203-
}
204-
while (!closed.get()) {
205-
Thread.sleep(3)
206-
}
207-
val expectedOnSlowInit = listOf(
208-
SafeBoxState.STARTING,
209-
SafeBoxState.WRITING,
210-
SafeBoxState.IDLE,
211-
SafeBoxState.CLOSED,
212-
)
213-
val expectedOnFastInit = listOf(
214-
SafeBoxState.STARTING,
215-
SafeBoxState.IDLE, // finished STARTING before launching any write operation
216-
SafeBoxState.WRITING,
217-
SafeBoxState.IDLE,
218-
SafeBoxState.CLOSED,
219-
)
220-
assertTrue(observedStates == expectedOnSlowInit || observedStates == expectedOnFastInit)
221-
}
222-
223-
@Test
224-
fun putString_shouldDoNothingAfterClosing() {
225-
val hasEmissionAfterClose = AtomicBoolean(false)
226-
val closed = AtomicBoolean(false)
227-
safeBox = createSafeBox(
228-
stateListener = SafeBoxStateListener { state ->
229-
if (closed.get()) {
230-
hasEmissionAfterClose.set(true)
231-
}
232-
closed.set(state == SafeBoxState.CLOSED)
233-
},
234-
)
235-
236-
safeBox.closeWhenIdle()
237-
safeBox.edit()
238-
.putString("key", "value")
239-
.commit()
240-
241-
assertFalse(hasEmissionAfterClose.get())
242-
}
243-
244165
private fun createSafeBox(
245166
ioDispatcher: CoroutineDispatcher = UnconfinedTestDispatcher(),
246167
stateListener: SafeBoxStateListener? = null,

‎safebox/src/main/java/com/harrytmthy/safebox/SafeBox.kt‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import com.harrytmthy.safebox.extensions.safeBoxScope
3232
import com.harrytmthy.safebox.extensions.toBytes
3333
import com.harrytmthy.safebox.extensions.toEncodedByteArray
3434
import com.harrytmthy.safebox.keystore.SecureRandomKeyProvider
35-
import com.harrytmthy.safebox.registry.SafeBoxBlobFileRegistry
3635
import com.harrytmthy.safebox.state.SafeBoxStateListener
3736
import com.harrytmthy.safebox.state.SafeBoxStateManager
3837
import com.harrytmthy.safebox.storage.Bytes
@@ -199,27 +198,25 @@ public class SafeBox private constructor(
199198
}
200199

201200
/**
201+
* **Deprecated:** SafeBox no longer supports instance closing.
202+
*
202203
* Immediately closes the underlying file channel and releases resources.
203-
* Also unregisters the file from [SafeBoxBlobFileRegistry], allowing a new SafeBox
204-
* instance to be created with the same filename.
205204
*
206205
* ⚠️ Once closed, this instance becomes *permanently unusable*. Any further access will fail.
207206
*
208207
* ⚠️ Only use this method when you're certain that no writes are in progress.
209208
*
210209
* Closing during an active write can result in data corruption or incomplete persistence.
211210
*/
211+
@Deprecated(message = "This method is now a no-op, as SafeBox is always active and reusable.")
212212
public fun close() {
213-
SafeBoxBlobFileRegistry.unregister(blobStore.getFileName())
214-
blobStore.close()
215-
keyCipherProvider.destroyKey()
216-
valueCipherProvider.destroyKey()
213+
// no-op
217214
}
218215

219216
/**
217+
* **Deprecated:** SafeBox no longer supports instance closing.
218+
*
220219
* Closes the underlying file channel only after all pending writes have completed.
221-
* Also unregisters the file from [SafeBoxBlobFileRegistry], allowing a new SafeBox
222-
* instance to be created with the same filename.
223220
*
224221
* ⚠️ Once closed, this instance becomes *permanently unusable*. Any further access will fail.
225222
*
@@ -228,8 +225,9 @@ public class SafeBox private constructor(
228225
* Internally, this launches a coroutine on [safeBoxScope] to wait until the SafeBox
229226
* becomes idle before releasing resources.
230227
*/
228+
@Deprecated(message = "This method is now a no-op, as SafeBox is always active and reusable.")
231229
public fun closeWhenIdle() {
232-
stateManager.closeWhenIdle(::close)
230+
// no-op
233231
}
234232

235233
override fun getAll(): Map<String, Any?> {
@@ -376,6 +374,9 @@ public class SafeBox private constructor(
376374
@VisibleForTesting
377375
internal const val DEFAULT_VALUE_KEYSTORE_ALIAS = "SafeBoxValue"
378376

377+
@VisibleForTesting
378+
internal val instances = ConcurrentHashMap<String, SafeBox>()
379+
379380
/**
380381
* Creates a [SafeBox] instance with secure defaults:
381382
* - Keys are deterministically encrypted using [ChaCha20CipherProvider].
@@ -411,7 +412,10 @@ public class SafeBox private constructor(
411412
ioDispatcher: CoroutineDispatcher = Dispatchers.IO,
412413
stateListener: SafeBoxStateListener? = null,
413414
): SafeBox {
414-
SafeBoxBlobFileRegistry.register(fileName)
415+
instances[fileName]?.let { safeBox ->
416+
stateListener?.let(safeBox.stateManager::setStateListener)
417+
return safeBox
418+
}
415419
val aesGcmCipherProvider = AesGcmCipherProvider.create(
416420
alias = valueKeyStoreAlias,
417421
aad = additionalAuthenticatedData,
@@ -428,6 +432,7 @@ public class SafeBox private constructor(
428432
val stateManager = SafeBoxStateManager(fileName, stateListener, ioDispatcher)
429433
val blobStore = SafeBoxBlobStore.create(context, fileName)
430434
return SafeBox(blobStore, keyCipherProvider, valueCipherProvider, stateManager)
435+
.also { instances[fileName] = it }
431436
}
432437

433438
/**
@@ -461,10 +466,14 @@ public class SafeBox private constructor(
461466
ioDispatcher: CoroutineDispatcher = Dispatchers.IO,
462467
stateListener: SafeBoxStateListener? = null,
463468
): SafeBox {
464-
SafeBoxBlobFileRegistry.register(fileName)
469+
instances[fileName]?.let { safeBox ->
470+
stateListener?.let(safeBox.stateManager::setStateListener)
471+
return safeBox
472+
}
465473
val stateManager = SafeBoxStateManager(fileName, stateListener, ioDispatcher)
466474
val blobStore = SafeBoxBlobStore.create(context, fileName)
467475
return SafeBox(blobStore, keyCipherProvider, valueCipherProvider, stateManager)
476+
.also { instances[fileName] = it }
468477
}
469478
}
470479
}

‎safebox/src/main/java/com/harrytmthy/safebox/registry/SafeBoxBlobFileRegistry.kt‎

Lines changed: 0 additions & 60 deletions
This file was deleted.

‎safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxState.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,6 @@ public enum class SafeBoxState {
5353
* Indicates that SafeBox has been closed and is no longer usable.
5454
* To access the same file again, a new SafeBox instance must be created.
5555
*/
56+
@Deprecated(message = "This state is never emitted, as SafeBox is always active and reusable.")
5657
CLOSED,
5758
}

‎safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxStateManager.kt‎

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package com.harrytmthy.safebox.state
1818

1919
import com.harrytmthy.safebox.SafeBox
2020
import com.harrytmthy.safebox.extensions.safeBoxScope
21-
import com.harrytmthy.safebox.state.SafeBoxState.CLOSED
2221
import com.harrytmthy.safebox.state.SafeBoxState.IDLE
2322
import com.harrytmthy.safebox.state.SafeBoxState.STARTING
2423
import com.harrytmthy.safebox.state.SafeBoxState.WRITING
@@ -27,7 +26,6 @@ import kotlinx.coroutines.CoroutineDispatcher
2726
import kotlinx.coroutines.CoroutineExceptionHandler
2827
import kotlinx.coroutines.launch
2928
import kotlinx.coroutines.runBlocking
30-
import java.util.concurrent.atomic.AtomicBoolean
3129
import java.util.concurrent.atomic.AtomicInteger
3230
import java.util.concurrent.atomic.AtomicReference
3331

@@ -39,17 +37,16 @@ import java.util.concurrent.atomic.AtomicReference
3937
*
4038
* Key behaviors:
4139
* - Tracks concurrent writes using an atomic counter.
42-
* - Waits for the blob store's initial read before permitting writes or close.
40+
* - Waits for the blob store's initial read before permitting writes.
4341
* - Guarantees transition to [IDLE] after all writes complete.
44-
* - Supports safe, deferred closing via [closeWhenIdle], ensuring no writes are in progress.
4542
*
4643
* @param fileName The unique file identifier associated with this SafeBox instance.
4744
* @param stateListener Optional listener for observing state transitions on this instance.
4845
* @param ioDispatcher Dispatcher used for coroutine-based I/O tasks.
4946
*/
5047
internal class SafeBoxStateManager(
5148
private val fileName: String,
52-
private val stateListener: SafeBoxStateListener?,
49+
private var stateListener: SafeBoxStateListener?,
5350
private val ioDispatcher: CoroutineDispatcher,
5451
) {
5552

@@ -59,7 +56,9 @@ internal class SafeBoxStateManager(
5956

6057
private val writeCompleted = AtomicReference<CompletableDeferred<Unit>>()
6158

62-
private val closed = AtomicBoolean(false)
59+
fun setStateListener(stateListener: SafeBoxStateListener?) {
60+
this.stateListener = stateListener
61+
}
6362

6463
inline fun launchWithStartingState(crossinline block: suspend () -> Unit) {
6564
updateState(STARTING)
@@ -75,9 +74,6 @@ internal class SafeBoxStateManager(
7574
}
7675

7776
inline fun launchCommitWithWritingState(crossinline block: suspend () -> Boolean): Boolean {
78-
if (closed.get()) {
79-
return false
80-
}
8177
if (concurrentWriteCount.incrementAndGet() == 1) {
8278
writeCompleted.set(CompletableDeferred())
8379
if (initialReadCompleted.isCompleted) {
@@ -96,9 +92,6 @@ internal class SafeBoxStateManager(
9692
exceptionHandler: CoroutineExceptionHandler,
9793
crossinline block: suspend () -> Unit,
9894
) {
99-
if (closed.get()) {
100-
return
101-
}
10295
if (concurrentWriteCount.incrementAndGet() == 1) {
10396
writeCompleted.set(CompletableDeferred())
10497
if (initialReadCompleted.isCompleted) {
@@ -113,22 +106,6 @@ internal class SafeBoxStateManager(
113106
}
114107
}
115108

116-
inline fun closeWhenIdle(crossinline block: () -> Unit) {
117-
if (concurrentWriteCount.get() == 0 && initialReadCompleted.isCompleted) {
118-
closed.set(true)
119-
block()
120-
updateState(CLOSED)
121-
return
122-
}
123-
safeBoxScope.launch(ioDispatcher) {
124-
initialReadCompleted.await()
125-
writeCompleted.get()?.await()
126-
closed.set(true)
127-
block()
128-
updateState(CLOSED)
129-
}
130-
}
131-
132109
private fun updateState(newState: SafeBoxState) {
133110
stateListener?.onStateChanged(newState)
134111
SafeBoxGlobalStateObserver.updateState(fileName, newState)

0 commit comments

Comments
 (0)