Refactor Scim.Scim.UserSpec tests to use composable pieces (#2762)

* refactor: build UserSpec tests out of more composable pieces

changelog.d/5-internal/cleanup-scim-tests

@@ -0,0 +1 @@
+Refactor some internal Scim user tests

services/spar/test/Test/Spar/Scim/UserSpec.hs

@@ -6,16 +6,15 @@ import Brig.Types.User
 import Control.Monad.Except (runExceptT)
 import Data.Handle (parseHandle)
 import Data.Id
-import qualified Data.Json.Util
 import Imports
 import Polysemy
 import Polysemy.TinyLog
 import Spar.Scim.User (deleteScimUser)
 import Spar.Sem.BrigAccess
 import Spar.Sem.IdPConfigStore
-import Spar.Sem.IdPConfigStore.Mem (TypedState, idPToMem)
+import Spar.Sem.IdPConfigStore.Mem (idPToMem)
 import Spar.Sem.SAMLUserStore
-import Spar.Sem.SAMLUserStore.Mem (UserRefOrd, samlUserStoreToMem)
+import Spar.Sem.SAMLUserStore.Mem (samlUserStoreToMem)
 import qualified Spar.Sem.ScimExternalIdStore as ScimExternalIdStore
 import Spar.Sem.ScimExternalIdStore.Mem (scimExternalIdStoreToMem)
 import Spar.Sem.ScimUserTimesStore
@@ -25,7 +24,6 @@ import Test.Hspec
 import Test.QuickCheck
 import Web.Scim.Schema.Error
 import Wire.API.User
-import qualified Wire.API.User.Identity
 import Wire.API.User.Scim
 import Wire.Sem.Logger.TinyLog (discardTinyLogs)
 
@@ -36,33 +34,33 @@ spec = describe "deleteScimUser" $ do
     acc <- someActiveUser tokenInfo
     r <-
       interpretWithBrigAccessMock
-        (mockBrigForActiveUser acc AccountDeleted)
+        (mockBrig (withActiveUser acc) AccountDeleted)
         (deleteUserAndAssertDeletionInSpar acc tokenInfo)
-    handlerResult r `shouldBe` Right ()
+    r `shouldBe` Right ()
   it "returns an error when the account was deleted before" $ do
     tokenInfo <- generate arbitrary
     acc <- someActiveUser tokenInfo
     r <-
       interpretWithBrigAccessMock
-        (mockBrigForActiveUser acc AccountAlreadyDeleted)
+        (mockBrig (withActiveUser acc) AccountAlreadyDeleted)
         (deleteUserAndAssertDeletionInSpar acc tokenInfo)
-    handlerResult r `shouldBe` Left (notFound "user" ((idToText . userId . accountUser) acc))
+    r `shouldBe` Left (notFound "user" ((idToText . userId . accountUser) acc))
   it "returns an error when there never was an account" $ do
     uid <- generate arbitrary
     tokenInfo <- generate arbitrary
     r <-
       interpretWithBrigAccessMock
-        mockBrigForNonExistendUser
+        (mockBrig (const Nothing) NoUser)
         (runExceptT $ deleteScimUser tokenInfo uid)
-    handlerResult r `shouldBe` Left (notFound "user" (idToText uid))
+    r `shouldBe` Left (notFound "user" (idToText uid))
   it "returns no error when there was a partially deleted account" $ do
     uid <- generate arbitrary
     tokenInfo <- generate arbitrary
     r <-
       interpretWithBrigAccessMock
-        mockBrigForPartiallyDeletedUser
+        (mockBrig (const Nothing) AccountDeleted)
         (runExceptT $ deleteScimUser tokenInfo uid)
-    handlerResult r `shouldBe` Right ()
+    r `shouldBe` Right ()
 
 deleteUserAndAssertDeletionInSpar ::
   forall (r :: EffectRow).
@@ -99,77 +97,45 @@ type EffsWithoutBrigAccess =
      Final IO
    ]
 
