refactor: create local stores and UTS - WPB-12100 #2141

jullianm · 2024-11-08T09:35:35Z

WPB-12100 [iOS] Create remaining local stores and add UTs for all stores

Key points

This PR isolates tests for our storage layer (local stores) and revisits tests for all our existing repositories.
Missing local stores were also created so the storage logic could be moved out from related repositories.

BEFORE:

Local stores were indirectly tested through our repositories

AFTER:

Each local store is directly tested.
Mock local stores are injected into repositories, repositories tests were revisited consequently.

In addition:

Removes super.setUp() and super.tearDown() code from all tests
Ensures local stores know nothing about API objects (using model mappers)
Uses .mockID instances from WireTestingPackage
Harmonizes tests naming

Testing

Add dedicated tests for all local stores
Revisit repositories tests

Checklist

Title contains a reference JIRA issue number like [WPB-XXX].
Description is filled and free of optional paragraphs.
Adds/updates automated tests.

UI accessibility checklist

If your PR includes UI changes, please utilize this checklist:

Make sure you use the API for UI elements that support large fonts.
All colors are taken from WireDesign.ColorTheme or constructed using WireDesign.BaseColorPalette.
New UI elements have Accessibility strings for VoiceOver.

…ects in storage layer

jullianm · 2024-11-08T09:39:36Z

