Skip to content

Commit

Permalink
feat: Remove the Version argument from querySessionId.
Browse files Browse the repository at this point in the history
We aren't using it, and it should not factor into a query by session
ID, anyway, because the session ID alone is sufficient to choose a
unique session in our schema.

Closes #268
  • Loading branch information
dhess committed Mar 3, 2022
1 parent d8a7b71 commit 0aa8017
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 39 deletions.
5 changes: 1 addition & 4 deletions primer-rel8/src/Primer/Database/Rel8/Rel8Db.hs
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,7 @@ instance (MonadThrow m, MonadIO m) => MonadDb (Rel8DbT m) where
-- session names loaded from the database.
safeMkSession (s, n) = Session s (safeMkSessionName n)

-- Note: we ignore the stored Primer version for now.
--
-- See https://github.com/hackworthltd/primer/issues/268
querySessionId _ sid = do
querySessionId sid = do
result <- runStatement (LoadSessionError sid) $ select $ sessionById sid
case result of
[] -> return $ Left $ SessionIdNotFound sid
Expand Down
2 changes: 1 addition & 1 deletion primer-rel8/test/Tests/InsertSession.hs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ test_insertSession_roundtrip = testCaseSteps "insertSession database round-tripp
sessionId <- liftIO newSessionId
insertSession version sessionId newApp name
step "Retrieve it"
result <- querySessionId version sessionId
result <- querySessionId sessionId
result @?= Right (SessionData newApp name)

test_insertSession_failure :: TestTree
Expand Down
6 changes: 3 additions & 3 deletions primer-rel8/test/Tests/QuerySessionId.hs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ test_querySessionId = testCaseSteps "querySessionId corner cases" $ \step' ->

step "Attempt to look up a session that doesn't exist"
nonexistentSessionId <- liftIO newSessionId
r1 <- querySessionId version nonexistentSessionId
r1 <- querySessionId nonexistentSessionId
r1 @?= Left (SessionIdNotFound nonexistentSessionId)

step "Attempt to fetch a session whose program is invalid"
Expand All @@ -81,7 +81,7 @@ test_querySessionId = testCaseSteps "querySessionId corner cases" $ \step' ->
, Schema.name = fromSessionName invalidProgramName
}
liftIO $ insertSessionRow invalidProgramRow conn
assertException "querySessionId" (expectedError invalidProgramSessionId) $ querySessionId version invalidProgramSessionId
assertException "querySessionId" (expectedError invalidProgramSessionId) $ querySessionId invalidProgramSessionId

step "Attempt to fetch a session whose name is invalid"
invalidNameSessionId <- liftIO newSessionId
Expand All @@ -95,7 +95,7 @@ test_querySessionId = testCaseSteps "querySessionId corner cases" $ \step' ->
, Schema.name = invalidName
}
liftIO $ insertSessionRow invalidNameRow conn
r3 <- querySessionId version invalidNameSessionId
r3 <- querySessionId invalidNameSessionId
-- In this scenario, we should get the program back with the
-- default session name, rather than the invalid name we used to
-- store it in the database.
Expand Down
16 changes: 3 additions & 13 deletions primer-rel8/test/Tests/UpdateSessionApp.hs
Original file line number Diff line number Diff line change
Expand Up @@ -47,30 +47,20 @@ test_updateSessionApp_roundtrip = testCaseSteps "updateSessionApp database round

step "Update it with the same version and app"
updateSessionApp version sessionId newEmptyApp
r1 <- querySessionId version sessionId
r1 <- querySessionId sessionId
r1 @?= Right (SessionData newEmptyApp name)

step "Update it with a new version, but the same app"
let newVersion = "new-" <> version
updateSessionApp newVersion sessionId newEmptyApp
r2 <- querySessionId newVersion sessionId
r2 <- querySessionId sessionId
r2 @?= Right (SessionData newEmptyApp name)

