From 7e5f58b969bea6ed0c136d8fad63c58d948fca51 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Sun, 3 Mar 2019 13:24:55 -0800 Subject: [PATCH] refactor: use timeline item summaries instead of ids (#1072) As described in https://github.com/nolanlawson/pinafore/issues/369#issuecomment-467211908 this is the first step toward fixing #369 --- .../_actions/addStatusOrNotification.js | 56 +++++++++++------- src/routes/_actions/deleteStatuses.js | 11 ++-- src/routes/_actions/timeline.js | 58 ++++++++++--------- src/routes/_components/timeline/Timeline.html | 24 ++++---- .../computations/timelineComputations.js | 21 +++---- src/routes/_store/mixins/timelineMixins.js | 2 +- .../_store/observers/instanceObservers.js | 13 +++-- .../_store/observers/timelineObservers.js | 7 ++- src/routes/_utils/arrays.js | 10 ++-- src/routes/_utils/getIdFromItemSummaries.js | 11 ++++ src/routes/_utils/sorting.js | 8 +-- src/routes/_utils/timelineItemToSummary.js | 8 +++ 12 files changed, 134 insertions(+), 95 deletions(-) create mode 100644 src/routes/_utils/getIdFromItemSummaries.js create mode 100644 src/routes/_utils/timelineItemToSummary.js diff --git a/src/routes/_actions/addStatusOrNotification.js b/src/routes/_actions/addStatusOrNotification.js index 9138854..fb81b31 100644 --- a/src/routes/_actions/addStatusOrNotification.js +++ b/src/routes/_actions/addStatusOrNotification.js @@ -1,15 +1,15 @@ import { mark, stop } from '../_utils/marks' import { store } from '../_store/store' import uniqBy from 'lodash-es/uniqBy' -import uniq from 'lodash-es/uniq' import isEqual from 'lodash-es/isEqual' import { database } from '../_database/database' -import { concat } from '../_utils/arrays' +import { concat, indexWhere } from '../_utils/arrays' import { scheduleIdleTask } from '../_utils/scheduleIdleTask' +import { timelineItemToSummary } from '../_utils/timelineItemToSummary' function getExistingItemIdsSet (instanceName, timelineName) { - let timelineItemIds = store.getForTimeline(instanceName, timelineName, 'timelineItemIds') || [] - return new Set(timelineItemIds) + let timelineItemSummaries = store.getForTimeline(instanceName, timelineName, 'timelineItemSummaries') || [] + return new Set(timelineItemSummaries.map(_ => _.id)) } function removeDuplicates (instanceName, timelineName, updates) { @@ -27,28 +27,37 @@ async function insertUpdatesIntoTimeline (instanceName, timelineName, updates) { await database.insertTimelineItems(instanceName, timelineName, updates) - let itemIdsToAdd = store.getForTimeline(instanceName, timelineName, 'itemIdsToAdd') || [] - let newItemIdsToAdd = uniq(concat(itemIdsToAdd, updates.map(_ => _.id))) - if (!isEqual(itemIdsToAdd, newItemIdsToAdd)) { - console.log('adding ', (newItemIdsToAdd.length - itemIdsToAdd.length), - 'items to itemIdsToAdd for timeline', timelineName) - store.setForTimeline(instanceName, timelineName, { itemIdsToAdd: newItemIdsToAdd }) + let itemSummariesToAdd = store.getForTimeline(instanceName, timelineName, 'timelineItemSummariesToAdd') || [] + console.log('itemSummariesToAdd', JSON.parse(JSON.stringify(itemSummariesToAdd))) + console.log('updates.map(timelineItemToSummary)', JSON.parse(JSON.stringify(updates.map(timelineItemToSummary)))) + console.log('concat(itemSummariesToAdd, updates.map(timelineItemToSummary))', + JSON.parse(JSON.stringify(concat(itemSummariesToAdd, updates.map(timelineItemToSummary))))) + let newItemSummariesToAdd = uniqBy( + concat(itemSummariesToAdd, updates.map(timelineItemToSummary)), + _ => _.id + ) + if (!isEqual(itemSummariesToAdd, newItemSummariesToAdd)) { + console.log('adding ', (newItemSummariesToAdd.length - itemSummariesToAdd.length), + 'items to timelineItemSummariesToAdd for timeline', timelineName) + store.setForTimeline(instanceName, timelineName, { timelineItemSummariesToAdd: newItemSummariesToAdd }) } } -function isValidStatusForThread (thread, timelineName, itemIdsToAdd) { +function isValidStatusForThread (thread, timelineName, itemSummariesToAdd) { + let itemSummariesToAddIdSet = new Set(itemSummariesToAdd.map(_ => _.id)) + let threadIdSet = new Set(thread.map(_ => _.id)) let focusedStatusId = timelineName.split('/')[1] // e.g. "status/123456" - let focusedStatusIdx = thread.indexOf(focusedStatusId) + let focusedStatusIdx = indexWhere(thread, _ => _.id === focusedStatusId) return status => { - let repliedToStatusIdx = thread.indexOf(status.in_reply_to_id) + let repliedToStatusIdx = indexWhere(thread, _ => _.id === status.in_reply_to_id) return ( // A reply to an ancestor status is not valid for this thread, but for the focused status // itself or any of its descendents, it is valid. repliedToStatusIdx >= focusedStatusIdx && // Not a duplicate - !thread.includes(status.id) && + !threadIdSet.has(status.id) && // Not already about to be added - !itemIdsToAdd.includes(status.id) + !itemSummariesToAddIdSet.has(status.id) ) } } @@ -63,16 +72,19 @@ async function insertUpdatesIntoThreads (instanceName, updates) { for (let timelineName of timelineNames) { let thread = threads[timelineName] - let itemIdsToAdd = store.getForTimeline(instanceName, timelineName, 'itemIdsToAdd') || [] - let validUpdates = updates.filter(isValidStatusForThread(thread, timelineName, itemIdsToAdd)) + let itemSummariesToAdd = store.getForTimeline(instanceName, timelineName, 'timelineItemSummariesToAdd') || [] + let validUpdates = updates.filter(isValidStatusForThread(thread, timelineName, itemSummariesToAdd)) if (!validUpdates.length) { continue } - let newItemIdsToAdd = uniq(concat(itemIdsToAdd, validUpdates.map(_ => _.id))) - if (!isEqual(itemIdsToAdd, newItemIdsToAdd)) { - console.log('adding ', (newItemIdsToAdd.length - itemIdsToAdd.length), - 'items to itemIdsToAdd for thread', timelineName) - store.setForTimeline(instanceName, timelineName, { itemIdsToAdd: newItemIdsToAdd }) + let newItemSummariesToAdd = uniqBy( + concat(itemSummariesToAdd, validUpdates.map(timelineItemToSummary)), + _ => _.id + ) + if (!isEqual(itemSummariesToAdd, newItemSummariesToAdd)) { + console.log('adding ', (newItemSummariesToAdd.length - itemSummariesToAdd.length), + 'items to timelineItemSummariesToAdd for thread', timelineName) + store.setForTimeline(instanceName, timelineName, { timelineItemSummariesToAdd: newItemSummariesToAdd }) } } } diff --git a/src/routes/_actions/deleteStatuses.js b/src/routes/_actions/deleteStatuses.js index 2fdcd9b..e8f13d8 100644 --- a/src/routes/_actions/deleteStatuses.js +++ b/src/routes/_actions/deleteStatuses.js @@ -5,20 +5,21 @@ import { database } from '../_database/database' import { scheduleIdleTask } from '../_utils/scheduleIdleTask' function filterItemIdsFromTimelines (instanceName, timelineFilter, idFilter) { - let keys = ['timelineItemIds', 'itemIdsToAdd'] + let keys = ['timelineItemSummaries', 'timelineItemSummariesToAdd'] + let summaryFilter = _ => idFilter(_.id) keys.forEach(key => { let timelineData = store.getAllTimelineData(instanceName, key) Object.keys(timelineData).forEach(timelineName => { - let ids = timelineData[timelineName] + let summaries = timelineData[timelineName] if (!timelineFilter(timelineName)) { return } - let filteredIds = ids.filter(idFilter) - if (!isEqual(ids, filteredIds)) { + let filteredSummaries = summaries.filter(summaryFilter) + if (!isEqual(summaries, filteredSummaries)) { console.log('deleting an item from timelineName', timelineName, 'for key', key) store.setForTimeline(instanceName, timelineName, { - [key]: filteredIds + [key]: filteredSummaries }) } }) diff --git a/src/routes/_actions/timeline.js b/src/routes/_actions/timeline.js index 0e91a4f..cb2867c 100644 --- a/src/routes/_actions/timeline.js +++ b/src/routes/_actions/timeline.js @@ -3,12 +3,13 @@ import { getTimeline } from '../_api/timelines' import { toast } from '../_components/toast/toast' import { mark, stop } from '../_utils/marks' import { concat, mergeArrays } from '../_utils/arrays' -import { byItemIds } from '../_utils/sorting' +import { compareTimelineItemSummaries } from '../_utils/sorting' import isEqual from 'lodash-es/isEqual' import { database } from '../_database/database' import { getStatus, getStatusContext } from '../_api/statuses' import { emit } from '../_utils/eventBus' import { TIMELINE_BATCH_SIZE } from '../_static/timelines' +import { timelineItemToSummary } from '../_utils/timelineItemToSummary' async function storeFreshTimelineItemsInDatabase (instanceName, timelineName, items) { await database.insertTimelineItems(instanceName, timelineName, items) @@ -59,23 +60,23 @@ async function fetchTimelineItems (instanceName, accessToken, timelineName, last async function addTimelineItems (instanceName, timelineName, items, stale) { console.log('addTimelineItems, length:', items.length) - mark('addTimelineItems') - let newIds = items.map(item => item.id) - addTimelineItemIds(instanceName, timelineName, newIds, stale) - stop('addTimelineItems') + mark('addTimelineItemSummaries') + let newSummaries = items.map(timelineItemToSummary) + addTimelineItemSummaries(instanceName, timelineName, newSummaries, stale) + stop('addTimelineItemSummaries') } -export async function addTimelineItemIds (instanceName, timelineName, newIds, newStale) { - let oldIds = store.getForTimeline(instanceName, timelineName, 'timelineItemIds') - let oldStale = store.getForTimeline(instanceName, timelineName, 'timelineItemIdsAreStale') +export async function addTimelineItemSummaries (instanceName, timelineName, newSummaries, newStale) { + let oldSummaries = store.getForTimeline(instanceName, timelineName, 'timelineItemSummaries') || [] + let oldStale = store.getForTimeline(instanceName, timelineName, 'timelineItemSummariesAreStale') - let mergedIds = mergeArrays(oldIds || [], newIds) + let mergedSummaries = mergeArrays(oldSummaries, newSummaries, compareTimelineItemSummaries) - if (!isEqual(oldIds, mergedIds)) { - store.setForTimeline(instanceName, timelineName, { timelineItemIds: mergedIds }) + if (!isEqual(oldSummaries, mergedSummaries)) { + store.setForTimeline(instanceName, timelineName, { timelineItemSummaries: mergedSummaries }) } if (oldStale !== newStale) { - store.setForTimeline(instanceName, timelineName, { timelineItemIdsAreStale: newStale }) + store.setForTimeline(instanceName, timelineName, { timelineItemSummariesAreStale: newStale }) } } @@ -96,17 +97,17 @@ async function fetchTimelineItemsAndPossiblyFallBack () { export async function setupTimeline () { mark('setupTimeline') - // If we don't have any item ids, or if the current item ids are stale + // If we don't have any item summaries, or if the current item summaries are stale // (i.e. via offline mode), then we need to re-fetch // Also do this if it's a thread, because threads change pretty frequently and // we don't have a good way to update them. let { - timelineItemIds, - timelineItemIdsAreStale, + timelineItemSummaries, + timelineItemSummariesAreStale, currentTimeline } = store.get() - if (!timelineItemIds || - timelineItemIdsAreStale || + if (!timelineItemSummaries || + timelineItemSummariesAreStale || currentTimeline.startsWith('status/')) { await fetchTimelineItemsAndPossiblyFallBack() } @@ -123,11 +124,11 @@ export async function fetchTimelineItemsOnScrollToBottom (instanceName, timeline export async function showMoreItemsForTimeline (instanceName, timelineName) { mark('showMoreItemsForTimeline') - let itemIdsToAdd = store.getForTimeline(instanceName, timelineName, 'itemIdsToAdd') - itemIdsToAdd = itemIdsToAdd.sort(byItemIds).reverse() - addTimelineItemIds(instanceName, timelineName, itemIdsToAdd, false) + let itemSummariesToAdd = store.getForTimeline(instanceName, timelineName, 'timelineItemSummariesToAdd') + itemSummariesToAdd = itemSummariesToAdd.sort(compareTimelineItemSummaries).reverse() + addTimelineItemSummaries(instanceName, timelineName, itemSummariesToAdd, false) store.setForTimeline(instanceName, timelineName, { - itemIdsToAdd: [], + timelineItemSummariesToAdd: [], shouldShowHeader: false, showHeader: false }) @@ -144,17 +145,18 @@ export async function showMoreItemsForCurrentTimeline () { export async function showMoreItemsForThread (instanceName, timelineName) { mark('showMoreItemsForThread') - let itemIdsToAdd = store.getForTimeline(instanceName, timelineName, 'itemIdsToAdd') - let timelineItemIds = store.getForTimeline(instanceName, timelineName, 'timelineItemIds') + let itemSummariesToAdd = store.getForTimeline(instanceName, timelineName, 'timelineItemSummariesToAdd') + let timelineItemSummaries = store.getForTimeline(instanceName, timelineName, 'timelineItemSummaries') + let timelineItemIds = new Set(timelineItemSummaries.map(_ => _.id)) // TODO: update database and do the thread merge correctly - for (let itemIdToAdd of itemIdsToAdd) { - if (!timelineItemIds.includes(itemIdToAdd)) { - timelineItemIds.push(itemIdToAdd) + for (let itemSummaryToAdd of itemSummariesToAdd) { + if (!timelineItemIds.has(itemSummaryToAdd.id)) { + timelineItemSummaries.push(itemSummaryToAdd) } } store.setForTimeline(instanceName, timelineName, { - itemIdsToAdd: [], - timelineItemIds: timelineItemIds + timelineItemSummariesToAdd: [], + timelineItemSummaries: timelineItemSummaries }) stop('showMoreItemsForThread') } diff --git a/src/routes/_components/timeline/Timeline.html b/src/routes/_components/timeline/Timeline.html index e426452..68fe971 100644 --- a/src/routes/_components/timeline/Timeline.html +++ b/src/routes/_components/timeline/Timeline.html @@ -1,11 +1,4 @@

{label}

-
( - timelineType === 'status' && $firstTimelineItemId && - timelineValue !== $firstTimelineItemId && timelineValue + timelineType === 'status' && + $firstTimelineItemId && + timelineValue !== $firstTimelineItemId && + timelineValue + ), + itemIds: ({ $timelineItemSummaries }) => ( + // TODO: filter + $timelineItemSummaries && $timelineItemSummaries.map(_ => _.id) + ), + itemIdsToAdd: ({ $timelineItemSummariesToAdd }) => ( + // TODO: filter + $timelineItemSummariesToAdd && $timelineItemSummariesToAdd.map(_ => _.id) ), - itemIdsToAdd: ({ $itemIdsToAdd }) => $itemIdsToAdd, headerProps: ({ itemIdsToAdd }) => { return { count: itemIdsToAdd ? itemIdsToAdd.length : 0, diff --git a/src/routes/_store/computations/timelineComputations.js b/src/routes/_store/computations/timelineComputations.js index 50b79a3..fea2af9 100644 --- a/src/routes/_store/computations/timelineComputations.js +++ b/src/routes/_store/computations/timelineComputations.js @@ -1,4 +1,5 @@ import { get } from '../../_utils/lodash-lite' +import { getFirstIdFromItemSummaries, getLastIdFromItemSummaries } from '../../_utils/getIdFromItemSummaries' function computeForTimeline (store, key, defaultValue) { store.compute(key, @@ -10,24 +11,24 @@ function computeForTimeline (store, key, defaultValue) { } export function timelineComputations (store) { - computeForTimeline(store, 'timelineItemIds', null) + computeForTimeline(store, 'timelineItemSummaries', null) + computeForTimeline(store, 'timelineItemSummariesToAdd', null) computeForTimeline(store, 'runningUpdate', false) computeForTimeline(store, 'lastFocusedElementId', null) computeForTimeline(store, 'ignoreBlurEvents', false) - computeForTimeline(store, 'itemIdsToAdd', null) computeForTimeline(store, 'showHeader', false) computeForTimeline(store, 'shouldShowHeader', false) - computeForTimeline(store, 'timelineItemIdsAreStale', false) + computeForTimeline(store, 'timelineItemSummariesAreStale', false) - store.compute('firstTimelineItemId', ['timelineItemIds'], (timelineItemIds) => { - return timelineItemIds && timelineItemIds[0] - }) - store.compute('lastTimelineItemId', ['timelineItemIds'], (timelineItemIds) => { - return timelineItemIds && timelineItemIds[timelineItemIds.length - 1] - }) + store.compute('firstTimelineItemId', ['timelineItemSummaries'], (timelineItemSummaries) => ( + getFirstIdFromItemSummaries(timelineItemSummaries) + )) + store.compute('lastTimelineItemId', ['timelineItemSummaries'], (timelineItemSummaries) => ( + getLastIdFromItemSummaries(timelineItemSummaries) + )) store.compute('numberOfNotifications', - [`timelineData_itemIdsToAdd`, 'currentInstance'], + [`timelineData_timelineItemSummariesToAdd`, 'currentInstance'], (root, currentInstance) => ( (root && root[currentInstance] && root[currentInstance].notifications && root[currentInstance].notifications.length) || 0 diff --git a/src/routes/_store/mixins/timelineMixins.js b/src/routes/_store/mixins/timelineMixins.js index 6be04cf..ecec063 100644 --- a/src/routes/_store/mixins/timelineMixins.js +++ b/src/routes/_store/mixins/timelineMixins.js @@ -31,7 +31,7 @@ export function timelineMixins (Store) { } Store.prototype.getThreads = function (instanceName) { - let instanceData = this.getAllTimelineData(instanceName, 'timelineItemIds') + let instanceData = this.getAllTimelineData(instanceName, 'timelineItemSummaries') return pickBy(instanceData, (value, key) => { return key.startsWith('status/') diff --git a/src/routes/_store/observers/instanceObservers.js b/src/routes/_store/observers/instanceObservers.js index aeebc3f..b98095d 100644 --- a/src/routes/_store/observers/instanceObservers.js +++ b/src/routes/_store/observers/instanceObservers.js @@ -9,6 +9,7 @@ import { TIMELINE_BATCH_SIZE } from '../../_static/timelines' import { scheduleIdleTask } from '../../_utils/scheduleIdleTask' import { mark, stop } from '../../_utils/marks' import { store } from '../store' +import { getFirstIdFromItemSummaries } from '../../_utils/getIdFromItemSummaries' // stream to watch for home timeline updates and notifications let currentInstanceStream @@ -56,12 +57,12 @@ async function refreshInstanceData (instanceName) { } function stream (store, instanceName, currentInstanceInfo) { - let homeTimelineItemIds = store.getForTimeline(instanceName, - 'home', 'timelineItemIds') - let firstHomeTimelineItemId = homeTimelineItemIds && homeTimelineItemIds[0] - let notificationItemIds = store.getForTimeline(instanceName, - 'notifications', 'timelineItemIds') - let firstNotificationTimelineItemId = notificationItemIds && notificationItemIds[0] + let homeTimelineItemSummaries = store.getForTimeline(instanceName, + 'home', 'timelineItemSummaries') + let firstHomeTimelineItemId = getFirstIdFromItemSummaries(homeTimelineItemSummaries) + let notificationItemSummaries = store.getForTimeline(instanceName, + 'notifications', 'timelineItemSummaries') + let firstNotificationTimelineItemId = getFirstIdFromItemSummaries(notificationItemSummaries) let { accessToken } = store.get() let streamingApi = currentInstanceInfo.urls.streaming_api diff --git a/src/routes/_store/observers/timelineObservers.js b/src/routes/_store/observers/timelineObservers.js index cb5885a..69e7755 100644 --- a/src/routes/_store/observers/timelineObservers.js +++ b/src/routes/_store/observers/timelineObservers.js @@ -4,6 +4,7 @@ import { getTimeline } from '../../_api/timelines' import { addStatusesOrNotifications } from '../../_actions/addStatusOrNotification' import { TIMELINE_BATCH_SIZE } from '../../_static/timelines' import { store } from '../store' +import { getFirstIdFromItemSummaries } from '../../_utils/getIdFromItemSummaries' export function timelineObservers () { // stream to watch for local/federated/etc. updates. home and notification @@ -58,9 +59,9 @@ export function timelineObservers () { return } - let timelineItemIds = store.getForTimeline(currentInstance, - currentTimeline, 'timelineItemIds') - let firstTimelineItemId = timelineItemIds && timelineItemIds[0] + let timelineItemSummaries = store.getForTimeline(currentInstance, + currentTimeline, 'timelineItemSummaries') + let firstTimelineItemId = getFirstIdFromItemSummaries(timelineItemSummaries) let onOpenStream = async () => { if (!firstTimelineItemId || !currentTimelineIsUnchanged()) { diff --git a/src/routes/_utils/arrays.js b/src/routes/_utils/arrays.js index b13882a..4a4cd6c 100644 --- a/src/routes/_utils/arrays.js +++ b/src/routes/_utils/arrays.js @@ -1,6 +1,5 @@ -// Merge two arrays, assuming both input arrays have the same order -// and items are comparable -export function mergeArrays (leftArray, rightArray) { +// Merge two arrays, using the given comparator +export function mergeArrays (leftArray, rightArray, comparator) { let leftIndex = 0 let rightIndex = 0 let merged = [] @@ -17,11 +16,12 @@ export function mergeArrays (leftArray, rightArray) { } let left = leftArray[leftIndex] let right = rightArray[rightIndex] - if (right === left) { + let comparison = comparator(right, left) + if (comparison === 0) { merged.push(left) rightIndex++ leftIndex++ - } else if (parseInt(right, 10) > parseInt(left, 10)) { + } else if (comparison > 0) { merged.push(right) rightIndex++ } else { diff --git a/src/routes/_utils/getIdFromItemSummaries.js b/src/routes/_utils/getIdFromItemSummaries.js new file mode 100644 index 0000000..fb54562 --- /dev/null +++ b/src/routes/_utils/getIdFromItemSummaries.js @@ -0,0 +1,11 @@ +export function getFirstIdFromItemSummaries (itemSummaries) { + return itemSummaries && + itemSummaries[0] && + itemSummaries[0].id +} + +export function getLastIdFromItemSummaries (itemSummaries) { + return itemSummaries && + itemSummaries[itemSummaries.length - 1] && + itemSummaries[itemSummaries.length - 1].id +} diff --git a/src/routes/_utils/sorting.js b/src/routes/_utils/sorting.js index 22d4a3d..747cdea 100644 --- a/src/routes/_utils/sorting.js +++ b/src/routes/_utils/sorting.js @@ -17,8 +17,8 @@ export function toReversePaddedBigInt (id) { return res } -export function byItemIds (a, b) { - let aPadded = toPaddedBigInt(a) - let bPadded = toPaddedBigInt(b) - return aPadded < bPadded ? -1 : aPadded === bPadded ? 0 : 1 +export function compareTimelineItemSummaries (left, right) { + let leftPadded = toPaddedBigInt(left.id) + let rightPadded = toPaddedBigInt(right.id) + return leftPadded < rightPadded ? -1 : leftPadded === rightPadded ? 0 : 1 } diff --git a/src/routes/_utils/timelineItemToSummary.js b/src/routes/_utils/timelineItemToSummary.js new file mode 100644 index 0000000..b3e7bab --- /dev/null +++ b/src/routes/_utils/timelineItemToSummary.js @@ -0,0 +1,8 @@ +export function timelineItemToSummary (item) { + return { + id: item.id, + replyId: (item.in_reply_to_id) || void 0, + reblogId: (item.reblog && item.reblog.id) || void 0, + type: item.type || void 0 + } +}