Merge pull request 'Retry unresolved profiles' (#104) from feature/retry_unresolved_profiles into master

Reviewed-on: #104

app/routes/activity.js

@@ -8,6 +8,10 @@ export default class ActivityRoute extends Route {
 
   activate() {
     this.mapUi.showSidebar();
+    // Re-request profiles for senders that couldn't be resolved when the feed
+    // was first loaded (e.g. metadata published since then). No-op on the first
+    // entry, when there are no items yet.
+    void this.activity.retryUnresolvedProfiles();
   }
 
   setupController(controller, model) {

app/services/activity.js

@@ -119,6 +119,27 @@ export default class ActivityService extends Service {
       this.isLoading = false;
       this._cacheReadyCallback = null;
     }
+
+    void this.retryUnresolvedProfiles();
+  }
+
+  /**
+   * Re-requests kind 0 metadata for senders whose profiles could not be
+   * resolved. Called when the activity view is re-entered or the source mode
+   * changes, so a profile published after the feed was first loaded can still
+   * show up in the same session.
+   *
+   * @returns {Promise<void>}
+   */
+  async retryUnresolvedProfiles() {
+    const pubkeys = new Set();
+    for (const item of this.items) {
+      if (item.senderPubkey && item.senderProfileLoading) {
+        pubkeys.add(item.senderPubkey);
+      }
+    }
+    if (pubkeys.size === 0) return;
+    await this.nostrData.refreshProfiles([...pubkeys]);
   }
 
   /**

app/services/nostr-data.js

@@ -95,6 +95,10 @@ export default class NostrDataService extends Service {
   _contributionsSub = null;
   _deletionsSub = null;
   _profileModelSubs = new Map();
+  // Pubkeys whose profiles have already been requested at least once. Used to
+  // distinguish a first lookup (handled by the ProfileModel loader) from a
+  // retry on place re-entry, so we don't duplicate the initial request.
+  _attemptedProfilePubkeys = new Set();
 
   _zapReceiptsSub = null;
   _zapReceiptsNetworkSub = null;
@@ -922,6 +926,8 @@ export default class NostrDataService extends Service {
       (pk) => pk && !this._profileModelSubs.has(pk)
     );
 
+    const retryPubkeys = [];
+
     for (const pubkey of newPubkeys) {
       const sub = this.store
         .model(ProfileModel, pubkey)
@@ -929,7 +935,60 @@ export default class NostrDataService extends Service {
           this.profiles = { ...this.profiles, [pubkey]: profileContent };
         });
       this._profileModelSubs.set(pubkey, sub);
+
+      // If we already tried this pubkey before (e.g. selecting the same place
+      // again) and it still isn't in the store, explicitly re-request it. The
+      // ProfileModel loader only runs once per cached model, so without this a
+      // profile published after the first lookup would never be retried.
+      if (
+        this._attemptedProfilePubkeys.has(pubkey) &&
+        !this.store.getReplaceable(0, pubkey)
+      ) {
+        retryPubkeys.push(pubkey);
+      }
+      this._attemptedProfilePubkeys.add(pubkey);
     }
+
+    if (retryPubkeys.length > 0) {
+      void this.refreshProfiles(retryPubkeys);
+    }
+  }
+
+  /**
+   * Re-requests kind 0 metadata for the given pubkeys from the directory and
+   * active read relays, adding any results to the event store.
+   *
+   * Used to retry profiles that were not available on a first lookup (e.g. the
+   * author published their metadata after their content was loaded, or a
+   * cached applesauce model prevented the fallback loader from running again).
+   * Inserting the events into the store notifies any active `ProfileModel`
+   * subscriptions, so callers don't need to manage subscriptions themselves.
+   *
+   * @param {string[]} pubkeys Pubkeys to refresh
+   * @returns {Promise<void>}
+   */
+  async refreshProfiles(pubkeys) {
+    const unique = [...new Set(pubkeys)].filter(Boolean);
+    if (unique.length === 0) return;
+
+    const relays = uniqNormalizedRelays([
+      ...DIRECTORY_RELAYS,
+      ...this.activeReadRelays,
+    ]);
+    const filters = this._batchAuthorFilters(unique, [0]);
+
+    await new Promise((resolve) => {
+      this.nostrRelay.pool
+        .request(relays, filters)
+        .pipe(timeout({ first: 15_000 }))
+        .subscribe({
+          next: (event) => {
+            this.store.add(event);
+          },
+          error: () => resolve(),
+          complete: () => resolve(),
+        });
+    });
   }
 
   getProfile(pubkey) {
@@ -1257,6 +1316,7 @@ export default class NostrDataService extends Service {
     super.willDestroy(...arguments);
     this._cleanupSubscriptions();
     this._clearProfileSubs();
+    this._attemptedProfilePubkeys.clear();
 
     if (this._deletionsSub) {
       this._deletionsSub.unsubscribe();

tests/acceptance/activity-test.js

@@ -31,6 +31,8 @@ class MockActivityService extends Service {
 
   async setSourceMode() {}
 
+  async retryUnresolvedProfiles() {}
+
   loadMore() {}
 
   stop() {

tests/helpers/mock-nostr.js

@@ -62,6 +62,10 @@ export class MockNostrDataService extends Service {
     return this.profiles[pubkey];
   }
 
+  refreshProfiles() {
+    return Promise.resolve();
+  }
+
   get activeReadRelays() {
     return [];
   }

tests/unit/services/activity-test.js

@@ -87,6 +87,12 @@ function makePhotoEvent(opts = {}) {
 class MockNostrDataService extends Service {
   @tracked profiles = {};
   _contactPubkeys = new Set();
+  refreshProfilesCalls = [];
+
+  refreshProfiles(pubkeys) {
+    this.refreshProfilesCalls.push(pubkeys);
+    return Promise.resolve();
+  }
 
   store = {
     events: new Map(),
@@ -301,6 +307,115 @@ module('Unit | Service | activity', function (hooks) {
     assert.false(entry.senderProfileLoading);
   });
 
+  test('retryUnresolvedProfiles refreshes only unresolved sender pubkeys', async function (assert) {
+    const service = this.owner.lookup('service:activity');
+    service._userPubkey = USER_PUBKEY;
+    service._sourceMode = 'explore';
+    service.nostrData._contactPubkeys = new Set();
+    service.nostrData.isTrustedEvent = () => true;
+
+    const OTHER_PUBKEY = 'c'.repeat(64);
+    service.nostrData.profiles[SENDER_PUBKEY] = { name: 'Alice' };
+
+    const resolvedPhoto = makePhotoEvent({
+      id: 'rp'.padEnd(64, '0'),
+      author: SENDER_PUBKEY,
+      placeIdentifier: 'osm:node:100',
+      created_at: 1000,
+    });
+    const unresolvedPhoto = makePhotoEvent({
+      id: 'up'.padEnd(64, '0'),
+      author: OTHER_PUBKEY,
+      placeIdentifier: 'osm:node:200',
+      created_at: 2000,
+    });
+    service._updateSocialItems([resolvedPhoto, unresolvedPhoto]);
+
+    assert.strictEqual(service.items.length, 2, 'both entries present');
+    assert.false(
+      service.items.find((i) => i.senderPubkey === SENDER_PUBKEY)
+        .senderProfileLoading,
+      'resolved sender not loading'
+    );
+    assert.true(
+      service.items.find((i) => i.senderPubkey === OTHER_PUBKEY)
+        .senderProfileLoading,
+      'unresolved sender still loading'
+    );
+
+    await service.retryUnresolvedProfiles();
+
+    assert.strictEqual(
+      service.nostrData.refreshProfilesCalls.length,
+      1,
+      'refresh called once'
+    );
+    assert.deepEqual(
+      service.nostrData.refreshProfilesCalls[0],
+      [OTHER_PUBKEY],
+      'only the unresolved sender is refreshed'
+    );
+  });
+
+  test('retryUnresolvedProfiles does nothing when all senders are resolved', async function (assert) {
+    const service = this.owner.lookup('service:activity');
+    service._userPubkey = USER_PUBKEY;
+    service._sourceMode = 'explore';
+    service.nostrData._contactPubkeys = new Set();
+    service.nostrData.isTrustedEvent = () => true;
+    service.nostrData.profiles[SENDER_PUBKEY] = { name: 'Alice' };
+
+    service._updateSocialItems([
+      makePhotoEvent({
+        id: 'rp2'.padEnd(64, '0'),
+        author: SENDER_PUBKEY,
+        placeIdentifier: 'osm:node:100',
+        created_at: 1000,
+      }),
+    ]);
+
+    await service.retryUnresolvedProfiles();
+
+    assert.strictEqual(
+      service.nostrData.refreshProfilesCalls.length,
+      0,
+      'no refresh when every sender is resolved'
+    );
+  });
+
+  test('setSourceMode retries unresolved profiles after fetching', async function (assert) {
+    const service = this.owner.lookup('service:activity');
+    service._userPubkey = USER_PUBKEY;
+    service.nostrData._contactPubkeys = new Set();
+    service.nostrData.isTrustedEvent = () => true;
+
+    const photo = makePhotoEvent({
+      id: 'sm1'.padEnd(64, '0'),
+      author: SENDER_PUBKEY,
+      placeIdentifier: 'osm:node:100',
+      created_at: 1000,
+    });
+
+    service.nostrData.fetchActivityPhotos = async (_since, _until, mode) => {
+      if (mode === 'explore')
+        return { cacheEvents: [photo], networkEvents: Promise.resolve([]) };
+      return { cacheEvents: [], networkEvents: Promise.resolve([]) };
+    };
+
+    await service.setSourceMode('explore');
+
+    assert.strictEqual(
+      service.nostrData.refreshProfilesCalls.length,
+      1,
+      'refresh triggered by mode switch'
+    );
+    assert.deepEqual(
+      service.nostrData.refreshProfilesCalls[0],
+      [SENDER_PUBKEY],
+      'refreshes the unresolved sender'
+    );
+  });
+
   test('_updateSocialItems groups photos by author + place', function (assert) {
     const service = this.owner.lookup('service:activity');
     service._userPubkey = USER_PUBKEY;

tests/unit/services/nostr-data-test.js

@@ -1287,3 +1287,87 @@ module('Unit | Service | nostr-data | zap receipts', function (hooks) {
     );
   });
 });
+
+module('Unit | Service | nostr-data | profiles', function (hooks) {
+  setupNostrDataService(hooks);
+
+  function makeProfileEvent(pubkey, content, opts = {}) {
+    return {
+      id: opts.id || makeEventId(3000),
+      pubkey,
+      kind: 0,
+      created_at: opts.created_at || 1000,
+      tags: [],
+      content: JSON.stringify(content),
+      sig: 'sig',
+    };
+  }
+
+  test('refreshProfiles requests kind 0 for the given pubkeys', async function (assert) {
+    const service = this.owner.lookup('service:nostr-data');
+    const pubkey = makePubkey(80);
+
+    await service.refreshProfiles([pubkey]);
+
+    const filter = this.requestedFilters.find((f) => f.kinds?.includes(0));
+    assert.ok(filter, 'requested kind 0');
+    assert.deepEqual(filter.authors, [pubkey], 'requested the pubkey');
+  });
+
+  test('refreshProfiles adds returned profiles to the store', async function (assert) {
+    const service = this.owner.lookup('service:nostr-data');
+    const pubkey = makePubkey(81);
+    const event = makeProfileEvent(pubkey, { name: 'Alice' });
+
+    service.nostrRelay.pool.request = (_relays, filters) => {
+      this.requestedFilters.push(...filters);
+      return of(event);
+    };
+
+    await service.refreshProfiles([pubkey]);
+
+    assert.true(service.store.hasReplaceable(0, pubkey), 'profile stored');
+  });
+
+  test('refreshProfiles deduplicates input and skips empty input', async function (assert) {
+    const service = this.owner.lookup('service:nostr-data');
+    const pubkey = makePubkey(82);
+
+    await service.refreshProfiles([pubkey, pubkey, null]);
+    await service.refreshProfiles([]);
+
+    const filters = this.requestedFilters.filter((f) => f.kinds?.includes(0));
+    assert.strictEqual(filters.length, 1, 'one request for deduped input');
+    assert.deepEqual(filters[0].authors, [pubkey], 'deduped authors');
+  });
+
+  test('loadProfiles re-requests an unresolved profile on a second call', async function (assert) {
+    const service = this.owner.lookup('service:nostr-data');
+    const pubkey = makePubkey(83);
+
+    // First call: the ProfileModel loader handles it, no explicit refresh.
+    service.loadProfiles([pubkey]);
+    assert.strictEqual(
+      this.requestedFilters.filter((f) => f.kinds?.includes(0)).length,
+      0,
+      'first lookup is not refreshed explicitly'
+    );
+
+    // Re-entering the place clears the model subscriptions, but the ProfileModel
+    // stays cached and its loader won't run again. The second call should retry.
+    service._clearProfileSubs();
+    service.loadProfiles([pubkey]);
+
+    const filters = this.requestedFilters.filter((f) => f.kinds?.includes(0));
+    assert.strictEqual(
+      filters.length,
+      1,
+      'second lookup refreshes the profile'
+    );
+    assert.deepEqual(
+      filters[0].authors,
+      [pubkey],
+      'refreshes the missing pubkey'
+    );
+  });
+});