Fetch basic user metadata from SQLite if available #925

bfollington · 2023-10-05T00:23:37Z

Fixes #924

This turned into quite a meaty refactor. The goal is to avoid hitting the network to produce the feed or our local profile view. To accomplish this we had to revisit our entire approach to loading and caching user profiles. While a lot of code changed, the new approach is much simpler: we have two code paths, one for loading user data for list views and another for the profile detail view.

I've tested loading the feed with several hundred posts in it from various peers and it loads instantly for me without reading any of the peer's _profile_ memos.

Changes

Introduce UserProfileService.identifyUser which consults the DB to produce a UserProfile and never traverses (intended for list views which need user metadata)
- If we don't follow this user or their profile is missing from the DB we fall back to a "shallow" profile display
Remove UserProfileService.buildUserProfile
Roll buildUserProfile into UserProfileService.loadFullProfileData which always fetches the latest profile from noosphere (intended for the profile detail view)
- We do not fetch profiles for any users in the following list unless we follow them and have their profile indexed in the DB
Consolidate and fix UserProfile.aliases logic
EntryStub now comes with a did field

TODO

explicitly read from memo for the actual users profile, fall back to DB
when listing users if we follow them, check the DB for their profile, fallback to nothing
stop passing user profiles around everywhere
do a join to list the feed?
show AKA blocks on followed users

Uses the same code path as our peers. We were loading the profile for every post we displayed in the feed, this cuts it down to one profile read per sphere revision.

xcode/Subconscious/Shared/Services/UserProfileService.swift

Drastically reworking how profiles are fetched and calculated

bfollington · 2023-10-10T03:11:28Z

xcode/Subconscious/Shared/Components/Common/Byline/PetnameView.swift

+            return NameVariant.known(Slashlink.ourProfile, selfNickname)
        case (_, .following(let petname), _, _):
-            return NameVariant.petname(self.address, petname)
+            return NameVariant.known(self.address, petname)
        case (_, .notFollowing, .some(let selfNickname), _):
-            return NameVariant.selfNickname(self.address, selfNickname)
+            return NameVariant.unknown(self.address, selfNickname)
        case (_, .notFollowing, _, .some(let proposedName)):
-            return NameVariant.proposedName(self.address, proposedName)
+            return NameVariant.unknown(self.address, proposedName)


Simplified the modelling for name display, we either know a user or we don't.

If we know them, we use our name for them falling back to their nickname for themselves. If we don't know them, we use their nickname (if available) or fall back to the leaf of their current address.

I like the clarity, however, I want to note that this loses us information.

It seems likely at some point we will want to be able to distinguish proposed names from self-proposed names (https://github.com/cwebber/rebooting-the-web-of-trust-spring2018/blob/petnames/draft-documents/petnames.md). For example,

We may want to prefer a self-proposed name to a 3p-proposed name when following and suggesting a name.

(Speculative) We may wish, in future, to show the user's self-proposed name in context menus or place a badge next to self-proposed names to show user's preferred name.

It doesn't actually lose us any information in the model itself, just in the mapping to an (under utilized) view construct. We were rendering proposedName and selfNickname identically before this change so nothing visibly changes as a result of this.

If the kind of cases you point out do arise, we can derive all that we need from a UserProfile regardless.

xcode/Subconscious/Shared/Components/Detail/DetailStackView.swift

xcode/Subconscious/Shared/Components/Detail/MemoViewerDetailView.swift

xcode/Subconscious/Shared/Models/Slashlink.swift

xcode/Subconscious/Shared/Models/EntryStub.swift

xcode/Subconscious/Shared/Components/Detail/UserProfileDetailView.swift

xcode/Subconscious/Shared/Services/DataService.swift

xcode/Subconscious/Shared/Components/Feed.swift

xcode/Subconscious/Shared/Services/DatabaseService.swift

bfollington · 2023-10-10T04:31:43Z

xcode/Subconscious/Shared/Services/UserProfileService.swift

+struct UserProfileCacheKey: Hashable {
+    let did: Did
+    let petname: Petname?
+}


The same user with the same Did can have different UserProfiles depending on the context you encounter them.

A trivial example: following the same Did under two Petnames changes which name is chosen as the "primary" name, depending on which version you're looking at.

Worth saying: I'm open to the idea of dropping the in-memory cache. The main drawback is that we'd be sending a query to SQLite for each post in the feed except in the case of our profile which is read from our sphere.

We can keep it for now. My main question would be how is it invalidated?

Also, instead of hitting the DB for each post, couldn't we work this logic into the SQL query that constructs the feed?

Yep! I was considering that in this comment: #925 (comment)

But I realised today that since EntryStub now has the Did attached, I don't need to load a profile at all for the feed. In fact, I managed to drop author from EntryStub entirely (I wasn't a fan of having to introduce it in the first place).

