Merge #1193

1193: re-use 'deleteWallet' function to avoid logic duplication r=KtorZ a=KtorZ



# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

#1071 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [ ] I have re-used 'deleteWallet' function to implement byron delete to avoid logic duplication And seemingly, share fixes when they are done..... !


# Comments

<!-- Additional comments or screenshots to attach if any -->



<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: KtorZ <matthias.benkort@gmail.com>

lib/core/src/Cardano/Wallet/Api/Server.hs

@@ -470,9 +470,8 @@ wallets ctx =
     :<|> getUTxOsStatistics ctx
 
 deleteWallet
-    :: forall ctx s t k n.
-        ( s ~ SeqState n k
-        , ctx ~ ApiLayer s t k
+    :: forall ctx s t k.
+        ( ctx ~ ApiLayer s t k
         )
     => ctx
     -> ApiT WalletId
@@ -1009,15 +1008,8 @@ deleteByronWallet
     => ApiLayer s t k
     -> ApiT WalletId
     -> Handler NoContent
-deleteByronWallet ctx (ApiT wid) = do
-    liftHandler $ withWorkerCtx ctx wid throwE $
-        \worker -> W.deleteWallet worker wid
-    liftIO $ Registry.remove re wid
-    liftIO $ (df ^. #removeDatabase) wid
-    return NoContent
-  where
-    re = ctx ^. workerRegistry @s @k
-    df = ctx ^. dbFactory @s @k
+deleteByronWallet =
+    deleteWallet
 
 getByronWallet
     :: forall t k. ()

lib/core/src/Cardano/Wallet/DB/Sqlite.hs

@@ -92,7 +92,7 @@ import Cardano.Wallet.Primitive.AddressDiscovery
 import Control.Arrow
     ( (***) )
 import Control.Concurrent.MVar
-    ( newEmptyMVar, putMVar, readMVar )
+    ( modifyMVar_, newMVar )
 import Control.DeepSeq
     ( NFData )
 import Control.Exception
@@ -111,6 +111,8 @@ import Data.Coerce
     ( coerce )
 import Data.Either
     ( isRight )
+import Data.Functor
+    ( ($>) )
 import Data.Generics.Internal.VL.Lens
     ( (^.) )
 import Data.List.Split
@@ -220,7 +222,7 @@ newDBFactory
        -- ^ Path to database directory, or Nothing for in-memory database
     -> IO (DBFactory IO s k)
 newDBFactory cfg tr mDatabaseDir = do
-    mvar <- newEmptyMVar
+    mvar <- newMVar mempty
     case mDatabaseDir of
         Nothing -> pure DBFactory
             { withDatabase = \_ action ->
@@ -229,17 +231,20 @@ newDBFactory cfg tr mDatabaseDir = do
                 pure ()
             }
         Just databaseDir -> pure DBFactory
-            { withDatabase = \wid action ->
-                withDBLayer cfg tracerDB (Just $ databaseFile wid) $ \(ctx,db) -> do
-                    putMVar mvar ctx
-                    action db
+            { withDatabase = \wid action -> do
+                withDBLayer cfg tracerDB (Just $ databaseFile wid)
+                    $ \(ctx, db) -> do
+                        modifyMVar_ mvar (pure . Map.insert wid ctx)
+                        action db
             , removeDatabase = \wid -> do
                 let files =
                         [ databaseFile wid
                         , databaseFile wid <> "-wal"
                         , databaseFile wid <> "-shm"
                         ]
-                readMVar mvar >>= destroyDBLayer
+                modifyMVar_ mvar $ \m -> case Map.lookup wid m of
+                    Nothing -> pure m
+                    Just ctx -> destroyDBLayer ctx $> Map.delete wid m
                 mapM_ removePathForcibly files
             }
           where

lib/core/test/unit/Cardano/Wallet/DB/SqliteSpec.hs

@@ -113,6 +113,8 @@ import Cardano.Wallet.Unsafe
     ( unsafeRunExceptT )
 import Control.Concurrent
     ( forkIO )
+import Control.Concurrent.MVar
+    ( newEmptyMVar, takeMVar, tryPutMVar )
 import Control.DeepSeq
     ( NFData )
 import Control.Exception
@@ -356,15 +358,18 @@ fileModeSpec =  do
             withSystemTempDirectory "DBFactory" $ \dir -> do
                 cfg <- defaultConfigTesting
                 DBFactory{..} <- newDBFactory cfg nullTracer (Just dir)
+                mvar <- newEmptyMVar
 
                 -- NOTE
                 -- Start a concurrent worker which makes action on the DB in
                 -- parallel to simulate activity.
                 _ <- forkIO $ withDatabase testWid $ \(DBLayer{..} :: TestDBSeq) -> do
                     forever $ do
                         cp <- atomically $ readCheckpoint $ PrimaryKey testWid
+                        _ <- tryPutMVar mvar ()
                         B8.putStrLn (B8.pack $ show cp)
 
+                takeMVar mvar
                 removeDatabase testWid
                 listDirectory dir `shouldReturn` mempty