From d6f0d28a1fe7f9b3b01a05d177c1e604e893fa8f Mon Sep 17 00:00:00 2001 From: Sarah Vaupel <> Date: Tue, 28 Jul 2020 19:42:09 +0200 Subject: [PATCH 01/17] fix: correct (switch) sheetHint and sheetSolution mail templates --- templates/mail/sheetHint.hamlet | 6 +++--- templates/mail/sheetSolution.hamlet | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/templates/mail/sheetHint.hamlet b/templates/mail/sheetHint.hamlet index 621f15227..a20ded146 100644 --- a/templates/mail/sheetHint.hamlet +++ b/templates/mail/sheetHint.hamlet @@ -11,11 +11,11 @@ $newline never }

- _{MsgMailSheetSolutionIntro (CI.original courseName) termDesc sheetName} + _{MsgMailSheetHintIntro (CI.original courseName) termDesc sheetName}

#{sheetName}

- - _{MsgSheetSolution} + + _{MsgSheetHint} ^{editNotifications} diff --git a/templates/mail/sheetSolution.hamlet b/templates/mail/sheetSolution.hamlet index a20ded146..621f15227 100644 --- a/templates/mail/sheetSolution.hamlet +++ b/templates/mail/sheetSolution.hamlet @@ -11,11 +11,11 @@ $newline never }

- _{MsgMailSheetHintIntro (CI.original courseName) termDesc sheetName} + _{MsgMailSheetSolutionIntro (CI.original courseName) termDesc sheetName}

#{sheetName}