WireDomain/Sources/WireDomain/Repositories/Connections/ConnectionsLocalStore.swift

    func storeConnection(
-        _ connectionPayload: Connection
+        _ connectionInfo: ConnectionInfo


You'll see changes like this in this PR: storage layer should not be aware of API objects so we create a domain model between repository and local store, here's the steps:

Repo will fetch API model from remote.

Repo will prepare data for the local store by mapping it to a domain model

Repo will pass this model to the local store

Local store will rely that model to manipulate (fetch, update, create) NSManagedObject

So now, it looks like all NSManagedObjects or context are isolated to Local stores, right?
I wonder if they would be cases where we want to be more flexible : thinking about the save contexts strategy we discussed.

Or going even further isolate all this to a WireStorage package

jullianm · 2024-11-08T09:40:42Z

WireDomain/Sources/WireDomain/Repositories/Connections/ConnectionsRepository.swift

@@ -70,7 +70,7 @@ public struct ConnectionsRepository: ConnectionsRepositoryProtocol {
            await withThrowingTaskGroup(of: Void.self) { taskGroup in
                for connection in connections {
                    taskGroup.addTask {
-                        try await connectionsLocalStore.storeConnection(connection)
+                        try await connectionsLocalStore.storeConnection(connection.toDomainModel())


prepare data for the store by mapping it to a domain model (so the local store doesn't know about the API layer)

jullianm · 2024-11-08T09:41:33Z

...omain/Sources/WireDomain/Repositories/ConversationsLabels/ConversationLabelsLocalStore.swift

+    ) async throws
+}
+
+public final class ConversationLabelsLocalStore: ConversationLabelsLocalStoreProtocol {


This store was created. Storage operations were moved out from the related repository.

jullianm · 2024-11-08T09:42:30Z

WireDomain/Sources/WireDomain/Repositories/FeatureConfig/FeatureConfigLocalStore.swift

+
+}
+
+final class FeatureConfigLocalStore: FeatureConfigLocalStoreProtocol {


This store was created. Storage operations were moved out from related repository.

jullianm · 2024-11-08T09:43:50Z

WireDomain/Sources/WireDomain/Repositories/FeatureConfig/Models/FeatureState.swift

+    let name: Feature.Name
+    let isEnabled: Bool
+    let shouldNotifyUser: Bool
+}


these models were encapsulated in the repository, it was moved out and dedicated files were created in the FeatureConfig folder.

jullianm · 2024-11-08T09:44:42Z

WireDomain/Sources/WireDomain/Repositories/Team/Models/TeamMemberInfo.swift

+    let selfPermission: Int64?
+    let creatorID: UUID?
+    let creationDate: Date?
+}


same as mentioned above, mapping API object to domain model to pass them to the local store

jullianm · 2024-11-08T09:45:17Z

WireDomain/Sources/WireDomain/Repositories/Team/TeamLocalStore.swift

+import WireDataModel
+
+// sourcery: AutoMockable
+public protocol TeamLocalStoreProtocol {


This store was created and storage operations were moved out from related repository

jullianm · 2024-11-08T09:45:52Z

WireDomain/Sources/WireDomain/Repositories/Team/TeamModelMappings.swift

+// along with this program. If not, see http://www.gnu.org/licenses/.
+//
+
+import WireAPI


TeamModelMappings used to convert API objects to domain model.

jullianm · 2024-11-08T09:46:41Z

WireDomain/Sources/WireDomain/Repositories/UpdateEvents/UpdateEventsLocalStore.swift

+import WireFoundation
+
+// sourcery: AutoMockable
+protocol UpdateEventsLocalStoreProtocol {


This store was created and storage operations moved out from related repository

jullianm · 2024-11-08T09:47:54Z

WireDomain/Sources/WireDomain/Repositories/User/Models/NewUserInfo.swift

+
+import WireDataModel
+
+public struct NewUserInfo: Sendable {


This is used to map API object User to domain model

jullianm · 2024-11-08T09:48:47Z

WireDomain/Sources/WireDomain/Repositories/User/Models/UserUpdateInfo.swift

+
+import WireDataModel
+
+public struct UserUpdateInfo: Sendable {


This is used to map API object UpdateUserEvent to domain model, probably these two models are quite similar and could be merged at some point or maybe it's better to have two separated models

jullianm · 2024-11-08T09:49:18Z

WireDomain/Sources/WireDomain/Repositories/User/UserLocalStore.swift

-        from remoteClient: WireAPI.SelfUserClient,
-        isNewClient: Bool
-    ) async throws
-


dead code that was factored out a few weeks ago to the UserClients component

jullianm · 2024-11-08T09:49:58Z

WireDomain/Sources/WireDomain/Repositories/User/UserLocalStore.swift


            user.isPendingMetadataRefresh = false
        }
    }

-    // swiftlint:disable:next todo_requires_jira_link
-    // TODO: refactor, do not pass API object (WireAPI.UserClient) directly
-    public func updateUserClient(


same dead code that was factored out

jullianm · 2024-11-08T09:50:27Z

WireDomain/Sources/WireDomain/Repositories/UserClients/UserClientsLocalStore.swift

+import WireDataModel
+
+// sourcery: AutoMockable
+public protocol UserClientsLocalStoreProtocol {


This local store was created and storage operations moved out from related repo

github-actions · 2024-11-08T10:00:39Z

Test Results

165 tests 165 ✅ 7s ⏱️
47 suites 0 💤
1 files 0 ❌

Results for commit 3d9ee9d.

♻️ This comment has been updated with latest results.

jullianm · 2024-11-08T10:05:04Z

WireDomain/Tests/WireDomainTests/LocalStores/ConversationLabelsLocalStoreTests.swift

+        )
+
+        static let conversationLabel2 = ConversationLabelInfo(
+            id: .mockID4,


across tests we now use UUID helpers from WireTestingPackage

jullianm · 2024-11-08T10:06:22Z

WireDomain/Tests/WireDomainTests/LocalStores/FeatureConfigLocalStoreTests.swift

+    }
+
+    override func setUp() async throws {
+        coreDataStackHelper = CoreDataStackHelper()


removing super.setUp() and super.tearDown() across tests

jullianm · 2024-11-08T10:06:51Z

WireDomain/Tests/WireDomainTests/LocalStores/FeatureConfigLocalStoreTests.swift

+        XCTAssertEqual(result, true)
+    }
+
+    func testFetchFeature_It_Retrieves_Feature_With_Correct_Config() async throws {


harmonizing all tests naming format

jullianm · 2024-11-08T10:08:00Z

WireDomain/Tests/WireDomainTests/LocalStores/TeamLocalStoreTests.swift

+import XCTest
+import WireTestingPackage
+
+final class TeamLocalStoreTests: XCTestCase {


basically all the local store tests is code that was moved out from repositories (with some adjustments sometimes)

jullianm · 2024-11-08T10:11:25Z

WireDomain/Sources/WireDomain/Repositories/Conversations/ConversationLocalStore.swift

@@ -16,7 +16,6 @@
 // along with this program. If not, see http://www.gnu.org/licenses/.
 //

-import CoreData
 import WireAPI


This local store still uses a WireAPI.Conversation object I didn't do it in this PR because we still have conversation events to tackle but eventually it will be mapped to a domain model as well.

We also have a SystemMessage model and message creation related methods in this repo this will be removed (in another PR) now that we have a Message dedicated component (Repository and local store)

jullianm · 2024-11-08T10:48:12Z

WireDomain/Tests/WireDomainTests/LocalStores/UpdateEventsLocalStoreTests.swift

+import WireTestingPackage
+import XCTest
+
+final class UpdateEventsLocalStoreTests: XCTestCase {


@johnxnguyen could you please check this one particularly, I've split up tests in both repo and local stores and I want to make sure I didn't break your tests. thanks!

jullianm added 13 commits November 6, 2024 17:10

Conversation - add UTs for local store and repository

204fd96

User - add UTs for local store and repository

07e8c01

Connection - add UTs for local store and repository

ee3927c

Team - add UTs for local store and repository

3a3234b

UserClients - add UTs for local store and repo

8d72857

FeatureConfig - add UTS for local store and repo

30796bf

UpdateEvents - add UTs for local store and repository

c47a3e2

add missing documentation for FeatureConfigLocalStore

6e80d78

ConversationLabels - add UTs for local store and repository

0a90373

add TeamModelMappings to avoid passing API object to storage layer

a185f03

Merge branch 'develop' into refactor/create-local-stores-and-UTs

205b4d2

move message local store file to dedicated folder

8aebf87

remove dead code, clean up import, add domain models to avoid API obj…

aa8da54

…ects in storage layer

echoes-hq bot added echoes: technical-roadmap Work contributing to the Technical Roadmap, to improve our velocity or reduce the technical debt. echoes/initiative: ios-sync-refactoring labels Nov 8, 2024

github-actions bot assigned jullianm Nov 8, 2024

jullianm commented Nov 8, 2024

View reviewed changes

created dedicated file for UserClientInfo domain model

36e1e98

create UserClientsModelMappings to map API model to domain model

3d9ee9d

jullianm commented Nov 8, 2024

View reviewed changes

jullianm requested review from johnxnguyen, netbe and caldrian November 8, 2024 10:20

jullianm commented Nov 8, 2024

View reviewed changes

jullianm mentioned this pull request Nov 18, 2024

refactor: process conversation proteus/MLS message add event - WPB-10174 #2164

Open

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: create local stores and UTS - WPB-12100 #2141

refactor: create local stores and UTS - WPB-12100 #2141

jullianm commented Nov 8, 2024 •

edited

Loading

jullianm Nov 8, 2024 •

edited

Loading

netbe Nov 12, 2024

jullianm Nov 8, 2024

jullianm Nov 8, 2024

jullianm Nov 8, 2024

jullianm Nov 8, 2024 •

edited

Loading

jullianm Nov 8, 2024

jullianm Nov 8, 2024

jullianm Nov 8, 2024

jullianm Nov 8, 2024

jullianm Nov 8, 2024

jullianm Nov 8, 2024

jullianm Nov 8, 2024

jullianm Nov 8, 2024

jullianm Nov 8, 2024

github-actions bot commented Nov 8, 2024 •

edited

Loading

jullianm Nov 8, 2024 •

edited

Loading

jullianm Nov 8, 2024

jullianm Nov 8, 2024

jullianm Nov 8, 2024

jullianm Nov 8, 2024 •

edited

Loading

jullianm Nov 8, 2024


		}

		final class FeatureConfigLocalStore: FeatureConfigLocalStoreProtocol {


		import WireDataModel

		public struct UserUpdateInfo: Sendable {

refactor: create local stores and UTS - WPB-12100 #2141

Are you sure you want to change the base?

refactor: create local stores and UTS - WPB-12100 #2141

Conversation

jullianm commented Nov 8, 2024 • edited Loading

Key points

Testing

Checklist

UI accessibility checklist

jullianm Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jullianm Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 8, 2024 • edited Loading

Test Results

jullianm Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jullianm Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jullianm commented Nov 8, 2024 •

edited

Loading

jullianm Nov 8, 2024 •

edited

Loading

jullianm Nov 8, 2024 •

edited

Loading

github-actions bot commented Nov 8, 2024 •

edited

Loading

jullianm Nov 8, 2024 •

edited

Loading

jullianm Nov 8, 2024 •

edited

Loading