Now, we can produce the feed + backlinks + transcludes without having to fetch the user's profile in the process. If we introduce something like customisable PFPs in the future we will have to revisit this but I would argue that that's a whole seperate domain & merits a different approach.

As a result, there's really no benefit coming from the in-memory cache so... I removed it. One less invalidation layer to care about.

xcode/Subconscious/Shared/Services/DataService.swift

xcode/Subconscious/Shared/Services/UserProfileService.swift

gordonbrander

On the whole, looks great. Requests for some tests given this touches DB queries, which is error prone.

gordonbrander · 2023-10-10T18:10:05Z

xcode/Subconscious/Shared/Components/Common/Byline/PetnameView.swift

+            return NameVariant.known(Slashlink.ourProfile, selfNickname)
        case (_, .following(let petname), _, _):
-            return NameVariant.petname(self.address, petname)
+            return NameVariant.known(self.address, petname)
        case (_, .notFollowing, .some(let selfNickname), _):
-            return NameVariant.selfNickname(self.address, selfNickname)
+            return NameVariant.unknown(self.address, selfNickname)
        case (_, .notFollowing, _, .some(let proposedName)):
-            return NameVariant.proposedName(self.address, proposedName)
+            return NameVariant.unknown(self.address, proposedName)


I like the clarity, however, I want to note that this loses us information.