- - _{MsgSheetHint} + + _{MsgSheetSolution} ^{editNotifications} From 9213b7554a6da2a40ea0c82ad4601a951dd7ebb4 Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Tue, 28 Jul 2020 20:04:44 +0200 Subject: [PATCH 02/17] feat(failover): treat alternatives cyclically --- src/Utils/Failover.hs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Utils/Failover.hs b/src/Utils/Failover.hs index 6eed49d4b..3e745b5ed 100644 --- a/src/Utils/Failover.hs +++ b/src/Utils/Failover.hs @@ -161,9 +161,12 @@ withFailover' testTarget' f@Failover{..} mode detAcceptable act = withFailoverRe $logErrorS "withFailover'" $ tshow (hashUnique alreadyTested) <> " recording failure for item " <> failoverLabel atomically . modifyTVar failover $ \failover' -> if | views (P.focus . _failoverReferences) (Set.member currentlyTesting) failover' - -> fromMaybe failover' $ P.next failover' + -> fromMaybe (goFirst failover') $ P.next failover' | otherwise -> failover' + where goFirst l = case P.previous l of + Nothing -> l + Just l' -> goFirst l' $logDebugS "withFailover'" $ tshow (hashUnique alreadyTested) <> " using item " <> failoverLabel res' <- handleAll (\err -> $logErrorS "withFailover'" (tshow (hashUnique alreadyTested) <> " exception during act or detAcceptable: " <> tshow err) >> recordFailure >> throwM err) $ From 22882c1fa061106cfc06a8e0536cffcd8b5f5b0f Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Tue, 28 Jul 2020 20:21:02 +0200 Subject: [PATCH 03/17] refactor: hlint --- src/Utils/Failover.hs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Utils/Failover.hs b/src/Utils/Failover.hs index 3e745b5ed..e8c51dae7 100644 --- a/src/Utils/Failover.hs +++ b/src/Utils/Failover.hs @@ -164,9 +164,7 @@ withFailover' testTarget' f@Failover{..} mode detAcceptable act = withFailoverRe -> fromMaybe (goFirst failover') $ P.next failover' | otherwise -> failover' - where goFirst l = case P.previous l of - Nothing -> l - Just l' -> goFirst l' + where goFirst l = maybe l goFirst $ P.previous l $logDebugS "withFailover'" $ tshow (hashUnique alreadyTested) <> " using item " <> failoverLabel res' <- handleAll (\err -> $logErrorS "withFailover'" (tshow (hashUnique alreadyTested) <> " exception during act or detAcceptable: " <> tshow err) >> recordFailure >> throwM err) $ From ec42d834ee627401849910c44ed18ee696c8fc76 Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Tue, 28 Jul 2020 20:54:55 +0200 Subject: [PATCH 04/17] fix(campus-auth): properly handle login failures --- src/Auth/LDAP.hs | 45 +++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/src/Auth/LDAP.hs b/src/Auth/LDAP.hs index 0e52e4f13..dac6bd1fd 100644 --- a/src/Auth/LDAP.hs +++ b/src/Auth/LDAP.hs @@ -189,28 +189,33 @@ campusLogin pool mode = AuthPlugin{..} searchResults <- findUser conf ldap campusIdent [ldapUserPrincipalName] case searchResults of [Ldap.SearchEntry (Ldap.Dn userDN) userAttrs] - | [principalName] <- fold [ v | (k, v) <- userAttrs, k == ldapUserPrincipalName ] + | [principalName] <- nub $ fold [ v | (k, v) <- userAttrs, k == ldapUserPrincipalName ] , Right credsIdent <- Text.decodeUtf8' principalName - -> Right (userDN, credsIdent) <$ Ldap.bind ldap (Ldap.Dn credsIdent) (Ldap.Password $ Text.encodeUtf8 campusPassword) - other -> return $ Left other + -> handleIf isInvalidCredentials (return . Left) $ do + Ldap.bind ldap (Ldap.Dn credsIdent) . Ldap.Password $ Text.encodeUtf8 campusPassword + return . Right $ Right (userDN, credsIdent) + other -> return . Right $ Left other case ldapResult of - Left err - | LdapError (Ldap.ResponseError (Ldap.ResponseErrorCode _ Ldap.InvalidCredentials _ _)) <- err - -> do - $logDebugS apName "Invalid credentials" - observeLoginOutcome apName LoginInvalidCredentials - loginErrorMessageI LoginR Msg.InvalidLogin - | otherwise -> do - $logErrorS apName $ "Error during login: " <> tshow err - observeLoginOutcome apName LoginError - loginErrorMessageI LoginR Msg.AuthError - Right (Right (userDN, credsIdent)) -> do - observeLoginOutcome apName LoginSuccessful - setCredsRedirect $ Creds apName credsIdent [("DN", userDN)] - Right (Left searchResults) -> do - $logWarnS apName $ "Could not extract principal name: " <> tshow searchResults + Left err -> do + $logErrorS apName $ "Error during login: " <> tshow err observeLoginOutcome apName LoginError loginErrorMessageI LoginR Msg.AuthError + Right (Left _bindErr) -> do + $logDebugS apName "Invalid credentials" + observeLoginOutcome apName LoginInvalidCredentials + loginErrorMessageI LoginR Msg.InvalidLogin + Right (Right (Left searchResults)) + | null searchResults -> do + $logDebugS apName "User not found" + observeLoginOutcome apName LoginInvalidCredentials + loginErrorMessageI LoginR Msg.InvalidLogin + | otherwise -> do + $logWarnS apName $ "Could not extract principal name: " <> tshow searchResults + observeLoginOutcome apName LoginError + loginErrorMessageI LoginR Msg.AuthError + Right (Right (Right (userDN, credsIdent))) -> do + observeLoginOutcome apName LoginSuccessful + setCredsRedirect $ Creds apName credsIdent [("DN", userDN)] maybe (redirect $ tp LoginR) return resp apDispatch _ [] = badMethod @@ -228,3 +233,7 @@ campusLogin pool mode = AuthPlugin{..} , formAnchor = Just "login--campus" :: Maybe Text } $(widgetFile "widgets/campus-login/campus-login-form") + + isInvalidCredentials = \case + Ldap.ResponseErrorCode _ Ldap.InvalidCredentials _ _ -> True + _other -> False From c0b79274d8e9bf1a3f7296f256381a9760298342 Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Tue, 28 Jul 2020 21:05:09 +0200 Subject: [PATCH 05/17] chore(release): 18.3.0 --- CHANGELOG.md | 15 +++++++++++++++ package-lock.json | 2 +- package.json | 2 +- package.yaml | 2 +- 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8706f0f80..ece61ece0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [18.3.0](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/compare/v18.2.2...v18.3.0) (2020-07-28) + + +### Bug Fixes + +* **campus-auth:** properly handle login failures ([ec42d83](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/ec42d83)) +* correct (switch) sheetHint and sheetSolution mail templates ([d6f0d28](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/d6f0d28)) + + +### Features + +* **failover:** treat alternatives cyclically ([9213b75](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/9213b75)) + + + ### [18.2.2](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/compare/v18.2.1...v18.2.2) (2020-07-23) diff --git a/package-lock.json b/package-lock.json index 6b5c7e762..c86aab431 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "uni2work", - "version": "18.2.2", + "version": "18.3.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 3131405ae..bf15ad329 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "uni2work", - "version": "18.2.2", + "version": "18.3.0", "description": "", "keywords": [], "author": "", diff --git a/package.yaml b/package.yaml index b1c6e1af3..cfabb80ef 100644 --- a/package.yaml +++ b/package.yaml @@ -1,5 +1,5 @@ name: uniworx -version: 18.2.2 +version: 18.3.0 dependencies: - base From d47d6aa6ccaa3007aae64f555ceec519dd03f029 Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Wed, 29 Jul 2020 09:46:38 +0200 Subject: [PATCH 06/17] fix: suppress exceptions relating to expired sessions --- src/Application.hs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/Application.hs b/src/Application.hs index d6f72c080..65bdf4ea1 100644 --- a/src/Application.hs +++ b/src/Application.hs @@ -98,6 +98,8 @@ import qualified Web.ServerSession.Backend.Acid as Acid import qualified Ldap.Client as Ldap (Host(Plain, Tls)) import qualified Network.Minio as Minio + +import Web.ServerSession.Core (StorageException(..)) -- Import all relevant handler modules here. -- (HPack takes care to add new modules to our cabal file nowadays.) @@ -433,7 +435,7 @@ warpSettings foundation = defaultSettings & setHost (foundation ^. _appHost) & setPort (foundation ^. _appPort) & setOnException (\_req e -> - when (defaultShouldDisplayException e) $ do + when (shouldDisplayException e) $ do logger <- readTVarIO . snd $ appLogger foundation messageLoggerSource foundation @@ -443,6 +445,16 @@ warpSettings foundation = defaultSettings LevelError (toLogStr $ "Exception from Warp: " ++ show e) ) + where + shouldDisplayException e = and + [ defaultShouldDisplayException e + , case fromException e of + Just (SessionDoesNotExist{} :: StorageException (MemcachedSqlStorage SessionMap)) -> False + _other -> True + , case fromException e of + Just (SessionDoesNotExist{} :: StorageException (AcidStorage SessionMap)) -> False + _other -> True + ] getAppDevSettings, getAppSettings :: MonadIO m => m AppSettings From a0392dd329c871ed855ee832ee97230d9c72d59e Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Fri, 31 Jul 2020 18:00:30 +0200 Subject: [PATCH 07/17] fix: better concurrency behaviour --- models/files.model | 7 ++++++- src/Handler/Allocation/Application.hs | 4 ++-- src/Handler/Course/Register.hs | 9 ++++---- src/Handler/Sheet/Pseudonym.hs | 2 -- src/Handler/Submission/Helper.hs | 2 +- src/Handler/Utils/Form.hs | 9 ++++---- src/Handler/Utils/Submission.hs | 2 +- src/Import/NoModel.hs | 1 + src/Jobs.hs | 2 -- src/Jobs/Handler/Files.hs | 30 ++++++++++++--------------- src/Jobs/Queue.hs | 1 - src/Utils/Files.hs | 18 ++++++++++++---- 12 files changed, 48 insertions(+), 39 deletions(-) diff --git a/models/files.model b/models/files.model index eae0276d7..fcf0b3809 100644 --- a/models/files.model +++ b/models/files.model @@ -5,4 +5,9 @@ FileContent SessionFile content FileContentReference Maybe - touched UTCTime \ No newline at end of file + touched UTCTime + +FileLock + content FileContentReference + instance InstanceId + time UTCTime \ No newline at end of file diff --git a/src/Handler/Allocation/Application.hs b/src/Handler/Allocation/Application.hs index c5229cbfb..73d898959 100644 --- a/src/Handler/Allocation/Application.hs +++ b/src/Handler/Allocation/Application.hs @@ -260,7 +260,7 @@ editApplicationR maId uid cid mAppId afMode allowAction postAction = do if | BtnAllocationApply <- afAction , allowAction afAction - -> runDB $ do + -> runDB . setSerializable $ do haveOld <- exists [ CourseApplicationCourse ==. cid , CourseApplicationUser ==. uid , CourseApplicationAllocation ==. maId @@ -291,7 +291,7 @@ editApplicationR maId uid cid mAppId afMode allowAction postAction = do | is _BtnAllocationApplicationEdit afAction || is _BtnAllocationApplicationRate afAction , allowAction afAction , Just appId <- mAppId - -> runDB $ do + -> runDB . setSerializable $ do now <- liftIO getCurrentTime changes <- if diff --git a/src/Handler/Course/Register.hs b/src/Handler/Course/Register.hs index 5e0165a1c..66b9b3566 100644 --- a/src/Handler/Course/Register.hs +++ b/src/Handler/Course/Register.hs @@ -175,8 +175,9 @@ postCRegisterR tid ssh csh = do formResult regResult $ \CourseRegisterForm{..} -> do cTime <- liftIO getCurrentTime let + doApplication = courseApplicationsRequired || is _Just (void crfApplicationText <|> void crfApplicationFiles) mkApplication - | courseApplicationsRequired || is _Just (void crfApplicationText <|> void crfApplicationFiles) + | doApplication = void <$> do appIds <- selectKeysList [ CourseApplicationAllocation ==. Nothing, CourseApplicationCourse ==. cid, CourseApplicationUser ==. uid ] [] appRes <- case appIds of @@ -210,12 +211,12 @@ postCRegisterR tid ssh csh = do ] case courseRegisterButton of - BtnCourseRegister -> runDB $ do + BtnCourseRegister -> runDB . bool id setSerializable doApplication $ do regOk <- (\app reg -> (, reg) <$> app) <$> mkApplication <*> mkRegistration case regOk of Nothing -> transactionUndo Just _ -> addMessageIconI Success IconEnrolTrue MsgCourseRegisterOk - BtnCourseDeregister -> runDB $ do + BtnCourseDeregister -> runDB . setSerializable $ do part <- fmap (assertM . has $ _entityVal . _courseParticipantState . _CourseParticipantActive) . getBy $ UniqueParticipant uid cid forM_ part $ \(Entity _partId CourseParticipant{..}) -> do deregisterParticipant uid cid @@ -237,7 +238,7 @@ postCRegisterR tid ssh csh = do when courseDeregisterNoShow . runConduit $ selectKeys [ ExamCourse ==. cid ] [] .| C.mapM_ recordNoShow addMessageIconI Info IconEnrolFalse MsgCourseDeregisterOk - BtnCourseApply -> runDB $ do + BtnCourseApply -> runDB . setSerializable $ do regOk <- mkApplication case regOk of Nothing -> transactionUndo diff --git a/src/Handler/Sheet/Pseudonym.hs b/src/Handler/Sheet/Pseudonym.hs index b9c055fa6..f269ef18c 100644 --- a/src/Handler/Sheet/Pseudonym.hs +++ b/src/Handler/Sheet/Pseudonym.hs @@ -7,8 +7,6 @@ import Import import Handler.Utils -import Utils.Sql - data ButtonGeneratePseudonym = BtnGenerate deriving (Enum, Eq, Ord, Bounded, Read, Show, Generic, Typeable) diff --git a/src/Handler/Submission/Helper.hs b/src/Handler/Submission/Helper.hs index d808b501d..286481651 100644 --- a/src/Handler/Submission/Helper.hs +++ b/src/Handler/Submission/Helper.hs @@ -318,7 +318,7 @@ submissionHelper tid ssh csh shn mcid = do , formEncoding = formEnctype } - mCID <- fmap join . msgSubmissionErrors . runDBJobs $ do + mCID <- fmap join . msgSubmissionErrors . runDBJobs . setSerializable $ do (Entity shid Sheet{..}, _, _, _, isLecturer, _, msubmission, _) <- hoist lift getSheetInfo submissions <- E.select . E.from $ \(submission `E.InnerJoin` submissionUser) -> do diff --git a/src/Handler/Utils/Form.hs b/src/Handler/Utils/Form.hs index 86e56b95a..0345765f8 100644 --- a/src/Handler/Utils/Form.hs +++ b/src/Handler/Utils/Form.hs @@ -899,13 +899,14 @@ genericFileField mkOpts = Field{..} handleUpload FileField{fieldMaxFileSize} mIdent = C.filter (\File{..} -> maybe (const True) (>) fieldMaxFileSize $ maybe 0 (fromIntegral . olength) fileContent) .| sinkFiles - .| maybe (C.map id) mkSessionFile mIdent + .| C.mapM mkSessionFile where - mkSessionFile ident = C.mapM $ \fRef@FileReference{..} -> fRef <$ do + mkSessionFile fRef@FileReference{..} = fRef <$ do now <- liftIO getCurrentTime sfId <- insert $ SessionFile fileReferenceContent now - modifySessionJson SessionFiles $ \(fromMaybe mempty -> MergeHashMap old) -> - Just . MergeHashMap $ HashMap.insert ident (Map.insert fileReferenceTitle (sfId, fileReferenceModified) $ HashMap.findWithDefault mempty ident old) old + whenIsJust mIdent $ \ident -> + modifySessionJson SessionFiles $ \(fromMaybe mempty -> MergeHashMap old) -> + Just . MergeHashMap $ HashMap.insert ident (Map.insert fileReferenceTitle (sfId, fileReferenceModified) $ HashMap.findWithDefault mempty ident old) old _FileTitle :: Prism' Text FilePath diff --git a/src/Handler/Utils/Submission.hs b/src/Handler/Utils/Submission.hs index 7af8f3d57..8d0a895cc 100644 --- a/src/Handler/Utils/Submission.hs +++ b/src/Handler/Utils/Submission.hs @@ -900,7 +900,7 @@ submissionDeleteRoute drRecords = DeleteRoute subUsers <- selectList [SubmissionUserSubmission ==. subId] [] if | length subUsers >= 1 - , maybe False (flip any subUsers . (. submissionUserUser . entityVal) . (==)) uid + , maybe True (flip any subUsers . (. submissionUserUser . entityVal) . (/=)) uid -> Just <$> messageI Warning (MsgSubmissionDeleteCosubmittorsWarning $ length infos) | otherwise -> return Nothing diff --git a/src/Import/NoModel.hs b/src/Import/NoModel.hs index bbb67eb6b..25c1330b5 100644 --- a/src/Import/NoModel.hs +++ b/src/Import/NoModel.hs @@ -38,6 +38,7 @@ import Yesod.Core.Types.Instances as Import import Utils as Import import Utils.Frontend.I18n as Import import Utils.DB as Import +import Utils.Sql as Import import Data.Fixed as Import diff --git a/src/Jobs.hs b/src/Jobs.hs index 5faf681e0..b917354f0 100644 --- a/src/Jobs.hs +++ b/src/Jobs.hs @@ -18,8 +18,6 @@ import Data.Aeson (fromJSON) import qualified Data.Aeson as Aeson import qualified Data.Aeson.Types as Aeson -import Utils.Sql - import Control.Monad.Random (evalRand, mkStdGen, uniformMay) import Cron diff --git a/src/Jobs/Handler/Files.hs b/src/Jobs/Handler/Files.hs index b9817f649..7b144ae05 100644 --- a/src/Jobs/Handler/Files.hs +++ b/src/Jobs/Handler/Files.hs @@ -37,9 +37,10 @@ fileReferences (E.just -> fHash) , E.from $ \newsFile -> E.where_ $ newsFile E.^. CourseNewsFileContent E.==. fHash , E.from $ \sheetFile -> E.where_ $ sheetFile E.^. SheetFileContent E.==. fHash , E.from $ \appInstr -> E.where_ $ appInstr E.^. CourseAppInstructionFileContent E.==. fHash - , E.from $ \matching -> E.where_ $ E.just (matching E.^. AllocationMatchingLog) E.==. fHash + , E.from $ \matching -> E.where_ $ E.just (matching E.^. AllocationMatchingLog) E.==. fHash , E.from $ \subFile -> E.where_ $ subFile E.^. SubmissionFileContent E.==. fHash , E.from $ \sessFile -> E.where_ $ sessFile E.^. SessionFileContent E.==. fHash + , E.from $ \lock -> E.where_ $ E.just (lock E.^. FileLockContent) E.==. fHash ] @@ -75,33 +76,28 @@ dispatchJobInjectFiles = JobHandlerException . maybeT (return ()) $ do extractReference _ = Nothing injectOrDelete :: (Minio.Object, FileContentReference) - -> Handler (Sum Int64, Sum Int64, Sum Int64) -- ^ Deleted, Injected, Existed + -> Handler (Sum Int64, Sum Int64) -- ^ Injected, Already existed injectOrDelete (obj, fRef) = maybeT (return mempty) $ do - res <- hoist runDB $ do - isReferenced <- lift . E.selectExists . E.where_ . E.any E.exists . fileReferences $ E.val fRef - if | isReferenced -> do - alreadyInjected <- lift $ exists [ FileContentHash ==. fRef ] - if | alreadyInjected -> return (mempty, mempty, Sum 1) - | otherwise -> do - content <- (hoistMaybe =<<) . runAppMinio . runMaybeT $ do - objRes <- catchIfMaybeT minioIsDoesNotExist $ Minio.getObject uploadBucket obj Minio.defaultGetObjectOptions - lift . runConduit $ Minio.gorObjectStream objRes .| C.fold - lift $ (mempty, Sum 1, mempty) <$ insert (FileContent fRef content) - | otherwise -> return (Sum 1, mempty, mempty) + res <- hoist (runDB . setSerializable) $ do + alreadyInjected <- lift $ exists [ FileContentHash ==. fRef ] + if | alreadyInjected -> return (mempty, Sum 1) + | otherwise -> do + content <- (hoistMaybe =<<) . runAppMinio . runMaybeT $ do + objRes <- catchIfMaybeT minioIsDoesNotExist $ Minio.getObject uploadBucket obj Minio.defaultGetObjectOptions + lift . runConduit $ Minio.gorObjectStream objRes .| C.fold + lift $ (Sum 1, mempty) <$ insertUnique (FileContent fRef content) runAppMinio . maybeT (return ()) . catchIfMaybeT minioIsDoesNotExist $ Minio.removeObject uploadBucket obj return res - (Sum del, Sum inj, Sum exc) <- + (Sum inj, Sum exc) <- runConduit $ transPipe runAppMinio (Minio.listObjects uploadBucket Nothing True) .| C.mapMaybe extractReference .| maybe (C.map id) (takeWhileTime . (/ 2)) interval - .| transPipe (lift . runDB) (persistentTokenBucketTakeC' TokenBucketInjectFiles $ views _1 Minio.oiSize) + .| transPipe (lift . runDB . setSerializable) (persistentTokenBucketTakeC' TokenBucketInjectFiles $ views _1 Minio.oiSize) .| C.map (over _1 Minio.oiObject) .| transPipe lift (C.mapM injectOrDelete) .| C.fold - when (del > 0) $ - $logInfoS "InjectFiles" [st|Deleted #{del} unreferenced files from upload cache|] when (exc > 0) $ $logInfoS "InjectFiles" [st|Deleted #{exc} files from upload cache because they were already referenced|] when (inj > 0) $ diff --git a/src/Jobs/Queue.hs b/src/Jobs/Queue.hs index 5275304d7..045649ed1 100644 --- a/src/Jobs/Queue.hs +++ b/src/Jobs/Queue.hs @@ -11,7 +11,6 @@ module Jobs.Queue import Import hiding ((<>)) -import Utils.Sql import Jobs.Types import Control.Monad.Writer.Class (MonadWriter(..)) diff --git a/src/Utils/Files.hs b/src/Utils/Files.hs index b9121904e..517c36034 100644 --- a/src/Utils/Files.hs +++ b/src/Utils/Files.hs @@ -24,17 +24,27 @@ import Control.Monad.State.Class (modify) import Database.Persist.Sql (deleteWhereCount) +import Control.Monad.Trans.Resource (allocate) -sinkFiles :: (MonadThrow m, MonadHandler m, HandlerSite m ~ UniWorX) => ConduitT File FileReference (SqlPersistT m) () + +sinkFiles :: (MonadThrow m, MonadHandler m, HandlerSite m ~ UniWorX, MonadUnliftIO m, BackendCompatible SqlBackend (YesodPersistBackend UniWorX), YesodPersist UniWorX) => ConduitT File FileReference (SqlPersistT m) () sinkFiles = C.mapM sinkFile -sinkFile :: (MonadThrow m, MonadHandler m, HandlerSite m ~ UniWorX) => File -> SqlPersistT m FileReference +sinkFile :: (MonadThrow m, MonadHandler m, HandlerSite m ~ UniWorX, MonadUnliftIO m, BackendCompatible SqlBackend (YesodPersistBackend UniWorX), YesodPersist UniWorX) => File -> SqlPersistT m FileReference sinkFile File{ fileContent = Nothing, .. } = return FileReference { fileReferenceContent = Nothing , fileReferenceTitle = fileTitle , fileReferenceModified = fileModified } sinkFile File{ fileContent = Just fileContentContent, .. } = do + void . withUnliftIO $ \UnliftIO{..} -> + let takeLock = do + fileLockTime <- liftIO getCurrentTime + fileLockInstance <- getsYesod appInstanceID + insert FileLock{ fileLockContent = fileContentHash, .. } + releaseLock lId = liftHandler . runDB $ (withReaderT projectBackend $ setSerializable (delete lId :: SqlPersistT (HandlerFor UniWorX) ()) :: YesodDB UniWorX ()) + in unliftIO $ allocate (unliftIO takeLock) (unliftIO . releaseLock) + inDB <- exists [ FileContentHash ==. fileContentHash ] let sinkFileDB = unless inDB $ repsert (FileContentKey fileContentHash) FileContent{..} @@ -60,10 +70,10 @@ sinkFile File{ fileContent = Just fileContentContent, .. } = do fileContentHash = Crypto.hash fileContentContent -sinkFiles' :: (MonadThrow m, MonadHandler m, HandlerSite m ~ UniWorX, HasFileReference record) => ConduitT (File, FileReferenceResidual record) record (SqlPersistT m) () +sinkFiles' :: (MonadThrow m, MonadHandler m, HandlerSite m ~ UniWorX, MonadUnliftIO m, BackendCompatible SqlBackend (YesodPersistBackend UniWorX), YesodPersist UniWorX, HasFileReference record) => ConduitT (File, FileReferenceResidual record) record (SqlPersistT m) () sinkFiles' = C.mapM $ uncurry sinkFile' -sinkFile' :: (MonadThrow m, MonadHandler m, HandlerSite m ~ UniWorX, HasFileReference record) => File -> FileReferenceResidual record -> SqlPersistT m record +sinkFile' :: (MonadThrow m, MonadHandler m, HandlerSite m ~ UniWorX, MonadUnliftIO m, BackendCompatible SqlBackend (YesodPersistBackend UniWorX), YesodPersist UniWorX, HasFileReference record) => File -> FileReferenceResidual record -> SqlPersistT m record sinkFile' file residual = do reference <- sinkFile file return $ _FileReference # (reference, residual) From 88a92390d580d618b15081c87dabe51c7c5e0eca Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Sat, 1 Aug 2020 14:43:33 +0200 Subject: [PATCH 08/17] feat(files): safer file deletion --- config/settings.yml | 2 + models/files.model | 9 ++-- .../Monad/Trans/Memo/StateCache/Instances.hs | 50 ++++++++++++++++++ src/Database/Esqueleto/Utils.hs | 24 ++++++++- src/Foundation/Type.hs | 6 +++ src/Import/NoModel.hs | 1 + src/Jobs/Handler/Files.hs | 47 +++++++++++------ src/Settings.hs | 39 ++------------ src/Settings/Log.hs | 52 +++++++++++++++++++ src/Utils.hs | 3 ++ src/Utils/Files.hs | 2 +- src/Utils/Sql.hs | 18 ++++--- 12 files changed, 190 insertions(+), 63 deletions(-) create mode 100644 src/Control/Monad/Trans/Memo/StateCache/Instances.hs create mode 100644 src/Settings/Log.hs diff --git a/config/settings.yml b/config/settings.yml index 8f21d7277..aea998f4b 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -36,6 +36,7 @@ bearer-encoding: HS256 maximum-content-length: "_env:MAX_UPLOAD_SIZE:134217728" session-files-expire: 3600 prune-unreferenced-files: 600 +keep-unreferenced-files: 86400 health-check-interval: matching-cluster-config: "_env:HEALTHCHECK_INTERVAL_MATCHING_CLUSTER_CONFIG:600" http-reachable: "_env:HEALTHCHECK_INTERVAL_HTTP_REACHABLE:600" @@ -61,6 +62,7 @@ log-settings: all: "_env:LOG_ALL:false" minimum-level: "_env:LOGLEVEL:warn" destination: "_env:LOGDEST:stderr" + serializable-transaction-retry-limit: 2 ip-retention-time: 1209600 diff --git a/models/files.model b/models/files.model index fcf0b3809..428331b36 100644 --- a/models/files.model +++ b/models/files.model @@ -1,7 +1,8 @@ FileContent - hash FileContentReference - content ByteString - Primary hash + hash FileContentReference + content ByteString + unreferencedSince UTCTime Maybe + Primary hash SessionFile content FileContentReference Maybe @@ -10,4 +11,4 @@ SessionFile FileLock content FileContentReference instance InstanceId - time UTCTime \ No newline at end of file + time UTCTime diff --git a/src/Control/Monad/Trans/Memo/StateCache/Instances.hs b/src/Control/Monad/Trans/Memo/StateCache/Instances.hs new file mode 100644 index 000000000..e885eb655 --- /dev/null +++ b/src/Control/Monad/Trans/Memo/StateCache/Instances.hs @@ -0,0 +1,50 @@ +{-# OPTIONS_GHC -fno-warn-orphans #-} + +module Control.Monad.Trans.Memo.StateCache.Instances + ( hoistStateCache + ) where + +import ClassyPrelude hiding (handle) +import Yesod.Core +import Control.Monad.Logger (MonadLoggerIO) +import Control.Monad.Trans.Memo.StateCache +import Control.Monad.Catch + + +instance MonadResource m => MonadResource (StateCache c m) where + liftResourceT = lift . liftResourceT + +instance MonadLogger m => MonadLogger (StateCache c m) +instance MonadLoggerIO m => MonadLoggerIO (StateCache c m) + +instance MonadHandler m => MonadHandler (StateCache c m) where + type HandlerSite (StateCache c m) = HandlerSite m + type SubHandlerSite (StateCache c m) = SubHandlerSite m + + liftHandler = lift . liftHandler + liftSubHandler = lift . liftSubHandler + +instance MonadWidget m => MonadWidget (StateCache c m) where + liftWidget = lift . liftWidget + +instance MonadThrow m => MonadThrow (StateCache c m) where + throwM = lift . throwM + +-- | Rolls back modifications to state in failing section +instance MonadCatch m => MonadCatch (StateCache c m) where + catch m h = do + s <- container + (x, s') <- lift . handle (flip runStateCache s . h) $ runStateCache m s + x <$ setContainer s' + +hoistStateCache :: forall m n c b. + Monad n + => (forall a. m a -> n a) + -> (StateCache c m b -> StateCache c n b) +-- ^ Morally identical to `Control.Monad.Morph.hoist` +-- +-- Due to limited exports from `Control.Monad.Trans.Memo.StateCache` we incur a @Monad n@ constraint which `Control.Monad.Morph.hoist` does not account for +hoistStateCache nat m = do + s <- container + (x, s') <- lift . nat $ runStateCache m s + x <$ setContainer s' diff --git a/src/Database/Esqueleto/Utils.hs b/src/Database/Esqueleto/Utils.hs index 026f3e79e..8828d9d4f 100644 --- a/src/Database/Esqueleto/Utils.hs +++ b/src/Database/Esqueleto/Utils.hs @@ -18,6 +18,8 @@ module Database.Esqueleto.Utils , SqlHashable , sha256 , maybe, unsafeCoalesce + , bool + , max, min , SqlProject(..) , (->.) , fromSqlKey @@ -27,7 +29,7 @@ module Database.Esqueleto.Utils ) where -import ClassyPrelude.Yesod hiding (isInfixOf, any, all, or, and, isJust, maybe) +import ClassyPrelude.Yesod hiding (isInfixOf, any, all, or, and, isJust, maybe, bool, max, min) import Data.Universe import qualified Data.Set as Set import qualified Data.List as List @@ -241,6 +243,26 @@ maybe onNothing onJust val = E.case_ ] (E.else_ onNothing) +bool :: PersistField a + => E.SqlExpr (E.Value a) + -> E.SqlExpr (E.Value a) + -> E.SqlExpr (E.Value Bool) + -> E.SqlExpr (E.Value a) +bool onFalse onTrue val = E.case_ + [ E.when_ + val + E.then_ + onTrue + ] + (E.else_ onFalse) + +max, min :: PersistField a + => E.SqlExpr (E.Value a) + -> E.SqlExpr (E.Value a) + -> E.SqlExpr (E.Value a) +max a b = bool a b $ b E.>. a +min a b = bool a b $ b E.<. a + unsafeCoalesce :: E.PersistField a => [E.SqlExpr (E.Value (Maybe a))] -> E.SqlExpr (E.Value a) unsafeCoalesce = E.veryUnsafeCoerceSqlExprValue . E.coalesce diff --git a/src/Foundation/Type.hs b/src/Foundation/Type.hs index 44e3cdd77..ee96ec211 100644 --- a/src/Foundation/Type.hs +++ b/src/Foundation/Type.hs @@ -1,3 +1,6 @@ +{-# OPTIONS_GHC -fno-warn-orphans #-} +{-# LANGUAGE UndecidableInstances #-} + module Foundation.Type ( UniWorX(..) , SomeSessionStorage(..) @@ -68,3 +71,6 @@ instance HasAppSettings UniWorX where appSettings = _appSettings' instance HasCookieSettings RegisteredCookie UniWorX where getCookieSettings = appCookieSettings . appSettings' + +instance (MonadHandler m, HandlerSite m ~ UniWorX) => ReadLogSettings m where + readLogSettings = liftIO . readTVarIO =<< getsYesod (view _appLogSettings) diff --git a/src/Import/NoModel.hs b/src/Import/NoModel.hs index 25c1330b5..d7a71dce2 100644 --- a/src/Import/NoModel.hs +++ b/src/Import/NoModel.hs @@ -165,6 +165,7 @@ import Network.HTTP.Types.Method.Instances as Import () import Crypto.Random.Instances as Import () import Network.Minio.Instances as Import () import System.Clock.Instances as Import () +import Control.Monad.Trans.Memo.StateCache.Instances as Import (hoistStateCache) import Crypto.Hash as Import (Digest, SHA3_256, SHA3_512) import Crypto.Random as Import (ChaChaDRG, Seed) diff --git a/src/Jobs/Handler/Files.hs b/src/Jobs/Handler/Files.hs index 7b144ae05..0d4774ea9 100644 --- a/src/Jobs/Handler/Files.hs +++ b/src/Jobs/Handler/Files.hs @@ -20,6 +20,8 @@ import qualified Network.Minio as Minio import qualified Crypto.Hash as Crypto import qualified Data.ByteString.Base64.URL as Base64 +import Control.Monad.Memo (startEvalMemoT, memo) + dispatchJobPruneSessionFiles :: JobHandler UniWorX dispatchJobPruneSessionFiles = JobHandlerAtomic . hoist lift $ do @@ -47,26 +49,36 @@ fileReferences (E.just -> fHash) dispatchJobPruneUnreferencedFiles :: JobHandler UniWorX dispatchJobPruneUnreferencedFiles = JobHandlerAtomic . hoist lift $ do - interval <- getsYesod $ view _appPruneUnreferencedFiles - Sum n <- runConduit $ getCandidates - .| maybe (C.map id) (takeWhileTime . (/ 2)) interval - .| persistentTokenBucketTakeC' TokenBucketPruneFiles (view $ _2 . _Value :: _ -> Word64) - .| C.map (view $ _1 . _Value) - .| C.mapM (\fRef -> Sum <$> deleteWhereCount [FileContentHash ==. fRef]) - .| C.fold - $logInfoS "PruneUnreferencedFiles" [st|Deleted #{n} unreferenced files|] - where + now <- liftIO getCurrentTime + interval <- fmap (fmap $ max 0) . getsYesod $ view _appPruneUnreferencedFiles + keep <- fmap (max 0) . getsYesod $ view _appKeepUnreferencedFiles + + E.update $ \fileContent -> do + let isReferenced = E.any E.exists . fileReferences $ fileContent E.^. FileContentHash + E.set fileContent [ FileContentUnreferencedSince E.=. E.bool (E.just . E.maybe (E.val now) (E.min $ E.val now) $ fileContent E.^. FileContentUnreferencedSince) E.nothing isReferenced ] + + let getCandidates = E.selectSource . E.from $ \fileContent -> do - E.where_ . E.not_ . E.any E.exists $ fileReferences (fileContent E.^. FileContentHash) + E.where_ . E.maybe E.false (E.<. E.val (addUTCTime (-keep) now)) $ fileContent E.^. FileContentUnreferencedSince return ( fileContent E.^. FileContentHash , E.length_ $ fileContent E.^. FileContentContent ) + + Sum deleted <- runConduit $ + getCandidates + .| maybe (C.map id) (takeWhileTime . (/ 2)) interval + .| persistentTokenBucketTakeC' TokenBucketPruneFiles (view $ _2 . _Value :: _ -> Word64) + .| C.map (view $ _1 . _Value) + .| C.mapM (\fRef -> Sum <$> deleteWhereCount [FileContentHash ==. fRef]) + .| C.fold + $logInfoS "PruneUnreferencedFiles" [st|Deleted #{deleted} long-unreferenced files|] dispatchJobInjectFiles :: JobHandler UniWorX dispatchJobInjectFiles = JobHandlerException . maybeT (return ()) $ do uploadBucket <- getsYesod $ view _appUploadCacheBucket interval <- getsYesod $ view _appInjectFiles + now <- liftIO getCurrentTime let extractReference (Minio.ListItemObject oi) @@ -78,14 +90,17 @@ dispatchJobInjectFiles = JobHandlerException . maybeT (return ()) $ do injectOrDelete :: (Minio.Object, FileContentReference) -> Handler (Sum Int64, Sum Int64) -- ^ Injected, Already existed injectOrDelete (obj, fRef) = maybeT (return mempty) $ do - res <- hoist (runDB . setSerializable) $ do - alreadyInjected <- lift $ exists [ FileContentHash ==. fRef ] + res <- hoist (startEvalMemoT . hoistStateCache (runDB . setSerializable)) $ do + alreadyInjected <- lift . lift $ exists [ FileContentHash ==. fRef ] if | alreadyInjected -> return (mempty, Sum 1) | otherwise -> do - content <- (hoistMaybe =<<) . runAppMinio . runMaybeT $ do - objRes <- catchIfMaybeT minioIsDoesNotExist $ Minio.getObject uploadBucket obj Minio.defaultGetObjectOptions + content <- flip memo obj $ \obj' -> hoistMaybeM . runAppMinio . runMaybeT $ do + objRes <- catchIfMaybeT minioIsDoesNotExist $ Minio.getObject uploadBucket obj' Minio.defaultGetObjectOptions lift . runConduit $ Minio.gorObjectStream objRes .| C.fold - lift $ (Sum 1, mempty) <$ insertUnique (FileContent fRef content) + + fmap ((, mempty) . Sum) . lift. lift . E.insertSelectCount $ + let isReferenced = E.any E.exists $ fileReferences (E.val fRef) + in return $ FileContent E.<# E.val fRef E.<&> E.val content E.<&> E.bool (E.just $ E.val now) E.nothing isReferenced runAppMinio . maybeT (return ()) . catchIfMaybeT minioIsDoesNotExist $ Minio.removeObject uploadBucket obj return res @@ -99,6 +114,6 @@ dispatchJobInjectFiles = JobHandlerException . maybeT (return ()) $ do .| C.fold when (exc > 0) $ - $logInfoS "InjectFiles" [st|Deleted #{exc} files from upload cache because they were already referenced|] + $logInfoS "InjectFiles" [st|Deleted #{exc} files from upload cache because they were already injected|] when (inj > 0) $ $logInfoS "InjectFiles" [st|Injected #{inj} files from upload cache into database|] diff --git a/src/Settings.hs b/src/Settings.hs index 8a3995342..490d8076c 100644 --- a/src/Settings.hs +++ b/src/Settings.hs @@ -11,6 +11,7 @@ module Settings , module Settings.Cluster , module Settings.Mime , module Settings.Cookies + , module Settings.Log ) where import Import.NoModel @@ -53,6 +54,7 @@ import Model import Settings.Cluster import Settings.Mime import Settings.Cookies +import Settings.Log import qualified System.FilePath as FilePath @@ -139,6 +141,7 @@ data AppSettings = AppSettings , appSessionFilesExpire :: NominalDiffTime , appPruneUnreferencedFiles :: Maybe NominalDiffTime + , appKeepUnreferencedFiles :: NominalDiffTime , appInitialLogSettings :: LogSettings @@ -188,23 +191,6 @@ newtype ServerSessionSettings instance Show ServerSessionSettings where showsPrec d _ = showParen (d > 10) $ showString "ServerSessionSettings _" -data LogSettings = LogSettings - { logAll, logDetailed :: Bool - , logMinimumLevel :: LogLevel - , logDestination :: LogDestination - } deriving (Show, Read, Generic, Eq, Ord) - -data LogDestination = LogDestStderr | LogDestStdout | LogDestFile { logDestFile :: !FilePath } - deriving (Show, Read, Generic, Eq, Ord) - -deriving instance Generic LogLevel -instance Hashable LogLevel -instance NFData LogLevel -instance Hashable LogSettings -instance NFData LogSettings -instance Hashable LogDestination -instance NFData LogDestination - data UserDefaultConf = UserDefaultConf { userDefaultTheme :: Theme , userDefaultMaxFavourites, userDefaultMaxFavouriteTerms :: Int @@ -306,17 +292,6 @@ deriveJSON defaultOptions { fieldLabelModifier = camelToPathPiece' 2 } ''TokenBucketConf -deriveJSON defaultOptions - { constructorTagModifier = camelToPathPiece' 2 - , fieldLabelModifier = camelToPathPiece' 2 - , sumEncoding = UntaggedValue - , unwrapUnaryRecords = True - } ''LogDestination - -deriveJSON defaultOptions - { fieldLabelModifier = camelToPathPiece' 1 - } ''LogSettings - deriveFromJSON defaultOptions ''Ldap.Scope deriveFromJSON defaultOptions { fieldLabelModifier = camelToPathPiece' 2 @@ -349,13 +324,6 @@ deriveFromJSON } ''ResourcePoolConf -deriveJSON - defaultOptions - { constructorTagModifier = camelToPathPiece' 1 - , sumEncoding = UntaggedValue - } - ''LogLevel - instance FromJSON HaskellNet.PortNumber where parseJSON = withScientific "PortNumber" $ \sciNum -> case toBoundedInteger sciNum of Just int -> return $ fromIntegral (int :: Word16) @@ -502,6 +470,7 @@ instance FromJSON AppSettings where appSessionFilesExpire <- o .: "session-files-expire" appPruneUnreferencedFiles <- o .:? "prune-unreferenced-files" + appKeepUnreferencedFiles <- o .:? "keep-unreferenced-files" .!= 0 appInjectFiles <- o .:? "inject-files" appMaximumContentLength <- o .: "maximum-content-length" diff --git a/src/Settings/Log.hs b/src/Settings/Log.hs new file mode 100644 index 000000000..112519e41 --- /dev/null +++ b/src/Settings/Log.hs @@ -0,0 +1,52 @@ +{-# OPTIONS_GHC -fno-warn-orphans #-} + +module Settings.Log + ( LogSettings(..) + , LogDestination(..) + , LogLevel(..) + , ReadLogSettings(..) + ) where + +import ClassyPrelude.Yesod +import Numeric.Natural + +import Data.Aeson.TH +import Utils.PathPiece + + +data LogSettings = LogSettings + { logAll, logDetailed :: Bool + , logMinimumLevel :: LogLevel + , logDestination :: LogDestination + , logSerializableTransactionRetryLimit :: Maybe Natural + } deriving (Show, Read, Generic, Eq, Ord) + +data LogDestination = LogDestStderr | LogDestStdout | LogDestFile { logDestFile :: !FilePath } + deriving (Show, Read, Generic, Eq, Ord) + +deriving instance Generic LogLevel +instance Hashable LogLevel +instance NFData LogLevel +instance Hashable LogSettings +instance NFData LogSettings +instance Hashable LogDestination +instance NFData LogDestination + +deriveJSON defaultOptions + { constructorTagModifier = camelToPathPiece' 1 + , sumEncoding = UntaggedValue + } ''LogLevel + +deriveJSON defaultOptions + { constructorTagModifier = camelToPathPiece' 2 + , fieldLabelModifier = camelToPathPiece' 2 + , sumEncoding = UntaggedValue + , unwrapUnaryRecords = True + } ''LogDestination + +deriveJSON defaultOptions + { fieldLabelModifier = camelToPathPiece' 1 + } ''LogSettings + +class ReadLogSettings m where + readLogSettings :: m LogSettings diff --git a/src/Utils.hs b/src/Utils.hs index 3181e52d5..0baeee670 100644 --- a/src/Utils.hs +++ b/src/Utils.hs @@ -569,6 +569,9 @@ hoistMaybe :: MonadPlus m => Maybe a -> m a -- ^ `hoist` regarding `Maybe` as if identical to @MaybeT Identity@ hoistMaybe = maybe mzero return +hoistMaybeM :: MonadPlus m => m (Maybe a) -> m a +hoistMaybeM = (=<<) hoistMaybe + catchIfMaybeT :: (MonadCatch m, Exception e) => (e -> Bool) -> m a -> MaybeT m a catchIfMaybeT p act = catchIf p (lift act) (const mzero) diff --git a/src/Utils/Files.hs b/src/Utils/Files.hs index 517c36034..8ccf64b13 100644 --- a/src/Utils/Files.hs +++ b/src/Utils/Files.hs @@ -47,7 +47,7 @@ sinkFile File{ fileContent = Just fileContentContent, .. } = do inDB <- exists [ FileContentHash ==. fileContentHash ] - let sinkFileDB = unless inDB $ repsert (FileContentKey fileContentHash) FileContent{..} + let sinkFileDB = unless inDB $ repsert (FileContentKey fileContentHash) FileContent{ fileContentUnreferencedSince = Nothing, .. } maybeT sinkFileDB $ do let uploadName = decodeUtf8 . Base64.encodeUnpadded $ ByteArray.convert fileContentHash uploadBucket <- getsYesod $ views appSettings appUploadCacheBucket diff --git a/src/Utils/Sql.hs b/src/Utils/Sql.hs index 47f90a449..eeb11c537 100644 --- a/src/Utils/Sql.hs +++ b/src/Utils/Sql.hs @@ -3,6 +3,8 @@ module Utils.Sql ) where import ClassyPrelude.Yesod +import Numeric.Natural +import Settings.Log import Database.PostgreSQL.Simple (SqlError) import Database.PostgreSQL.Simple.Errors (isSerializationError) @@ -16,23 +18,27 @@ import Control.Retry import Control.Lens ((&)) -setSerializable :: forall m a. (MonadLogger m, MonadMask m, MonadIO m) => ReaderT SqlBackend m a -> ReaderT SqlBackend m a +setSerializable :: forall m a. (MonadLogger m, MonadMask m, MonadIO m, ReadLogSettings (SqlPersistT m)) => SqlPersistT m a -> SqlPersistT m a setSerializable = setSerializable' $ fullJitterBackoff 1e3 & limitRetriesByCumulativeDelay 10e6 -setSerializable' :: forall m a. (MonadLogger m, MonadMask m, MonadIO m) => RetryPolicyM (ReaderT SqlBackend m) -> ReaderT SqlBackend m a -> ReaderT SqlBackend m a +setSerializable' :: forall m a. (MonadLogger m, MonadMask m, MonadIO m, ReadLogSettings (SqlPersistT m)) => RetryPolicyM (SqlPersistT m) -> SqlPersistT m a -> ReaderT SqlBackend m a setSerializable' policy act = do + LogSettings{logSerializableTransactionRetryLimit} <- readLogSettings didCommit <- newTVarIO False - recovering policy (skipAsyncExceptions `snoc` logRetries suggestRetry logRetry) $ act' didCommit + recovering policy (skipAsyncExceptions `snoc` logRetries suggestRetry (logRetry logSerializableTransactionRetryLimit)) $ act' didCommit where suggestRetry :: SqlError -> ReaderT SqlBackend m Bool suggestRetry = return . isSerializationError - logRetry :: Bool -- ^ Will retry + logRetry :: Maybe Natural + -> Bool -- ^ Will retry -> SqlError -> RetryStatus -> ReaderT SqlBackend m () - logRetry shouldRetry@False err status = $logErrorS "SQL.setSerializable" . pack $ defaultLogMsg shouldRetry err status - logRetry shouldRetry@True err status = $logDebugS "SQL.setSerializable" . pack $ defaultLogMsg shouldRetry err status + logRetry _ shouldRetry@False err status = $logErrorS "SQL.setSerializable" . pack $ defaultLogMsg shouldRetry err status + logRetry (Just limit) shouldRetry err status + | fromIntegral limit >= rsIterNumber status = $logInfoS "SQL.setSerializable" . pack $ defaultLogMsg shouldRetry err status + logRetry _ shouldRetry err status = $logDebugS "SQL.setSerializable" . pack $ defaultLogMsg shouldRetry err status act' :: TVar Bool -> RetryStatus -> ReaderT SqlBackend m a act' didCommit RetryStatus{..} = do From dfe68d5924d37ea4d3fd0df0a8e68871bcd187d5 Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Sun, 2 Aug 2020 10:27:50 +0200 Subject: [PATCH 09/17] feat: migrate indexes --- src/Model/Migration.hs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/Model/Migration.hs b/src/Model/Migration.hs index 6888a16a7..df6289f1f 100644 --- a/src/Model/Migration.hs +++ b/src/Model/Migration.hs @@ -71,6 +71,7 @@ migrateAll' :: Migration migrateAll' = sequence_ [ migrateUniWorX , migrateMemcachedSqlStorage + , migrateManual ] migrateAll :: ( MonadLogger m @@ -137,6 +138,27 @@ getMissingMigrations = do appliedMigrations <- selectKeysList [] [] return $ customMigrations `Map.withoutKeys` Set.fromList appliedMigrations + +migrateManual :: Migration +migrateManual = + addMigrations + [ (False, "CREATE INDEX IF NOT EXISTS course_application_file_content ON course_application_file (content)") + , (False, "CREATE INDEX IF NOT EXISTS material_file_content ON material_file (content)") + , (False, "CREATE INDEX IF NOT EXISTS course_news_file_content ON course_news_file (content)") + , (False, "CREATE INDEX IF NOT EXISTS sheet_file_content ON sheet_file (content)") + , (False, "CREATE INDEX IF NOT EXISTS course_app_instruction_file_content ON course_app_instruction_file (content)") + , (False, "CREATE INDEX IF NOT EXISTS allocation_matching_log ON allocation_matching (log)") + , (False, "CREATE INDEX IF NOT EXISTS submission_file_content ON submission_file (content)") + , (False, "CREATE INDEX IF NOT EXISTS session_file_content ON session_file (content)") + , (False, "CREATE INDEX IF NOT EXISTS file_lock_content ON file_lock (content)") + , (False, "CREATE INDEX IF NOT EXISTS user_lower_display_email ON \"user\" (lower(display_email))") + , (False, "CREATE INDEX IF NOT EXISTS user_lower_email ON \"user\" (lower(email))") + , (False, "CREATE INDEX IF NOT EXISTS user_lower_ident ON \"user\" (lower(ident))") + , (False, "CREATE INDEX IF NOT EXISTS submission_sheet ON submission (sheet)") + , (False, "CREATE INDEX IF NOT EXISTS submission_edit_submission ON submission_edit (submission)") + ] + + {- Confusion about quotes, from the PostgreSQL Manual: Single quotes for string constants, double quotes for table/column names. @@ -145,7 +167,6 @@ getMissingMigrations = do #{anything} (escaped as value); -} - customMigrations :: forall m. MonadResource m => Map (Key AppliedMigration) (ReaderT SqlBackend m ()) From 60be62b63bf328407e4ba0f01221d87020e89f24 Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Sun, 2 Aug 2020 10:40:35 +0200 Subject: [PATCH 10/17] fix(set-serializable): logging limit --- src/Utils/Sql.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utils/Sql.hs b/src/Utils/Sql.hs index eeb11c537..b3ad49706 100644 --- a/src/Utils/Sql.hs +++ b/src/Utils/Sql.hs @@ -37,7 +37,7 @@ setSerializable' policy act = do -> ReaderT SqlBackend m () logRetry _ shouldRetry@False err status = $logErrorS "SQL.setSerializable" . pack $ defaultLogMsg shouldRetry err status logRetry (Just limit) shouldRetry err status - | fromIntegral limit >= rsIterNumber status = $logInfoS "SQL.setSerializable" . pack $ defaultLogMsg shouldRetry err status + | fromIntegral limit <= rsIterNumber status = $logInfoS "SQL.setSerializable" . pack $ defaultLogMsg shouldRetry err status logRetry _ shouldRetry err status = $logDebugS "SQL.setSerializable" . pack $ defaultLogMsg shouldRetry err status act' :: TVar Bool -> RetryStatus -> ReaderT SqlBackend m a From eb9c6760b9a62d263f0c30531a643d43c7318b3f Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Sun, 2 Aug 2020 13:42:02 +0200 Subject: [PATCH 11/17] fix: weird sql casting --- src/Database/Esqueleto/Utils.hs | 21 ++++++++++++++++++++- src/Jobs/Handler/Files.hs | 11 ++++++++--- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/Database/Esqueleto/Utils.hs b/src/Database/Esqueleto/Utils.hs index 8828d9d4f..36c33d573 100644 --- a/src/Database/Esqueleto/Utils.hs +++ b/src/Database/Esqueleto/Utils.hs @@ -17,7 +17,7 @@ module Database.Esqueleto.Utils , selectExists, selectNotExists , SqlHashable , sha256 - , maybe, unsafeCoalesce + , maybe, maybeEq, unsafeCoalesce , bool , max, min , SqlProject(..) @@ -242,6 +242,25 @@ maybe onNothing onJust val = E.case_ (onJust $ E.veryUnsafeCoerceSqlExprValue val) ] (E.else_ onNothing) + +infix 4 `maybeEq` + +maybeEq :: PersistField a + => E.SqlExpr (E.Value (Maybe a)) + -> E.SqlExpr (E.Value (Maybe a)) + -> E.SqlExpr (E.Value Bool) +-- ^ `E.==.` but treat `E.nothing` as identical +maybeEq a b = E.case_ + [ E.when_ + (E.isNothing a) + E.then_ + (E.isNothing b) + , E.when_ + (E.isNothing b) + E.then_ + false -- (E.isNothing a) + ] + (E.else_ $ a E.==. b) bool :: PersistField a => E.SqlExpr (E.Value a) diff --git a/src/Jobs/Handler/Files.hs b/src/Jobs/Handler/Files.hs index 0d4774ea9..de85244c0 100644 --- a/src/Jobs/Handler/Files.hs +++ b/src/Jobs/Handler/Files.hs @@ -10,6 +10,7 @@ import Database.Persist.Sql (deleteWhereCount) import qualified Database.Esqueleto as E import qualified Database.Esqueleto.Utils as E +import qualified Database.Esqueleto.Internal.Sql as E (unsafeSqlCastAs) import qualified Data.Conduit.Combinators as C import qualified Data.Conduit.List as C (mapMaybe) @@ -55,7 +56,9 @@ dispatchJobPruneUnreferencedFiles = JobHandlerAtomic . hoist lift $ do E.update $ \fileContent -> do let isReferenced = E.any E.exists . fileReferences $ fileContent E.^. FileContentHash - E.set fileContent [ FileContentUnreferencedSince E.=. E.bool (E.just . E.maybe (E.val now) (E.min $ E.val now) $ fileContent E.^. FileContentUnreferencedSince) E.nothing isReferenced ] + now' = E.unsafeSqlCastAs "TIMESTAMP WITH TIME ZONE" $ E.val now + shouldBe = E.bool (E.just . E.maybe now' (E.min now') $ fileContent E.^. FileContentUnreferencedSince) E.nothing isReferenced + E.set fileContent [ FileContentUnreferencedSince E.=. shouldBe ] let getCandidates = E.selectSource . E.from $ \fileContent -> do @@ -71,7 +74,8 @@ dispatchJobPruneUnreferencedFiles = JobHandlerAtomic . hoist lift $ do .| C.map (view $ _1 . _Value) .| C.mapM (\fRef -> Sum <$> deleteWhereCount [FileContentHash ==. fRef]) .| C.fold - $logInfoS "PruneUnreferencedFiles" [st|Deleted #{deleted} long-unreferenced files|] + when (deleted > 0) $ + $logInfoS "PruneUnreferencedFiles" [st|Deleted #{deleted} long-unreferenced files|] dispatchJobInjectFiles :: JobHandler UniWorX @@ -100,7 +104,8 @@ dispatchJobInjectFiles = JobHandlerException . maybeT (return ()) $ do fmap ((, mempty) . Sum) . lift. lift . E.insertSelectCount $ let isReferenced = E.any E.exists $ fileReferences (E.val fRef) - in return $ FileContent E.<# E.val fRef E.<&> E.val content E.<&> E.bool (E.just $ E.val now) E.nothing isReferenced + now' = E.unsafeSqlCastAs "TIMESTAMP WITH TIME ZONE" $ E.val now + in return $ FileContent E.<# E.val fRef E.<&> E.val content E.<&> E.bool (E.just now') E.nothing isReferenced runAppMinio . maybeT (return ()) . catchIfMaybeT minioIsDoesNotExist $ Minio.removeObject uploadBucket obj return res From 7a17535600ef9408af2da5e0b01bea4b6e2fb63b Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Sun, 2 Aug 2020 14:08:01 +0200 Subject: [PATCH 12/17] fix(migration): make index migration truly idempotent --- src/Model/Migration.hs | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/Model/Migration.hs b/src/Model/Migration.hs index df6289f1f..6b4c67ee8 100644 --- a/src/Model/Migration.hs +++ b/src/Model/Migration.hs @@ -140,23 +140,31 @@ getMissingMigrations = do migrateManual :: Migration -migrateManual = - addMigrations - [ (False, "CREATE INDEX IF NOT EXISTS course_application_file_content ON course_application_file (content)") - , (False, "CREATE INDEX IF NOT EXISTS material_file_content ON material_file (content)") - , (False, "CREATE INDEX IF NOT EXISTS course_news_file_content ON course_news_file (content)") - , (False, "CREATE INDEX IF NOT EXISTS sheet_file_content ON sheet_file (content)") - , (False, "CREATE INDEX IF NOT EXISTS course_app_instruction_file_content ON course_app_instruction_file (content)") - , (False, "CREATE INDEX IF NOT EXISTS allocation_matching_log ON allocation_matching (log)") - , (False, "CREATE INDEX IF NOT EXISTS submission_file_content ON submission_file (content)") - , (False, "CREATE INDEX IF NOT EXISTS session_file_content ON session_file (content)") - , (False, "CREATE INDEX IF NOT EXISTS file_lock_content ON file_lock (content)") - , (False, "CREATE INDEX IF NOT EXISTS user_lower_display_email ON \"user\" (lower(display_email))") - , (False, "CREATE INDEX IF NOT EXISTS user_lower_email ON \"user\" (lower(email))") - , (False, "CREATE INDEX IF NOT EXISTS user_lower_ident ON \"user\" (lower(ident))") - , (False, "CREATE INDEX IF NOT EXISTS submission_sheet ON submission (sheet)") - , (False, "CREATE INDEX IF NOT EXISTS submission_edit_submission ON submission_edit (submission)") +migrateManual = do + mapM_ (uncurry addIndex) + [ ("course_application_file_content", "CREATE INDEX course_application_file_content ON course_application_file (content)" ) + , ("material_file_content", "CREATE INDEX material_file_content ON material_file (content)" ) + , ("course_news_file_content", "CREATE INDEX course_news_file_content ON course_news_file (content)" ) + , ("sheet_file_content", "CREATE INDEX sheet_file_content ON sheet_file (content)" ) + , ("course_app_instruction_file_content", "CREATE INDEX course_app_instruction_file_content ON course_app_instruction_file (content)") + , ("allocation_matching_log", "CREATE INDEX allocation_matching_log ON allocation_matching (log)" ) + , ("submission_file_content", "CREATE INDEX submission_file_content ON submission_file (content)" ) + , ("session_file_content", "CREATE INDEX session_file_content ON session_file (content)" ) + , ("file_lock_content", "CREATE INDEX file_lock_content ON file_lock (content)" ) + , ("user_lower_display_email", "CREATE INDEX user_lower_display_email ON \"user\" (lower(display_email))" ) + , ("user_lower_email", "CREATE INDEX user_lower_email ON \"user\" (lower(email))" ) + , ("user_lower_ident", "CREATE INDEX user_lower_ident ON \"user\" (lower(ident))" ) + , ("submission_sheet", "CREATE INDEX submission_sheet ON submission (sheet)" ) + , ("submission_edit_submission", "CREATE INDEX submission_edit_submission ON submission_edit (submission)" ) ] + where + addIndex :: Text -> Sql -> Migration + addIndex ixName ixDef = do + res <- lift $ lift [sqlQQ|SELECT EXISTS (SELECT 1 FROM pg_indexes WHERE schemaname = current_schema() AND indexname = #{ixName})|] + alreadyDefined <- case res of + [Single e] -> return e + _other -> return True + unless alreadyDefined $ addMigration False ixDef {- From 19de95f5a4312a1ab3b6e9eed42824ee59240c22 Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Sun, 2 Aug 2020 14:20:43 +0200 Subject: [PATCH 13/17] chore(release): 18.4.0 --- CHANGELOG.md | 19 +++++++++++++++++++ package-lock.json | 2 +- package.json | 2 +- package.yaml | 2 +- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ece61ece0..36b77fd11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,25 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [18.4.0](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/compare/v18.3.0...v18.4.0) (2020-08-02) + + +### Bug Fixes + +* **migration:** make index migration truly idempotent ([7a17535](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/7a17535)) +* weird sql casting ([eb9c676](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/eb9c676)) +* **set-serializable:** logging limit ([60be62b](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/60be62b)) +* better concurrency behaviour ([a0392dd](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/a0392dd)) +* suppress exceptions relating to expired sessions ([d47d6aa](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/d47d6aa)) + + +### Features + +* migrate indexes ([dfe68d5](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/dfe68d5)) +* **files:** safer file deletion ([88a9239](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/88a9239)) + + + ## [18.3.0](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/compare/v18.2.2...v18.3.0) (2020-07-28) diff --git a/package-lock.json b/package-lock.json index c86aab431..6357927a7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "uni2work", - "version": "18.3.0", + "version": "18.4.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index bf15ad329..91e5e1937 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "uni2work", - "version": "18.3.0", + "version": "18.4.0", "description": "", "keywords": [], "author": "", diff --git a/package.yaml b/package.yaml index cfabb80ef..ec34e6c40 100644 --- a/package.yaml +++ b/package.yaml @@ -1,5 +1,5 @@ name: uniworx -version: 18.3.0 +version: 18.4.0 dependencies: - base From 460c133aac316fb9317c5f08823e04c22eb63fe9 Mon Sep 17 00:00:00 2001 From: Gregor Kleen Date: Mon, 3 Aug 2020 10:11:52 +0200 Subject: [PATCH 14/17] feat: admin-crontab-r --- messages/uniworx/de-de-formal.msg | 8 +++++- messages/uniworx/en-eu.msg | 6 +++++ routes | 1 + src/Cron.hs | 4 ++- src/Foundation.hs | 9 +++++++ src/Handler/Admin.hs | 1 + src/Handler/Admin/Crontab.hs | 44 +++++++++++++++++++++++++++++++ src/Jobs.hs | 20 ++++++++------ src/Jobs/Types.hs | 4 +++ 9 files changed, 87 insertions(+), 10 deletions(-) create mode 100644 src/Handler/Admin/Crontab.hs diff --git a/messages/uniworx/de-de-formal.msg b/messages/uniworx/de-de-formal.msg index e54dde4cd..494f7490a 100644 --- a/messages/uniworx/de-de-formal.msg +++ b/messages/uniworx/de-de-formal.msg @@ -1340,6 +1340,7 @@ MenuAllocationPriorities: Zentrale Dringlichkeiten MenuAllocationCompute: Platzvergabe berechnen MenuAllocationAccept: Platzvergabe akzeptieren MenuFaq: FAQ +MenuAdminCrontab: Crontab BreadcrumbSubmissionFile: Datei BreadcrumbSubmissionUserInvite: Einladung zur Abgabe @@ -1411,6 +1412,7 @@ BreadcrumbAllocationCompute: Platzvergabe berechnen BreadcrumbAllocationAccept: Platzvergabe akzeptieren BreadcrumbMessageHide: Verstecken BreadcrumbFaq: FAQ +BreadcrumbAdminCrontab: Crontab ExternalExamEdit coursen@CourseName examn@ExamName: Bearbeiten: #{coursen}, #{examn} ExternalExamGrades coursen@CourseName examn@ExamName: Prüfungsleistungen: #{coursen}, #{examn} @@ -2664,4 +2666,8 @@ SubmissionDoneNever: Nie SubmissionDoneByFile: Je nach Bewertungsdatei SubmissionDoneAlways: Immer CorrUploadSubmissionDoneMode: Bewertung abgeschlossen -CorrUploadSubmissionDoneModeTip: Sollen hochgeladene Korrekturen als abgeschlossen markiert werden? Bewertungen sind erst für Studierende sichtbar und zählen gegen Examboni, wenn sie abgeschlossen sind. \ No newline at end of file +CorrUploadSubmissionDoneModeTip: Sollen hochgeladene Korrekturen als abgeschlossen markiert werden? Bewertungen sind erst für Studierende sichtbar und zählen gegen Examboni, wenn sie abgeschlossen sind. + +AdminCrontabNotGenerated: (Noch) keine Crontab generiert +CronMatchAsap: ASAP +CronMatchNone: Nie \ No newline at end of file diff --git a/messages/uniworx/en-eu.msg b/messages/uniworx/en-eu.msg index 30a50e075..192c8a8d6 100644 --- a/messages/uniworx/en-eu.msg +++ b/messages/uniworx/en-eu.msg @@ -1340,6 +1340,7 @@ MenuAllocationPriorities: Central priorities MenuAllocationCompute: Compute allocation MenuAllocationAccept: Accept allocation MenuFaq: FAQ +MenuAdminCrontab: Crontab BreadcrumbSubmissionFile: File BreadcrumbSubmissionUserInvite: Invitation to participate in a submission @@ -1411,6 +1412,7 @@ BreadcrumbAllocationCompute: Compute allocation BreadcrumbAllocationAccept: Accept allocation BreadcrumbMessageHide: Hide BreadcrumbFaq: FAQ +BreadcrumbAdminCrontab: Crontab ExternalExamEdit coursen examn: Edit: #{coursen}, #{examn} ExternalExamGrades coursen examn: Exam achievements: #{coursen}, #{examn} @@ -2665,3 +2667,7 @@ SubmissionDoneByFile: According to correction file SubmissionDoneAlways: Always CorrUploadSubmissionDoneMode: Rating finished CorrUploadSubmissionDoneModeTip: Should uploaded corrections be marked as finished? The rating is only visible to the submittors and considered for any exam bonuses if it is finished. + +AdminCrontabNotGenerated: Crontab not (yet) generated +CronMatchAsap: ASAP +CronMatchNone: Never diff --git a/routes b/routes index 54e9af960..e28cbc4d0 100644 --- a/routes +++ b/routes @@ -56,6 +56,7 @@ /admin/test AdminTestR GET POST /admin/errMsg AdminErrMsgR GET POST /admin/tokens AdminTokensR GET POST +/admin/crontab AdminCrontabR GET /health HealthR GET !free /instance InstanceR GET !free diff --git a/src/Cron.hs b/src/Cron.hs index 4cfc505ac..b448bf335 100644 --- a/src/Cron.hs +++ b/src/Cron.hs @@ -1,6 +1,6 @@ module Cron ( evalCronMatch - , CronNextMatch(..) + , CronNextMatch(..), _MatchAsap, _MatchAt, _MatchNone , nextCronMatch , module Cron.Types ) where @@ -84,6 +84,8 @@ consistentCronDate cd@CronDate{ cdWeekOfMonth = _, ..} = fromMaybe False $ do data CronNextMatch a = MatchAsap | MatchAt a | MatchNone deriving (Eq, Ord, Show, Read, Functor) +makePrisms ''CronNextMatch + instance Applicative CronNextMatch where pure = MatchAt _ <*> MatchNone = MatchNone diff --git a/src/Foundation.hs b/src/Foundation.hs index 284fe8ae1..bc2317469 100644 --- a/src/Foundation.hs +++ b/src/Foundation.hs @@ -2362,6 +2362,7 @@ instance YesodBreadcrumbs UniWorX where breadcrumb AdminTestR = i18nCrumb MsgMenuAdminTest $ Just AdminR breadcrumb AdminErrMsgR = i18nCrumb MsgMenuAdminErrMsg $ Just AdminR breadcrumb AdminTokensR = i18nCrumb MsgMenuAdminTokens $ Just AdminR + breadcrumb AdminCrontabR = i18nCrumb MsgBreadcrumbAdminCrontab $ Just AdminR breadcrumb SchoolListR = i18nCrumb MsgMenuSchoolList $ Just AdminR breadcrumb (SchoolR ssh SchoolEditR) = maybeT (i18nCrumb MsgBreadcrumbSchool $ Just SchoolListR) $ do @@ -2849,6 +2850,14 @@ defaultLinks = fmap catMaybes . mapM runMaybeT $ -- Define the menu items of the , navQuick' = mempty , navForceActive = False } + , NavLink + { navLabel = MsgMenuAdminCrontab + , navRoute = AdminCrontabR + , navAccess' = return True + , navType = NavTypeLink { navModal = False } + , navQuick' = mempty + , navForceActive = False + } , NavLink { navLabel = MsgMenuAdminTest , navRoute = AdminTestR diff --git a/src/Handler/Admin.hs b/src/Handler/Admin.hs index 0baadf2b8..67b387cd3 100644 --- a/src/Handler/Admin.hs +++ b/src/Handler/Admin.hs @@ -10,6 +10,7 @@ import Handler.Admin.Test as Handler.Admin import Handler.Admin.ErrorMessage as Handler.Admin import Handler.Admin.StudyFeatures as Handler.Admin import Handler.Admin.Tokens as Handler.Admin +import Handler.Admin.Crontab as Handler.Admin getAdminR :: Handler Html diff --git a/src/Handler/Admin/Crontab.hs b/src/Handler/Admin/Crontab.hs new file mode 100644 index 000000000..52c10eb8c --- /dev/null +++ b/src/Handler/Admin/Crontab.hs @@ -0,0 +1,44 @@ +module Handler.Admin.Crontab + ( getAdminCrontabR + ) where + +import Import +import Jobs +import Handler.Utils.DateTime + +import Data.Aeson.Encode.Pretty (encodePrettyToTextBuilder) + + +getAdminCrontabR :: Handler Html +getAdminCrontabR = do + jState <- getsYesod appJobState + mCrontab' <- atomically . runMaybeT $ do + JobState{jobCurrentCrontab} <- MaybeT $ tryReadTMVar jState + MaybeT $ readTVar jobCurrentCrontab + + let mCrontab = mCrontab' & mapped . _2 %~ filter (hasn't $ _1 . _MatchNone) + + siteLayoutMsg MsgMenuAdminCrontab $ do + setTitleI MsgMenuAdminCrontab + [whamlet| + $newline never + $maybe (genTime, crontab) <- mCrontab +

+ ^{formatTimeW SelFormatDateTime genTime} + + $forall (match, job) <- crontab + +
+ $case match + $of MatchAsap + _{MsgCronMatchAsap} + $of MatchNone + _{MsgCronMatchNone} + $of MatchAt t + ^{formatTimeW SelFormatDateTime t} + +
+                  #{encodePrettyToTextBuilder job}
+      $nothing
+        _{MsgAdminCrontabNotGenerated}
+    |]
diff --git a/src/Jobs.hs b/src/Jobs.hs
index b917354f0..bdf6b847f 100644
--- a/src/Jobs.hs
+++ b/src/Jobs.hs
@@ -97,6 +97,7 @@ handleJobs foundation@UniWorX{..}
       jobCrontab <- liftIO $ newTVarIO HashMap.empty
       jobConfirm <- liftIO $ newTVarIO HashMap.empty
       jobShutdown <- liftIO newEmptyTMVarIO
