Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize getting all feature configs #4002

Merged
merged 29 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
bba8ade
WIP
akshaymankar Apr 15, 2024
4752915
More WIP
akshaymankar Apr 15, 2024
d9e21ab
Finish implementing getAllFeatureConfigs for teams in the interpreter
akshaymankar Apr 16, 2024
ba73724
Use TeamFeatures.getAllFeatureConfigs
akshaymankar Apr 16, 2024
39c3e57
[wip] legalhold config tests
battermann Apr 16, 2024
9c8e023
wip tests, todo: allow listed config
battermann Apr 16, 2024
b017752
Failing test for legalhold
akshaymankar Apr 17, 2024
9dee21e
Add test for legalhold whitelist team and implicit consent
akshaymankar Apr 17, 2024
1499734
Better errors
akshaymankar Apr 17, 2024
63717f0
Slightly faster test
akshaymankar Apr 17, 2024
3cbec66
More complete test for LH setting being "whitelist-teams-and-implicit…
akshaymankar Apr 17, 2024
89df08d
WIP: TeamFeatures.testExposeInvitationURLsToTeamAdminConfig
akshaymankar Apr 17, 2024
6feabb2
tests for expose invitation url feature flag
battermann Apr 17, 2024
8529272
Merge remote-tracking branch 'origin/optimize-feature-configs-tests' …
fisx Apr 17, 2024
306be9e
Implement special cases.
fisx Apr 17, 2024
ca704c4
wip
battermann Apr 17, 2024
390ebc6
wip
battermann Apr 18, 2024
eca8f4b
Merge remote-tracking branch 'origin/develop' into optimize-feature-c…
battermann Apr 18, 2024
a2e262e
in the middle of debugging
battermann Apr 18, 2024
bb6127a
wip writing more tests
battermann Apr 18, 2024
63207f8
make expose url feature test pass
battermann Apr 18, 2024
a685b15
make lh tests pass
battermann Apr 18, 2024
49ad00a
changelog
fisx Apr 19, 2024
ecd67c7
clean up old tests
battermann Apr 19, 2024
3eb9132
more cleanup
battermann Apr 19, 2024
fdff77e
clean up tests
battermann Apr 19, 2024
c8c7cf8
lint
fisx Apr 19, 2024
46bc398
Fixup
fisx Apr 19, 2024
672ba31
Make TODO a FUTUREWORK.
fisx Apr 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Optimize getting all feature configs
2 changes: 1 addition & 1 deletion integration/test/API/GalleyInternal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ setTeamFeatureStatusExpectHttpStatus :: (HasCallStack, MakesValue domain, MakesV
setTeamFeatureStatusExpectHttpStatus domain team featureName status httpStatus = do
tid <- asString team
req <- baseRequest domain Galley Unversioned $ joinHttpPath ["i", "teams", tid, "features", featureName]
bindResponse (submit "PATCH" $ req & addJSONObject ["status" .= status]) $ \res ->
bindResponse (submit "PATCH" $ req & addJSONObject ["status" .= status]) $ \res -> do
res.status `shouldMatchInt` httpStatus

setTeamFeatureLockStatus :: (HasCallStack, MakesValue domain, MakesValue team) => domain -> team -> String -> String -> App ()
Expand Down
8 changes: 4 additions & 4 deletions integration/test/Test/FeatureFlags.hs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ testExposeInvitationURLsToTeamAdminConfig = do
runCodensity (acquireResources 1 resourcePool) $ \[testBackend] -> do
let domain = testBackend.berDomain

-- Happy case: DB has no config for the team
let testNoAllowlistEntry = runCodensity (startDynamicBackend testBackend $ cfgExposeInvitationURLsTeamAllowlist ([] :: [String])) $ \_ -> do
(owner, tid, _) <- createTeam domain 1
checkFeature "exposeInvitationURLsToTeamAdmin" owner tid disabledLocked
Expand All @@ -150,6 +149,7 @@ testExposeInvitationURLsToTeamAdminConfig = do
Internal.setTeamFeatureStatusExpectHttpStatus domain tid "exposeInvitationURLsToTeamAdmin" "disabled" 200
pure (owner, tid)

-- Happy case: DB has no config for the team
(owner, tid) <- testNoAllowlistEntry

-- Interesting case: The team is in the allow list
Expand All @@ -172,12 +172,12 @@ checkFeature feature user tid expected = do
bindResponse (Internal.getTeamFeature domain tidStr feature) $ \resp -> do
resp.status `shouldMatchInt` 200
resp.json `shouldMatch` expected
bindResponse (Public.getFeatureConfigs user) $ \resp -> do
resp.status `shouldMatchInt` 200
resp.json %. feature `shouldMatch` expected
bindResponse (Public.getTeamFeatures user tid) $ \resp -> do
resp.status `shouldMatchInt` 200
resp.json %. feature `shouldMatch` expected
bindResponse (Public.getTeamFeature user tid feature) $ \resp -> do
resp.status `shouldMatchInt` 200
resp.json `shouldMatch` expected
bindResponse (Public.getFeatureConfigs user) $ \resp -> do
resp.status `shouldMatchInt` 200
resp.json %. feature `shouldMatch` expected
4 changes: 3 additions & 1 deletion libs/wire-api/test/unit/Test/Wire/API/Run.hs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import Test.Wire.API.Routes.Version qualified as Routes.Version
import Test.Wire.API.Routes.Version.Wai qualified as Routes.Version.Wai
import Test.Wire.API.Swagger qualified as Swagger
import Test.Wire.API.Team.Export qualified as Team.Export
import Test.Wire.API.Team.Feature qualified as Team.Feature
import Test.Wire.API.Team.Member qualified as Team.Member
import Test.Wire.API.User qualified as User
import Test.Wire.API.User.Auth qualified as User.Auth
Expand Down Expand Up @@ -69,5 +70,6 @@ main =
unsafePerformIO Routes.Version.Wai.tests,
RawJson.tests,
OAuth.tests,
Password.tests
Password.tests,
Team.Feature.tests
]
92 changes: 92 additions & 0 deletions libs/wire-api/test/unit/Test/Wire/API/Team/Feature.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
{-# OPTIONS_GHC -Wno-incomplete-uni-patterns #-}

-- This file is part of the Wire Server implementation.
--
-- Copyright (C) 2024 Wire Swiss GmbH <opensource@wire.com>
--
-- This program is free software: you can redistribute it and/or modify it under
-- the terms of the GNU Affero General Public License as published by the Free
-- Software Foundation, either version 3 of the License, or (at your option) any
-- later version.
--
-- This program is distributed in the hope that it will be useful, but WITHOUT
-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
-- details.
--
-- You should have received a copy of the GNU Affero General Public License along
-- with this program. If not, see <https://www.gnu.org/licenses/>.

module Test.Wire.API.Team.Feature (tests) where

import Imports
import Test.Tasty
import Test.Tasty.HUnit
import Wire.API.Team.Feature

tests :: TestTree
tests =
testGroup
"Wire.API.Team.Feature"
[ testCase "no lock status in DB" testComputeFeatureConfigForTeamUserLsIsNothing,
testCase "feature is locked in DB" testComputeFeatureConfigForTeamUserLocked,
testCase "feature is unlocked in DB but has no feature status" testComputeFeatureConfigForTeamUserUnlocked,
testCase "feature is unlocked in DB and has feature status" testComputeFeatureConfigForTeamWithDbStatus
]

testComputeFeatureConfigForTeamUserLsIsNothing :: Assertion
testComputeFeatureConfigForTeamUserLsIsNothing = do
let mStatusDb = undefined
let mLockStatusDb = Nothing
let defStatus =
withStatus
FeatureStatusEnabled
LockStatusLocked
ExposeInvitationURLsToTeamAdminConfig
FeatureTTLUnlimited
let expected = defStatus
let actual = computeFeatureConfigForTeamUser @ExposeInvitationURLsToTeamAdminConfig mStatusDb mLockStatusDb defStatus
actual @?= expected

testComputeFeatureConfigForTeamUserLocked :: Assertion
testComputeFeatureConfigForTeamUserLocked = do
let mStatusDb = undefined
let mLockStatusDb = Just LockStatusLocked
let defStatus =
withStatus
FeatureStatusEnabled
LockStatusLocked
ExposeInvitationURLsToTeamAdminConfig
FeatureTTLUnlimited
let expected = defStatus
let actual = computeFeatureConfigForTeamUser @ExposeInvitationURLsToTeamAdminConfig mStatusDb mLockStatusDb defStatus
actual @?= expected

testComputeFeatureConfigForTeamUserUnlocked :: Assertion
testComputeFeatureConfigForTeamUserUnlocked = do
let mStatusDb = Nothing
let mLockStatusDb = Just LockStatusUnlocked
let defStatus =
withStatus
FeatureStatusEnabled
LockStatusLocked
ExposeInvitationURLsToTeamAdminConfig
FeatureTTLUnlimited
let expected = defStatus & setLockStatus LockStatusUnlocked
let actual = computeFeatureConfigForTeamUser @ExposeInvitationURLsToTeamAdminConfig mStatusDb mLockStatusDb defStatus
actual @?= expected

testComputeFeatureConfigForTeamWithDbStatus :: Assertion
testComputeFeatureConfigForTeamWithDbStatus = do
let mStatusDb =
Just . forgetLock $
withStatus
FeatureStatusDisabled
LockStatusUnlocked
ExposeInvitationURLsToTeamAdminConfig
FeatureTTLUnlimited
let mLockStatusDb = Just LockStatusUnlocked
let defStatus = undefined
let (Just expected) = withUnlocked <$> mStatusDb
let actual = computeFeatureConfigForTeamUser @ExposeInvitationURLsToTeamAdminConfig mStatusDb mLockStatusDb defStatus
actual @?= expected
1 change: 1 addition & 0 deletions libs/wire-api/wire-api.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ test-suite wire-api-tests
Test.Wire.API.Run
Test.Wire.API.Swagger
Test.Wire.API.Team.Export
Test.Wire.API.Team.Feature
Test.Wire.API.Team.Member
Test.Wire.API.User
Test.Wire.API.User.Auth
Expand Down
2 changes: 2 additions & 0 deletions services/galley/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
, conduit
, containers
, cookie
, cql
, crypton
, crypton-x509
, currency-codes
Expand Down Expand Up @@ -151,6 +152,7 @@ mkDerivation {
cereal
comonad
containers
cql
crypton
crypton-x509
currency-codes
Expand Down
2 changes: 2 additions & 0 deletions services/galley/galley.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ library
Galley.Cassandra.Conversation.MLS
Galley.Cassandra.ConversationList
Galley.Cassandra.CustomBackend
Galley.Cassandra.GetAllTeamFeatureConfigs
Galley.Cassandra.Instances
Galley.Cassandra.LegalHold
Galley.Cassandra.Proposal
Expand Down Expand Up @@ -302,6 +303,7 @@ library
, cereal >=0.4
, comonad
, containers >=0.5
, cql
, crypton
, crypton-x509
, currency-codes >=2.0
Expand Down
1 change: 1 addition & 0 deletions services/galley/src/Galley/API/Internal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import Galley.API.Teams.Features
import Galley.API.Update qualified as Update
import Galley.API.Util
import Galley.App
import Galley.Cassandra.TeamFeatures (getAllFeatureConfigsForServer)
import Galley.Data.Conversation qualified as Data
import Galley.Effects
import Galley.Effects.BackendNotificationQueueAccess
Expand Down
1 change: 0 additions & 1 deletion services/galley/src/Galley/API/Teams/Features.hs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ module Galley.API.Teams.Features
setFeatureStatusInternal,
patchFeatureStatusInternal,
getFeatureStatusForUser,
getAllFeatureConfigsForServer,
getAllFeatureConfigsForTeam,
getAllFeatureConfigsForUser,
updateLockStatus,
Expand Down
38 changes: 2 additions & 36 deletions services/galley/src/Galley/API/Teams/Features/Get.hs
Original file line number Diff line number Diff line change
Expand Up @@ -193,15 +193,13 @@ getAllFeatureConfigsForUser zusr = do
maybe (throwS @'NotATeamMember) (const $ pure ()) zusrMembership
case mbTeam of
Just tid ->
getAllFeatureConfigsTeam tid
TeamFeatures.getAllFeatureConfigs tid
Nothing ->
getAllFeatureConfigsUser zusr

getAllFeatureConfigsForTeam ::
forall r.
( Member (ErrorS 'NotATeamMember) r,
Member (Input Opts) r,
Member LegalHoldStore r,
Member TeamFeatureStore r,
Member TeamStore r
) =>
Expand All @@ -211,7 +209,7 @@ getAllFeatureConfigsForTeam ::
getAllFeatureConfigsForTeam luid tid = do
zusrMembership <- getTeamMember tid (tUnqualified luid)
maybe (throwS @'NotATeamMember) (const $ pure ()) zusrMembership
getAllFeatureConfigsTeam tid
TeamFeatures.getAllFeatureConfigs tid

getAllFeatureConfigsForServer ::
forall r.
Expand Down Expand Up @@ -276,38 +274,6 @@ getAllFeatureConfigsUser uid =
<*> getConfigForUser @EnforceFileDownloadLocationConfig uid
<*> getConfigForUser @LimitedEventFanoutConfig uid

getAllFeatureConfigsTeam ::
forall r.
( Member (Input Opts) r,
Member LegalHoldStore r,
Member TeamFeatureStore r,
Member TeamStore r
) =>
TeamId ->
Sem r AllFeatureConfigs
getAllFeatureConfigsTeam tid =
AllFeatureConfigs
<$> getConfigForTeam @LegalholdConfig tid
<*> getConfigForTeam @SSOConfig tid
<*> getConfigForTeam @SearchVisibilityAvailableConfig tid
<*> getConfigForTeam @SearchVisibilityInboundConfig tid
<*> getConfigForTeam @ValidateSAMLEmailsConfig tid
<*> getConfigForTeam @DigitalSignaturesConfig tid
<*> getConfigForTeam @AppLockConfig tid
<*> getConfigForTeam @FileSharingConfig tid
<*> getConfigForTeam @ClassifiedDomainsConfig tid
<*> getConfigForTeam @ConferenceCallingConfig tid
<*> getConfigForTeam @SelfDeletingMessagesConfig tid
<*> getConfigForTeam @GuestLinksConfig tid
<*> getConfigForTeam @SndFactorPasswordChallengeConfig tid
<*> getConfigForTeam @MLSConfig tid
<*> getConfigForTeam @ExposeInvitationURLsToTeamAdminConfig tid
<*> getConfigForTeam @OutlookCalIntegrationConfig tid
<*> getConfigForTeam @MlsE2EIdConfig tid
<*> getConfigForTeam @MlsMigrationConfig tid
<*> getConfigForTeam @EnforceFileDownloadLocationConfig tid
<*> getConfigForTeam @LimitedEventFanoutConfig tid

-- | Note: this is an internal function which doesn't cover all features, e.g. LegalholdConfig
genericGetConfigForTeam ::
forall cfg r.
Expand Down
13 changes: 12 additions & 1 deletion services/galley/src/Galley/App.hs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import Cassandra hiding (Set)
import Cassandra.Util (initCassandraForService)
import Control.Error hiding (err)
import Control.Lens hiding ((.=))
import Data.Id
import Data.Metrics.Middleware
import Data.Misc
import Data.Qualified
Expand Down Expand Up @@ -83,6 +84,7 @@ import Galley.Options hiding (brig, endpoint, federator)
import Galley.Options qualified as O
import Galley.Queue
import Galley.Queue qualified as Q
import Galley.Types.Teams (FeatureLegalHold)
import Galley.Types.Teams qualified as Teams
import HTTP2.Client.Manager (Http2Manager, http2ManagerWithSSLCtx)
import Imports hiding (forkIO)
Expand Down Expand Up @@ -259,6 +261,8 @@ evalGalley e =
. interpretWaiRoutes
. runInputConst (e ^. options)
. runInputConst (toLocalUnsafe (e ^. options . settings . federationDomain) ())
. interpretTeamFeatureSpecialContext e
. runInputSem getAllFeatureConfigsForServer
. interpretInternalTeamListToCassandra
. interpretTeamListToCassandra
. interpretLegacyConversationListToCassandra
Expand All @@ -268,11 +272,11 @@ evalGalley e =
. interpretTeamMemberStoreToCassandra lh
. interpretTeamStoreToCassandra lh
. interpretTeamNotificationStoreToCassandra
. interpretTeamFeatureStoreToCassandra
. interpretServiceStoreToCassandra
. interpretSearchVisibilityStoreToCassandra
. interpretMemberStoreToCassandra
. interpretLegalHoldStoreToCassandra lh
. interpretTeamFeatureStoreToCassandra
. interpretCustomBackendStoreToCassandra
. randomToIO
. interpretSubConversationStoreToCassandra
Expand All @@ -292,3 +296,10 @@ evalGalley e =
. interpretBrigAccess
where
lh = view (options . settings . featureFlags . Teams.flagLegalHold) e

interpretTeamFeatureSpecialContext :: Env -> Sem (Input (Maybe [TeamId], FeatureLegalHold) ': r) a -> Sem r a
interpretTeamFeatureSpecialContext e =
runInputConst
( e ^. options . settings . exposeInvitationURLsTeamAllowlist,
e ^. options . settings . featureFlags . Teams.flagLegalHold
)
Loading
Loading