It seems likely at some point we will want to be able to distinguish proposed names from self-proposed names (https://github.com/cwebber/rebooting-the-web-of-trust-spring2018/blob/petnames/draft-documents/petnames.md). For example,

We may want to prefer a self-proposed name to a 3p-proposed name when following and suggesting a name.

(Speculative) We may wish, in future, to show the user's self-proposed name in context menus or place a badge next to self-proposed names to show user's preferred name.

xcode/Subconscious/Shared/Components/Common/Profile/UserProfileView.swift

xcode/Subconscious/Shared/Components/Common/Story/StoryUserView.swift

xcode/Subconscious/Shared/Components/Detail/DetailStackView.swift

xcode/Subconscious/Shared/Components/Detail/UserProfileDetailView.swift

xcode/Subconscious/Shared/Services/DatabaseService.swift

gordonbrander · 2023-10-10T18:50:59Z

xcode/Subconscious/Shared/Services/UserProfileService.swift

+struct UserProfileCacheKey: Hashable {
+    let did: Did
+    let petname: Petname?
+}


We can keep it for now. My main question would be how is it invalidated?

gordonbrander · 2023-10-10T18:51:55Z

xcode/Subconscious/Shared/Services/UserProfileService.swift

+struct UserProfileCacheKey: Hashable {
+    let did: Did
+    let petname: Petname?
+}


Also, instead of hitting the DB for each post, couldn't we work this logic into the SQL query that constructs the feed?

gordonbrander · 2023-10-10T18:54:07Z

xcode/Subconscious/Shared/Services/UserProfileService.swift

+    /// we have encountered.
+    ///
+    /// `context` will be used to determine the preferred navigation address for a user.
+    func identifyUser(


There's a lot going on in this function. Probably deserves a test.

I had thought this would be too difficult to test without traversal, but after some thinking I came up with some reasonable test cases that cover the logic within this function itself.

See Tests_UserProfileService.testIdentifyUserCallSignatures and Tests_UserProfileService.testIdentifyUserRebaseOnContext

Now that we have the DID we actually don't need to attach an author to an EntryStub _at all_. In the future, if we have customisable PFPs for example, we may need to load author information but I would argue that's better as background metadata we fetch separately from the notes themselves to maximise resilience.

Also, fix the broken query

Also expand test assertions

bfollington · 2023-10-11T02:34:45Z

xcode/Subconscious/Shared/Components/Common/Story/StoryEntryView.swift

-                                peer: story.author.address.peer,
-                                slug: story.entry.address.slug
-                            )
+                            pfp: .generated(story.entry.did),


This is all we were using author for, which can be derived from just the DID.

bfollington · 2023-10-11T02:36:52Z

xcode/Subconscious/Shared/Services/UserProfileService.swift

    /// Attempt to read & deserialize a user `_profile_.json` at the given address.
    /// Because profile data is optional and we expect it will not always be present
    /// any errors are logged & handled and nil will be returned if reading fails.
-    private func readProfileMemo(
+    func readProfileMemo(


Public so we can access it in tests.

bfollington · 2023-10-11T02:38:31Z

xcode/Subconscious/SubconsciousTests/TestUtilities.swift

@@ -56,6 +56,7 @@ struct TestUtilities {
            path: "database.sqlite",
            directoryHint: .notDirectory
        )
+        print("Test SQLite Path: \(databaseURL.path(percentEncoded: false))")


Helpful for debugging tests.

gordonbrander

LGTM. Feel free to land.

Have you play tested? It's late my time, but will playtest myself in the AM.

gordonbrander · 2023-10-11T16:02:46Z

xcode/Subconscious/Shared/Services/DatabaseService.swift

                    .toString()?
-                    .toLink()?


It's subtle, but

A Link is always a Did plus Slug

A slashlink may be a Did plus Slug, a Petname plus Slug, or a Slug.

A Link is a stable identifier that can be used for indexing

A Slashlink is a locator which may be used to resolve a piece of content. It is not a stable identifier.

More background thinking in this commit message: f76a564

The Link is really only used for the database layer to provide an unambiguous identifier.

We store a serialized Link in the id field of the database (not a Slashlink). So we read it back out as a link and then convert it to a slashlink.

bfollington added 4 commits October 5, 2023 10:23

Cache our profile data

886449f

Uses the same code path as our peers. We were loading the profile for every post we displayed in the feed, this cuts it down to one profile read per sphere revision.

Improve time to first render & resilience of profile

fe865fa

Model loading status explicitly for feed

fc541b9

Drop .resolutionStatus from UserProfile

d00bfcc

bfollington marked this pull request as draft October 5, 2023 03:41

Load profiles from DB

e419a30

bfollington commented Oct 6, 2023

View reviewed changes

xcode/Subconscious/Shared/Services/UserProfileService.swift Outdated Show resolved Hide resolved

bfollington added 3 commits October 10, 2023 12:08

Guarantee we always know the DID of a user

2dbe11a

Drastically reworking how profiles are fetched and calculated

Fix navigation

36b50f2

Fix duplicate follows

6c9b1eb

bfollington commented Oct 10, 2023

View reviewed changes

xcode/Subconscious/Shared/Components/Detail/DetailStackView.swift Show resolved Hide resolved

bfollington commented Oct 10, 2023

View reviewed changes

xcode/Subconscious/Shared/Components/Detail/DetailStackView.swift Outdated Show resolved Hide resolved

bfollington commented Oct 10, 2023

View reviewed changes

xcode/Subconscious/Shared/Components/Detail/DetailStackView.swift Show resolved Hide resolved

bfollington commented Oct 10, 2023

View reviewed changes

xcode/Subconscious/Shared/Components/Detail/MemoViewerDetailView.swift Show resolved Hide resolved

Remove comments

77a8f01

bfollington commented Oct 10, 2023

View reviewed changes

xcode/Subconscious/Shared/Models/Slashlink.swift Show resolved Hide resolved

bfollington commented Oct 10, 2023

View reviewed changes

xcode/Subconscious/Shared/Models/EntryStub.swift Show resolved Hide resolved

bfollington commented Oct 10, 2023

View reviewed changes

xcode/Subconscious/Shared/Components/Detail/UserProfileDetailView.swift Show resolved Hide resolved

bfollington commented Oct 10, 2023

View reviewed changes

xcode/Subconscious/Shared/Services/DataService.swift Outdated Show resolved Hide resolved

bfollington added 4 commits October 10, 2023 13:57

Show alias for our profile

40dee23

Add tests for alias calculation

326f8f8

Restore dropped comment

b3ec16b

Drop unused action

537f8cd

bfollington commented Oct 10, 2023

View reviewed changes

xcode/Subconscious/Shared/Components/Feed.swift Show resolved Hide resolved

bfollington added 2 commits October 10, 2023 14:25

Formatting

0806460

Avoid hardcoding profile slug

8830916

bfollington commented Oct 10, 2023

View reviewed changes

xcode/Subconscious/Shared/Services/DatabaseService.swift Show resolved Hide resolved

bfollington commented Oct 10, 2023

View reviewed changes

Formatting and cleanup

22e3cc5

bfollington marked this pull request as ready for review October 10, 2023 04:40

bfollington requested a review from gordonbrander October 10, 2023 04:46

bfollington commented Oct 10, 2023

View reviewed changes

xcode/Subconscious/Shared/Services/DataService.swift Outdated Show resolved Hide resolved

bfollington changed the title ~~Cache our profile data~~ Fetch basic user metadata from SQLite if available Oct 10, 2023

bfollington commented Oct 10, 2023

View reviewed changes

xcode/Subconscious/Shared/Services/UserProfileService.swift Outdated Show resolved Hide resolved

gordonbrander suggested changes Oct 10, 2023

View reviewed changes

bfollington added 4 commits October 11, 2023 11:08

Drop in-memory cache

2cfea03

Assert DID is correct in listRecent

5e2d734

Add test for readRandomEntryInDateRange

3b7cf40

Also, fix the broken query

bfollington force-pushed the 2023-10-05-cache-our-profile branch from 3080e5d to 3b7cf40 Compare October 11, 2023 01:09

bfollington added 5 commits October 11, 2023 11:27

Restore .toLink()? calls

9c7491f

Introduce testReadRandomEntryMatching

5c38fc5

Test readUserProfile

541d13b

Test identifyUser (many ways)

80cbd87

Constrain contract on identifyUser to take Petname? instead of Peer?

938703f

Also expand test assertions

bfollington requested a review from gordonbrander October 11, 2023 02:31

bfollington added 2 commits October 11, 2023 12:33

Rename tests

8a4f5fe

Remove writeTestData

55bcf5c

bfollington commented Oct 11, 2023

View reviewed changes

Remove comment

12a68bf

bfollington commented Oct 11, 2023

View reviewed changes

gordonbrander approved these changes Oct 11, 2023

View reviewed changes

bfollington merged commit b4b8435 into main Oct 12, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fetch basic user metadata from SQLite if available #925

Fetch basic user metadata from SQLite if available #925

bfollington commented Oct 5, 2023 •

edited

Loading

bfollington Oct 10, 2023 •

edited

Loading

gordonbrander Oct 10, 2023

bfollington Oct 11, 2023 •

edited

Loading

bfollington Oct 10, 2023

bfollington Oct 10, 2023

gordonbrander Oct 10, 2023

gordonbrander Oct 10, 2023

bfollington Oct 11, 2023 •

edited

Loading

gordonbrander left a comment

gordonbrander Oct 10, 2023

gordonbrander Oct 10, 2023

gordonbrander Oct 10, 2023

gordonbrander Oct 10, 2023

bfollington Oct 11, 2023 •

edited

Loading

bfollington Oct 11, 2023

bfollington Oct 11, 2023

bfollington Oct 11, 2023

gordonbrander left a comment

gordonbrander Oct 11, 2023

Fetch basic user metadata from SQLite if available #925

Fetch basic user metadata from SQLite if available #925

Conversation

bfollington commented Oct 5, 2023 • edited Loading

Changes

TODO

bfollington Oct 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bfollington Oct 11, 2023 • 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

bfollington Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

gordonbrander left a 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

bfollington Oct 11, 2023 • 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

gordonbrander left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bfollington commented Oct 5, 2023 •

edited

Loading

bfollington Oct 10, 2023 •

edited

Loading

bfollington Oct 11, 2023 •

edited

Loading

bfollington Oct 11, 2023 •

edited

Loading

bfollington Oct 11, 2023 •

edited

Loading