+      jobCurrentCrontab <- liftIO $ newTVarIO Nothing
       atomically $ putTMVar appJobState JobState
         { jobContext = JobContext{..}
         , ..
@@ -109,12 +110,12 @@ manageCrontab :: forall m.
               => UniWorX -> (forall a. m a -> m a) -> m ()
 manageCrontab foundation@UniWorX{..} unmask = do
   ch <- allocateLinkedAsync $ do
-    context <- atomically . fmap jobContext $ readTMVar appJobState
+    jState <- atomically $ readTMVar appJobState
     liftIO . unsafeHandler foundation . void $ do
       atomically . assertM_ (not . Map.null . jobWorkers) $ readTMVar appJobState
       runReaderT ?? foundation $
         writeJobCtlBlock JobCtlDetermineCrontab
-      void $ evalRWST (forever execCrontab) context HashMap.empty
+      void $ evalRWST (forever execCrontab) jState HashMap.empty
 
   let awaitTermination = guardM $
         readTMVar appJobState >>= fmap not . isEmptyTMVar . jobShutdown
@@ -252,7 +253,7 @@ stopJobCtl UniWorX{appJobState} = do
       , jobCron jSt'
       ] ++ workers
 
-execCrontab :: RWST JobContext () (HashMap JobCtl (Max UTCTime)) (HandlerFor UniWorX) ()
+execCrontab :: RWST JobState () (HashMap JobCtl (Max UTCTime)) (HandlerFor UniWorX) ()
 -- ^ Keeping a `HashMap` of the latest execution times of `JobCtl`s we have
 --   seen, wait for the time of the next job and fire it
 execCrontab = do