step "Update it with a new app"
updateSessionApp newVersion sessionId newApp
r3 <- querySessionId newVersion sessionId
r3 <- querySessionId sessionId
r3 @?= Right (SessionData newApp name)

-- Note: at the moment, we ignore the stored Primer version when
-- we query the database. This is a bit odd, but it's not yet
-- clear whether it'll be useful to include the version in the
-- query.
--
-- See https://github.com/hackworthltd/primer/issues/268
step "We can still query the program using the old version"
r4 <- querySessionId version sessionId
r4 @?= Right (SessionData newApp name)

test_updateSessionApp_failure :: TestTree
test_updateSessionApp_failure = testCaseSteps "updateSessionApp failure modes" $ \step' ->
withDbSetup \conn -> do
Expand Down
16 changes: 3 additions & 13 deletions primer-rel8/test/Tests/UpdateSessionName.hs
Original file line number Diff line number Diff line change
Expand Up @@ -46,31 +46,21 @@ test_updateSessionName_roundtrip = testCaseSteps "updateSessionName database rou

step "Update it with the same version and name"
updateSessionName version sessionId name
r1 <- querySessionId version sessionId
r1 <- querySessionId sessionId
r1 @?= Right (SessionData newEmptyApp name)

step "Update it with a new version, but the same name"
let newVersion = "new-" <> version
updateSessionName newVersion sessionId name
r2 <- querySessionId newVersion sessionId
r2 <- querySessionId sessionId
r2 @?= Right (SessionData newEmptyApp name)

step "Update it with a new name"
let newName = safeMkSessionName "new new app"
updateSessionName newVersion sessionId newName
r3 <- querySessionId newVersion sessionId
r3 <- querySessionId sessionId
r3 @?= Right (SessionData newEmptyApp newName)

-- Note: at the moment, we ignore the stored Primer version when
-- we query the database. This is a bit odd, but it's not yet
-- clear whether it'll be useful to include the version in the
-- query.
--
-- See https://github.com/hackworthltd/primer/issues/268
step "We can still query the program using the old version"
r4 <- querySessionId version sessionId
r4 @?= Right (SessionData newEmptyApp newName)

test_updateSessionName_failure :: TestTree
test_updateSessionName_failure = testCaseSteps "updateSessionName failure modes" $ \step' ->
withDbSetup \conn -> do
Expand Down
9 changes: 4 additions & 5 deletions primer/src/Primer/Database.hs
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,8 @@ class (Monad m) => MonadDb m where
-- | Query a session ID from the database.
--
-- Returns 'Left' with a 'DbError' if the query failed (session
-- doesn't exist, version mismatch, etc.), 'Right' with the
-- 'SessionData' if successful.
querySessionId :: Version -> SessionId -> m (Either DbError SessionData)
-- doesn't exist), 'Right' with the 'SessionData' if successful.
querySessionId :: SessionId -> m (Either DbError SessionData)

-- | Routine errors that can occur during 'MonadDb' computations.
--
Expand Down Expand Up @@ -293,7 +292,7 @@ instance (MonadIO m) => MonadDb (NullDbT m) where
ss <- ask
kvs <- liftIO $ atomically $ ListT.toList $ StmMap.listT ss
pure $ pageList ol $ uncurry Session . second sessionName <$> kvs
querySessionId _ sid = pure $ Left $ SessionIdNotFound sid
querySessionId sid = pure $ Left $ SessionIdNotFound sid

-- | The database service computation.
--
Expand Down Expand Up @@ -323,7 +322,7 @@ serve cfg =
liftIO $ atomically $ putTMVar status result
where
loadSession = do
queryResult <- querySessionId v sid
queryResult <- querySessionId sid
case queryResult of
Left (SessionIdNotFound s) ->
return $ Failure $ "Couldn't load the requested session: no such session ID " <> UUID.toText s
Expand Down

0 comments on commit 0aa8017

Please sign in to comment.