-type Effs = BrigAccess ': EffsWithoutBrigAccess
-
-type InterpreterState =
-  ( Map (Data.Id.TeamId, Wire.API.User.Identity.Email) Data.Id.UserId,
-    ( Map Data.Id.UserId (Data.Json.Util.UTCTimeMillis, Data.Json.Util.UTCTimeMillis),
-      ( Map UserRefOrd UserId,
-        (Spar.Sem.IdPConfigStore.Mem.TypedState, Either ScimError ())
-      )
-    )
-  )
-
-handlerResult :: InterpreterState -> Either ScimError ()
-handlerResult = snd . snd . snd . snd
-
 interpretWithBrigAccessMock ::
-  ( Sem Effs (Either ScimError ()) ->
-    Sem EffsWithoutBrigAccess (Either ScimError ())
+  ( Sem (BrigAccess ': EffsWithoutBrigAccess) a ->
+    Sem EffsWithoutBrigAccess a
   ) ->
-  Sem Effs (Either ScimError ()) ->
-  IO InterpreterState
+  Sem (BrigAccess ': EffsWithoutBrigAccess) a ->
+  IO a
 interpretWithBrigAccessMock mock =
   runFinal
     . embedToFinal @IO
     . discardTinyLogs
-    . scimExternalIdStoreToMem
-    . scimUserTimesStoreToMem
-    . samlUserStoreToMem
-    . idPToMem
+    . ignoringState scimExternalIdStoreToMem
+    . ignoringState scimUserTimesStoreToMem
+    . ignoringState samlUserStoreToMem
+    . ignoringState idPToMem
     . mock
 
-mockBrigForNonExistendUser ::
-  forall (r :: EffectRow).
-  Members '[Embed IO] r =>
-  Sem (BrigAccess ': r) (Either ScimError ()) ->
-  Sem r (Either ScimError ())
-mockBrigForNonExistendUser = interpret $ \case
-  (GetAccount WithPendingInvitations _) -> pure Nothing
-  (Spar.Sem.BrigAccess.DeleteUser _) -> pure NoUser
-  _ -> do
-    liftIO $ expectationFailure $ "Unexpected effect (call to brig)"
-    error "Throw error here to avoid implementation of all cases."
-
-mockBrigForPartiallyDeletedUser ::
-  forall (r :: EffectRow).
-  Members '[Embed IO] r =>
-  Sem (BrigAccess ': r) (Either ScimError ()) ->
-  Sem r (Either ScimError ())
-mockBrigForPartiallyDeletedUser = interpret $ \case
-  (GetAccount WithPendingInvitations _) -> pure Nothing
-  (Spar.Sem.BrigAccess.DeleteUser _) -> pure AccountDeleted
-  _ -> do
-    liftIO $ expectationFailure $ "Unexpected effect (call to brig)"
-    error "Throw error here to avoid implementation of all cases."
+ignoringState :: Functor f => (a -> f (c, b)) -> a -> f b
+ignoringState f = fmap snd . f
 
-mockBrigForActiveUser ::
-  forall (r :: EffectRow).
+mockBrig ::
+  forall (r :: EffectRow) a.
   Members '[Embed IO] r =>
-  UserAccount ->
+  (UserId -> Maybe UserAccount) ->
   DeleteUserResult ->
-  Sem (BrigAccess ': r) (Either ScimError ()) ->
-  Sem r (Either ScimError ())
-mockBrigForActiveUser acc deletionResult = interpret $ \case
-  (GetAccount WithPendingInvitations uid) ->
-    if uid == (userId . accountUser) acc
-      then pure $ Just acc
-      else pure Nothing
-  (Spar.Sem.BrigAccess.DeleteUser _) -> pure deletionResult
+  Sem (BrigAccess ': r) a ->
+  Sem r a
+mockBrig lookup_user delete_response = interpret $ \case
+  (GetAccount WithPendingInvitations uid) -> pure $ lookup_user uid
+  (Spar.Sem.BrigAccess.DeleteUser _) -> pure delete_response
   _ -> do
     liftIO $ expectationFailure $ "Unexpected effect (call to brig)"
     error "Throw error here to avoid implementation of all cases."
 
+withActiveUser :: UserAccount -> UserId -> Maybe UserAccount
+withActiveUser acc uid =
+  if uid == (userId . accountUser) acc
+    then Just acc
+    else Nothing
+
 someActiveUser :: ScimTokenInfo -> IO UserAccount
 someActiveUser tokenInfo = do
   user <- generate arbitrary