@@ -276,7 +277,7 @@ execCrontab = do
   refT <- liftIO getCurrentTime
   settings <- getsYesod appSettings'
   (currentCrontab, (jobCtl, nextMatch), currentState) <- mapRWST (liftIO . atomically) $ do
-    crontab <- liftBase . readTVar =<< asks jobCrontab
+    crontab <- liftBase . readTVar =<< asks (jobCrontab . jobContext)
 
     State.modify . HashMap.filterWithKey $ \k _ -> HashMap.member k crontab
     prevExec <- State.get
@@ -288,13 +289,16 @@ execCrontab = do
   do
     lastTimes <- State.get
     now <- liftIO getCurrentTime
-    $logDebugS "Crontab" . intercalate "\n" . map tshow . sortOn fst . flip map (HashMap.toList currentCrontab) $ \(job, cron) -> (,job) $ nextCronMatch appTZ (getMax <$> HashMap.lookup job lastTimes) (debouncingAcc settings job) now cron
+    let currentCrontab' = sortOn fst . flip map (HashMap.toList currentCrontab) $ \(job, cron) -> (,job) $ nextCronMatch appTZ (getMax <$> HashMap.lookup job lastTimes) (debouncingAcc settings job) now cron
+    crontabTVar <- asks jobCurrentCrontab
+    atomically . writeTVar crontabTVar $ Just (now, currentCrontab')
+    $logDebugS "Crontab" . intercalate "\n" $ "Current crontab:" : map tshow currentCrontab'
 
   let doJob = mapRWST (liftHandler . runDBJobs . setSerializable) $ do
         newCrontab <- lift $ hoist lift determineCrontab'
         when (newCrontab /= currentCrontab) $ 
           mapRWST (liftIO . atomically) $
-            liftBase . void . flip swapTVar newCrontab =<< asks jobCrontab
+            liftBase . void . flip swapTVar newCrontab =<< asks (jobCrontab . jobContext)
 
         mergeState
         newState <- State.get
@@ -315,11 +319,11 @@ execCrontab = do
     MatchAsap -> doJob
     MatchNone -> return ()
     MatchAt nextTime -> do
-      JobContext{jobCrontab} <- ask
+      crontab <- asks $ jobCrontab . jobContext
       nextTime' <- applyJitter jobCtl nextTime
       $logDebugS "Cron" [st|Waiting until #{tshow (utcToLocalTimeTZ appTZ nextTime')} to execute #{tshow jobCtl}|]
       logFunc <- askLoggerIO
-      whenM (liftIO . flip runLoggingT logFunc $ waitUntil jobCrontab currentCrontab nextTime')
+      whenM (liftIO . flip runLoggingT logFunc $ waitUntil crontab currentCrontab nextTime')
         doJob
 
   where
diff --git a/src/Jobs/Types.hs b/src/Jobs/Types.hs
index 7b36c2801..a7b56be8d 100644
--- a/src/Jobs/Types.hs
+++ b/src/Jobs/Types.hs
@@ -17,6 +17,7 @@ module Jobs.Types
   , showWorkerId, newWorkerId
   , JobQueue, jqInsert, jqDequeue
   , JobPriority(..), prioritiseJob
+  , module Cron
   ) where
 
 import Import.NoFoundation hiding (Unique, state)
@@ -37,6 +38,8 @@ import Utils.Metrics (withJobWorkerStateLbls)
 
 import qualified Prometheus (Label4)
 
+import Cron (CronNextMatch(..), _MatchAsap, _MatchAt, _MatchNone)
+
 
 data Job = JobSendNotification { jRecipient :: UserId, jNotification :: Notification }
          | JobSendTestEmail { jEmail :: Email, jMailContext :: MailContext }
@@ -253,6 +256,7 @@ data JobState = JobState
   , jobPoolManager :: Async ()
   , jobCron :: Async ()
   , jobShutdown :: TMVar ()
+  , jobCurrentCrontab :: TVar (Maybe (UTCTime, [(CronNextMatch UTCTime, JobCtl)]))
   }
 
 jobWorkerNames :: JobState -> Set JobWorkerId

From 1be971677b25689a895734d9efa5898fcbf0ca08 Mon Sep 17 00:00:00 2001
From: Gregor Kleen 
Date: Mon, 3 Aug 2020 13:52:37 +0200
Subject: [PATCH 15/17] fix(jobs): queue certain jobs at most once

---
 config/settings.yml             |  2 +-
 src/Database/Esqueleto/Utils.hs |  2 ++
 src/Handler/Admin/Test.hs       |  8 +++----
 src/Handler/Utils/Exam.hs       |  4 ++--
 src/Jobs/Queue.hs               | 38 +++++++++++++++++++--------------
 src/Jobs/Types.hs               | 14 ++++++++++++
 6 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/config/settings.yml b/config/settings.yml
index aea998f4b..5a120e906 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -35,7 +35,7 @@ bearer-expiration: 604800
 bearer-encoding: HS256
 maximum-content-length: "_env:MAX_UPLOAD_SIZE:134217728"
 session-files-expire: 3600
-prune-unreferenced-files: 600
+prune-unreferenced-files: 28800
 keep-unreferenced-files: 86400
 health-check-interval:
   matching-cluster-config: "_env:HEALTHCHECK_INTERVAL_MATCHING_CLUSTER_CONFIG:600"
diff --git a/src/Database/Esqueleto/Utils.hs b/src/Database/Esqueleto/Utils.hs
index 36c33d573..474fe9fe9 100644
--- a/src/Database/Esqueleto/Utils.hs
+++ b/src/Database/Esqueleto/Utils.hs
@@ -298,6 +298,8 @@ instance (PersistEntity val, PersistField typ) => SqlProject val typ (Maybe (E.E
   sqlProject = (E.?.)
   unSqlProject _ _ = Just
 
+infixl 8 ->.
+
 (->.) :: E.SqlExpr (E.Value a) -> Text -> E.SqlExpr (E.Value b)
 (->.) expr t = E.unsafeSqlBinOp "->" expr $ E.val t
 
diff --git a/src/Handler/Admin/Test.hs b/src/Handler/Admin/Test.hs
index c31fd691b..645152b0e 100644
--- a/src/Handler/Admin/Test.hs
+++ b/src/Handler/Admin/Test.hs
@@ -86,11 +86,9 @@ postAdminTestR = do
 
   ((emailResult, emailWidget), emailEnctype) <- runFormPost . identifyForm ("email" :: Text) $ renderAForm FormStandard emailTestForm
   formResultModal emailResult AdminTestR $ \(email, ls) -> do
-    jId <- mapWriterT runDB $ do
-      jId <- queueJob $ JobSendTestEmail email ls
-      tell . pure $ Message Success [shamlet|Email-test gestartet (Job ##{tshow (fromSqlKey jId)})|] (Just IconEmail)
-      return jId
-    runReaderT (writeJobCtl $ JobCtlPerform jId) =<< getYesod
+    mapWriterT runDBJobs $ do
+      lift . queueDBJob $ JobSendTestEmail email ls
+      tell . pure $ Message Success [shamlet|Email-test gestartet|] (Just IconEmail)
     addMessage Warning [shamlet|Inkorrekt ausgegebener Alert|] -- For testing alert handling when short circuiting; for proper (not fallback-solution) handling always use `tell` within `formResultModal`
 
   let emailWidget' = wrapForm emailWidget def
diff --git a/src/Handler/Utils/Exam.hs b/src/Handler/Utils/Exam.hs
index 8ba5f5584..cc8405762 100644
--- a/src/Handler/Utils/Exam.hs
+++ b/src/Handler/Utils/Exam.hs
@@ -1,4 +1,4 @@
-{-# OPTIONS_GHC -fno-warn-deprecations -fno-warn-incomplete-uni-patterns #-}
+{-# OPTIONS_GHC -fno-warn-incomplete-uni-patterns #-}
 
 module Handler.Utils.Exam
   ( fetchExamAux
@@ -519,7 +519,7 @@ examAutoOccurrence (hash -> seed) rule ExamAutoOccurrenceConfig{..} occurrences
                    )
     postprocess result = (resultAscList, resultUsers)
       where
-        resultAscList = pad . Map.fromListWith Set.union . accRes (pure <$> Set.lookupMin rangeAlphabet) $ (\r -> traceShow (over (traverse . _2 . traverse . traverse) CI.original r) r) result
+        resultAscList = pad . Map.fromListWith Set.union $ accRes (pure <$> Set.lookupMin rangeAlphabet) result
           where
             accRes _ [] = []
             accRes prevEnd ((occA, nsA) : (occB, nsB) : xs)
diff --git a/src/Jobs/Queue.hs b/src/Jobs/Queue.hs
index 045649ed1..18c85be59 100644
--- a/src/Jobs/Queue.hs
+++ b/src/Jobs/Queue.hs
@@ -80,22 +80,28 @@ writeJobCtlBlock :: (MonadThrow m, MonadIO m, MonadReader UniWorX m) => JobCtl -
 -- | Pass an instruction to the `Job`-Workers and block until it was acted upon
 writeJobCtlBlock = writeJobCtlBlock' writeJobCtl
 
-queueJobUnsafe :: Bool -> Job -> YesodDB UniWorX QueuedJobId
+queueJobUnsafe :: Bool -> Job -> YesodDB UniWorX (Maybe QueuedJobId)
 queueJobUnsafe queuedJobWriteLastExec job = do
   $logInfoS "queueJob" $ tshow job
-  queuedJobCreationTime <- liftIO getCurrentTime
-  queuedJobCreationInstance <- getsYesod appInstanceID
-  insert QueuedJob
-    { queuedJobContent = toJSON job
-    , queuedJobLockInstance = Nothing
-    , queuedJobLockTime = Nothing
-    , ..
-    }
-  -- We should not immediately notify a worker; instead wait for the transaction to finish first
-  -- writeJobCtl $ JobCtlPerform jId -- FIXME: Should do fancy load balancing across instances (or something)
-  -- return jId
+  
+  doQueue <- fmap not . and2M (return $ jobNoQueueSame job) $ exists [ QueuedJobContent ==. toJSON job ]
+  
+  if
+    | doQueue -> Just <$> do
+        queuedJobCreationTime <- liftIO getCurrentTime
+        queuedJobCreationInstance <- getsYesod appInstanceID
+        insert QueuedJob
+          { queuedJobContent = toJSON job
+          , queuedJobLockInstance = Nothing
+          , queuedJobLockTime = Nothing
+          , ..
+          }
+        -- We should not immediately notify a worker; instead wait for the transaction to finish first
+        -- writeJobCtl $ JobCtlPerform jId -- FIXME: Should do fancy load balancing across instances (or something)
+        -- return jId
+    | otherwise -> return Nothing
 
-queueJob :: (MonadHandler m, HandlerSite m ~ UniWorX) => Job -> m QueuedJobId
+queueJob :: (MonadHandler m, HandlerSite m ~ UniWorX) => Job -> m (Maybe QueuedJobId)
 -- ^ Queue a job for later execution
 --
 -- Makes no guarantees as to when it will be executed (`queueJob'`) and does not interact with any running database transactions (`runDBJobs`)
@@ -105,15 +111,15 @@ queueJob' :: (MonadThrow m, MonadHandler m, HandlerSite m ~ UniWorX) => Job -> m
 -- ^ `queueJob` followed by `writeJobCtl` `JobCtlPerform` to ensure, that it is executed asap
 queueJob' job = do
   app <- getYesod
-  queueJob job >>= flip runReaderT app . writeJobCtl . JobCtlPerform
+  queueJob job >>= maybe (return ()) (flip runReaderT app . writeJobCtl . JobCtlPerform)
 
 -- | Slightly modified Version of `DB` for `runDBJobs`
 type JobDB = YesodJobDB UniWorX
 
 queueDBJob, queueDBJobCron :: Job -> YesodJobDB UniWorX ()
 -- | Queue a job as part of a database transaction and execute it after the transaction succeeds
-queueDBJob job = mapReaderT lift (queueJobUnsafe False job) >>= tell . Set.singleton
-queueDBJobCron job = mapReaderT lift (queueJobUnsafe True job) >>= tell . Set.singleton
+queueDBJob job = mapReaderT lift (queueJobUnsafe False job) >>= tell . maybe Set.empty Set.singleton
+queueDBJobCron job = mapReaderT lift (queueJobUnsafe True job) >>= tell . maybe Set.empty Set.singleton
 
 sinkDBJobs :: ConduitT Job Void (YesodJobDB UniWorX) ()
 -- | Queue many jobs as part of a database transaction and execute them after the transaction passes
diff --git a/src/Jobs/Types.hs b/src/Jobs/Types.hs
index a7b56be8d..11fe8b12e 100644
--- a/src/Jobs/Types.hs
+++ b/src/Jobs/Types.hs
@@ -17,6 +17,7 @@ module Jobs.Types
   , showWorkerId, newWorkerId
   , JobQueue, jqInsert, jqDequeue
   , JobPriority(..), prioritiseJob
+  , jobNoQueueSame
   , module Cron
   ) where
 
@@ -235,6 +236,19 @@ prioritiseJob (JobCtlGenerateHealthReport _)  = JobPrioRealtime
 prioritiseJob  JobCtlDetermineCrontab         = JobPrioRealtime
 prioritiseJob  _                              = JobPrioBatch
 
+jobNoQueueSame :: Job -> Bool
+jobNoQueueSame = \case
+  JobSendPasswordReset{}       -> True
+  JobTruncateTransactionLog{}  -> True
+  JobPruneInvitations{}        -> True
+  JobDeleteTransactionLogIPs{} -> True
+  JobSynchroniseLdapUser{}     -> True
+  JobChangeUserDisplayEmail{}  -> True
+  JobPruneSessionFiles{}       -> True
+  JobPruneUnreferencedFiles{}  -> True
+  JobInjectFiles{}             -> True
+  _                            -> False
+
 
 newtype JobQueue = JobQueue { getJobQueue :: MaxPQueue JobPriority JobCtl }
   deriving (Eq, Ord, Read, Show)

From a9cdfcc7e14440b24b43bf1ff26a94168ddb29ba Mon Sep 17 00:00:00 2001
From: Gregor Kleen 
Date: Mon, 3 Aug 2020 14:16:03 +0200
Subject: [PATCH 16/17] refactor: hlint

---
 src/Handler/Admin/Crontab.hs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Handler/Admin/Crontab.hs b/src/Handler/Admin/Crontab.hs
index 52c10eb8c..bc8a3097f 100644
--- a/src/Handler/Admin/Crontab.hs
+++ b/src/Handler/Admin/Crontab.hs
@@ -16,7 +16,7 @@ getAdminCrontabR = do
     JobState{jobCurrentCrontab} <- MaybeT $ tryReadTMVar jState
     MaybeT $ readTVar jobCurrentCrontab
 
-  let mCrontab = mCrontab' & mapped . _2 %~ filter (hasn't $ _1 . _MatchNone)
+  let mCrontab = mCrontab' <&> _2 %~ filter (hasn't $ _1 . _MatchNone)
 
   siteLayoutMsg MsgMenuAdminCrontab $ do
     setTitleI MsgMenuAdminCrontab

From 1d956a5fdc1cfbf57423925e34cd056cec2f6f9d Mon Sep 17 00:00:00 2001
From: Gregor Kleen 
Date: Mon, 3 Aug 2020 14:21:36 +0200
Subject: [PATCH 17/17] chore(release): 18.5.0

---
 CHANGELOG.md      | 14 ++++++++++++++
 package-lock.json |  2 +-
 package.json      |  2 +-
 package.yaml      |  2 +-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 36b77fd11..eb67e3710 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,20 @@
 
 All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.
 
+## [18.5.0](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/compare/v18.4.0...v18.5.0) (2020-08-03)
+
+
+### Bug Fixes
+
+* **jobs:** queue certain jobs at most once ([1be9716](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/1be9716))
+
+
+### Features
+
+* admin-crontab-r ([460c133](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/460c133))
+
+
+
 ## [18.4.0](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/compare/v18.3.0...v18.4.0) (2020-08-02)
 
 
diff --git a/package-lock.json b/package-lock.json
index 6357927a7..97223ff05 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,6 +1,6 @@
 {
   "name": "uni2work",
-  "version": "18.4.0",
+  "version": "18.5.0",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
diff --git a/package.json b/package.json
index 91e5e1937..c5e99919c 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "uni2work",
-  "version": "18.4.0",
+  "version": "18.5.0",
   "description": "",
   "keywords": [],
   "author": "",
diff --git a/package.yaml b/package.yaml
index ec34e6c40..182eddbf9 100644
--- a/package.yaml
+++ b/package.yaml
@@ -1,5 +1,5 @@
 name:    uniworx
-version: 18.4.0
+version: 18.5.0
 
 dependencies:
   - base