ref(issues): Remove StreamManager (#37972)

static/app/components/issues/groupList.tsx

@@ -21,7 +21,6 @@ import {t} from 'sentry/locale';
 import GroupStore from 'sentry/stores/groupStore';
 import {Group} from 'sentry/types';
 import {callIfFunction} from 'sentry/utils/callIfFunction';
-import StreamManager from 'sentry/utils/streamManager';
 import withApi from 'sentry/utils/withApi';
 import {TimePeriodType} from 'sentry/views/alerts/rules/metric/details/constants';
 import {RELATED_ISSUES_BOOLEAN_QUERY_ERROR} from 'sentry/views/alerts/rules/metric/details/relatedIssuesNotAvailable';
@@ -113,7 +112,6 @@ class GroupList extends Component<Props, State> {
   }
 
   listener = GroupStore.listen(() => this.onGroupChange(), undefined);
-  private _streamManager = new StreamManager(GroupStore);
 
   fetchData = async () => {
     GroupStore.loadInitialData([]);
@@ -155,7 +153,7 @@ class GroupList extends Component<Props, State> {
         includeAllArgs: true,
       });
 
-      this._streamManager.push(data);
+      GroupStore.add(data);
 
       this.setState(
         {
@@ -218,7 +216,7 @@ class GroupList extends Component<Props, State> {
   }
 
   onGroupChange() {
-    const groups = this._streamManager.getAllItems();
+    const groups = GroupStore.getAllItems() as Group[];
     if (!isEqual(groups, this.state.groups)) {
       this.setState({groups});
     }

static/app/stores/groupStore.tsx

@@ -1,4 +1,3 @@
-import isArray from 'lodash/isArray';
 import {createStore} from 'reflux';
 
 import {Indicator} from 'sentry/actionCreators/indicator';
@@ -45,6 +44,7 @@ interface InternalDefinition {
 interface GroupStoreDefinition extends CommonStoreDefinition<Item[]>, InternalDefinition {
   add: (items: Item[]) => void;
   addStatus: (id: string, status: string) => void;
+  addToFront: (items: Item[]) => void;
   clearStatus: (id: string, status: string) => void;
 
   get: (id: string) => Item | undefined;
@@ -114,22 +114,13 @@ const storeConfig: GroupStoreDefinition = {
     this.trigger(itemIds);
   },
 
-  add(items) {
-    if (!isArray(items)) {
-      items = [items];
-    }
-
-    const itemsById: Record<string, Item> = {};
-    const itemIds = new Set<string>();
-    items.forEach(item => {
-      itemsById[item.id] = item;
-      itemIds.add(item.id);
-    });
+  mergeItems(items: Item[]) {
+    const itemsById = items.reduce((acc, item) => ({...acc, [item.id]: item}), {});
 
-    // See if any existing items are updated by this new set of items
-    this.items.forEach((item, idx) => {
+    // Merge these items into the store and return a mapping of any that aren't already in the store
+    this.items.forEach((item, itemIndex) => {
       if (itemsById[item.id]) {
-        this.items[idx] = {
+        this.items[itemIndex] = {
           ...item,
           ...itemsById[item.id],
         };
@@ -137,11 +128,37 @@ const storeConfig: GroupStoreDefinition = {
       }
     });
 
-    // New items
-    const newItems = items.filter(item => itemsById.hasOwnProperty(item.id));
-    this.items = this.items.concat(newItems);
+    return items.filter(item => itemsById.hasOwnProperty(item.id));
+  },
 
-    this.trigger(itemIds);
+  /**
+   * Adds the provided items to the end of the list.
+   * If any items already exist, they will merged into the existing item index.
+   */
+  add(items) {
+    if (!Array.isArray(items)) {
+      items = [items];
+    }
+    const newItems = this.mergeItems(items);
+
+    this.items = [...this.items, ...newItems];
+
+    this.trigger(new Set(items.map(item => item.id)));
+  },
+
+  /**
+   * Adds the provided items to the front of the list.
+   * If any items already exist, they will be moved to the front in the order provided.
+   */
+  addToFront(items) {
+    if (!Array.isArray(items)) {
+      items = [items];
+    }
+    const itemMap = items.reduce((acc, item) => ({...acc, [item.id]: item}), {});
+
+    this.items = [...items, ...this.items.filter(item => !itemMap[item.id])];
+
+    this.trigger(new Set(items.map(item => item.id)));
   },
 
   /**

static/app/utils/streamManager.tsx

@@ -1,76 +0,0 @@
-type Options = {
-  /** Max number of items to keep at once */
-  limit?: number;
-};
-
-/**
- * Minimal type shape for objects that can be managed inside StreamManager.
- */
-type IdShape = {
-  id: string;
-};
-
-class StreamManager {
-  private idList: string[] = [];
-  private limit: number;
-  private store: any;
-
-  // TODO(dcramer): this should listen to changes on GroupStore and remove
-  // items that are removed there
-  // TODO(ts) Add better typing for store. Generally this is GroupStore, but it could be other things.
-  constructor(store: any, options: Options = {}) {
-    this.store = store;
-    this.limit = options.limit || 100;
-  }
-
-  reset() {
-    this.idList = [];
-  }
-
-  trim() {
-    if (this.limit > this.idList.length) {
-      return;
-    }
-
-    const excess = this.idList.splice(this.limit, this.idList.length - this.limit);
-    this.store.remove(excess);
-  }
-
-  push(items: IdShape | IdShape[] = []) {
-    items = Array.isArray(items) ? items : [items];
-    if (items.length === 0) {
-      return;
-    }
-
-    items = items.filter(item => item.hasOwnProperty('id'));
-    const ids = items.map(item => item.id);
-    this.idList = this.idList.filter(id => !ids.includes(id));
-    this.idList = [...this.idList, ...ids];
-
-    this.trim();
-    this.store.add(items);
-  }
-
-  getAllItems() {
-    return this.store
-      .getAllItems()
-      .slice()
-      .sort((a, b) => this.idList.indexOf(a.id) - this.idList.indexOf(b.id));
-  }
-
-  unshift(items: IdShape | IdShape[] = []) {
-    items = Array.isArray(items) ? items : [items];
-    if (items.length === 0) {
-      return;
-    }
-
-    const ids = items.map(item => item.id);
-    this.idList = this.idList.filter(id => !ids.includes(id));
-    this.idList = [...ids, ...this.idList];
-
-    this.trim();
-    this.store.add(items);
-  }
-}
-
-export default StreamManager;

static/app/views/issueList/overview.tsx

@@ -53,7 +53,6 @@ import parseApiError from 'sentry/utils/parseApiError';
 import parseLinkHeader from 'sentry/utils/parseLinkHeader';
 import {VisuallyCompleteWithData} from 'sentry/utils/performanceForSentry';
 import {decodeScalar} from 'sentry/utils/queryString';
-import StreamManager from 'sentry/utils/streamManager';
 import withApi from 'sentry/utils/withApi';
 import withIssueTags from 'sentry/utils/withIssueTags';
 import withOrganization from 'sentry/utils/withOrganization';
@@ -81,6 +80,7 @@ const DEFAULT_SORT = IssueSortOptions.DATE;
 const DEFAULT_GRAPH_STATS_PERIOD = '24h';
 // the allowed period choices for graph in each issue row
 const DYNAMIC_COUNTS_STATS_PERIODS = new Set(['14d', '24h', 'auto']);
+const MAX_ISSUES_COUNT = 100;
 
 type Params = {
   orgId: string;
@@ -278,7 +278,6 @@ class IssueListOverview extends Component<Props, State> {
   private _lastRequest: any;
   private _lastStatsRequest: any;
   private _lastFetchCountsRequest: any;
-  private _streamManager = new StreamManager(GroupStore);
 
   getQuery(): string {
     const {savedSearch, location} = this.props;
@@ -516,7 +515,6 @@ class IssueListOverview extends Component<Props, State> {
     if (hasIssueListRemovalAction && !this.state.realtimeActive) {
       if (!this.state.actionTaken && !this.state.undo) {
         GroupStore.loadInitialData([]);
-        this._streamManager.reset();
 
         this.setState({
           issuesLoading: true,
@@ -528,7 +526,6 @@ class IssueListOverview extends Component<Props, State> {
     } else {
       if (!this.state.reviewedIds.length || !isForReviewQuery(query)) {
         GroupStore.loadInitialData([]);
-        this._streamManager.reset();
 
         this.setState({
           issuesLoading: true,
@@ -610,7 +607,7 @@ class IssueListOverview extends Component<Props, State> {
         if (this.state.undo) {
           GroupStore.loadInitialData(data);
         }
-        this._streamManager.push(data);
+        GroupStore.add(data);
 
         // TODO(Kelly): update once issue-list-removal-action feature is stable
         if (!hasIssueListRemovalAction) {
@@ -730,7 +727,7 @@ class IssueListOverview extends Component<Props, State> {
   onRealtimePoll = (data: any, _links: any) => {
     // Note: We do not update state with cursors from polling,
     // `CursorPoller` updates itself with new cursors
-    this._streamManager.unshift(data);
+    GroupStore.addToFront(data);
   };
 
   listener = GroupStore.listen(() => this.onGroupChange(), undefined);
@@ -749,7 +746,7 @@ class IssueListOverview extends Component<Props, State> {
       !this.state.realtimeActive &&
       actionTakenGroupData.length > 0
     ) {
-      const filteredItems = this._streamManager.getAllItems().filter(item => {
+      const filteredItems = GroupStore.getAllItems().filter(item => {
         return actionTakenGroupData.findIndex(data => data.id === item.id) !== -1;
       });
 
@@ -792,7 +789,9 @@ class IssueListOverview extends Component<Props, State> {
       }
     }
 
-    const groupIds = this._streamManager.getAllItems().map(item => item.id) ?? [];
+    const groupIds = GroupStore.getAllItems()
+      .map(item => item.id)
+      .slice(0, MAX_ISSUES_COUNT);
     if (!isEqual(groupIds, this.state.groupIds)) {
       this.setState({groupIds});
     }

tests/js/spec/stores/groupStore.spec.tsx

@@ -31,6 +31,32 @@ describe('GroupStore', function () {
     });
   });
 
+  describe('addToFront()', function () {
+    it('should add new entries to beginning of the list', function () {
+      GroupStore.items = [g('2')];
+      GroupStore.addToFront([g('1'), g('3')]);
+
+      expect(GroupStore.items).toEqual([g('1'), g('3'), g('2')]);
+    });
+
+    it('should update matching existing entries', function () {
+      GroupStore.items = [g('1'), g('2')];
+
+      GroupStore.addToFront([{id: '1', foo: 'bar'}, g('3')]);
+
+      expect(GroupStore.getAllItems()).toEqual([
+        expect.objectContaining({id: '1', foo: 'bar'}),
+        g('3'),
+        g('2'),
+      ]);
+    });
+
+    it('should attempt to preserve order of ids', function () {
+      GroupStore.addToFront([g('2'), g('1'), g('3')]);
+      expect(GroupStore.getAllItemIds()).toEqual(['2', '1', '3']);
+    });
+  });
+
   describe('remove()', function () {
     it('should remove entry', function () {
       GroupStore.items = [g('1'), g('2')];

tests/js/spec/utils/streamManager.spec.jsx

@@ -1,155 +0,0 @@
-import {createStore} from 'reflux';
-
-import StreamManager from 'sentry/utils/streamManager';
-
-describe('StreamManager', function () {
-  let store;
-
-  beforeEach(function () {
-    store = createStore({
-      add() {},
-      getAllItems() {},
-      remove() {},
-    });
-  });
-
-  it('allows options configuration', function () {
-    const options = {limit: 2};
-    const mgr = new StreamManager(store, options);
-
-    expect(mgr.limit).toEqual(options.limit);
-  });
-
-  describe('push()', function () {
-    it('allows passing no items', function () {
-      const mgr = new StreamManager(store);
-      expect(() => mgr.push()).not.toThrow();
-      expect(() => mgr.push([])).not.toThrow();
-      expect(mgr.idList).toHaveLength(0);
-    });
-
-    it('adds items', function () {
-      const storeAdd = jest.spyOn(store, 'add');
-      const mgr = new StreamManager(store);
-      const items = [{id: 1}];
-      mgr.push(items);
-
-      expect(mgr.idList).toHaveLength(1);
-      expect(storeAdd).toHaveBeenCalledWith(items);
-    });
-
-    it('allows adding a single item', function () {
-      const storeAdd = jest.spyOn(store, 'add');
-      const mgr = new StreamManager(store);
-      const item = {id: 1};
-      mgr.push(item);
-
-      expect(mgr.idList).toHaveLength(1);
-      expect(storeAdd).toHaveBeenCalledWith([item]);
-    });
-
-    it('trims after adding', function () {
-      const mgr = new StreamManager(store, {limit: 1});
-      const storeRemove = jest.spyOn(store, 'remove');
-      const mgrTrim = jest.spyOn(mgr, 'trim');
-      mgr.push([{id: 1}, {id: 2}]);
-
-      expect(mgr.idList).toHaveLength(1);
-      expect(storeRemove).toHaveBeenCalledWith([2]);
-      expect(mgrTrim).toHaveBeenCalled();
-    });
-
-    it('preserves NEW order of duplicates', function () {
-      const mgr = new StreamManager(store);
-      mgr.push([{id: 1}, {id: 3}]);
-      mgr.push([{id: 1}, {id: 2}]); // New order of "1" if after "3"
-
-      expect(mgr.idList).toEqual([3, 1, 2]);
-    });
-  });
-
-  describe('trim()', function () {
-    it('removes trailing items in excess of the limit', function () {
-      const storeRemove = jest.spyOn(store, 'remove');
-      const mgr = new StreamManager(store, {limit: 1});
-      mgr.idList = [1, 2, 3];
-      mgr.trim();
-
-      expect(mgr.idList).toEqual([1]);
-      expect(mgr.idList).toHaveLength(1);
-      expect(storeRemove).toHaveBeenCalledWith([2, 3]);
-    });
-
-    it('does nothing with fewer items than limit', function () {
-      const storeRemove = jest.spyOn(store, 'remove');
-      const mgr = new StreamManager(store, {limit: 10});
-      mgr.idList = [1, 2, 3];
-      mgr.trim();
-
-      expect(mgr.idList).toEqual([1, 2, 3]);
-      expect(mgr.idList).toHaveLength(3);
-      expect(storeRemove).not.toHaveBeenCalled();
-    });
-  });
-
-  describe('getAllItems()', function () {
-    it('retrives ordered items from store', function () {
-      const storeGetAllItems = jest
-        .spyOn(store, 'getAllItems')
-        .mockImplementation(() => [{id: 1}, {id: 2}]);
-      const mgr = new StreamManager(store);
-      mgr.push({id: 2});
-      mgr.push({id: 1});
-      const items = mgr.getAllItems();
-
-      expect(items).toEqual([{id: 2}, {id: 1}]);
-      expect(storeGetAllItems).toHaveBeenCalled();
-    });
-
-    it('does not mutate store', function () {
-      const storeItems = [{id: 1}, {id: 2}];
-      jest.spyOn(store, 'getAllItems').mockImplementation(() => storeItems);
-      const mgr = new StreamManager(store);
-      mgr.push([{id: 2}, {id: 1}]);
-      mgr.getAllItems();
-
-      expect(store.getAllItems()).toEqual([{id: 1}, {id: 2}]);
-    });
-  });
-
-  describe('unshift()', function () {
-    it('adds items to the start of the list', function () {
-      const storeAdd = jest.spyOn(store, 'add');
-      const mgr = new StreamManager(store);
-      mgr.unshift([{id: 2}]);
-      mgr.unshift([{id: 1}]);
-
-      expect(mgr.idList).toEqual([1, 2]);
-      expect(storeAdd.mock.calls[0][0]).toEqual([{id: 2}]);
-      expect(storeAdd.mock.calls[1][0]).toEqual([{id: 1}]);
-    });
-
-    it('moves duplicates to the start of the list', function () {
-      const mgr = new StreamManager(store);
-      mgr.unshift([{id: 2}, {id: 1}]);
-      mgr.unshift([{id: 1}]);
-
-      expect(mgr.idList).toEqual([1, 2]);
-    });
-
-    it('moves a duplicate array to the start of the list and preserves order', function () {
-      const mgr = new StreamManager(store);
-      mgr.unshift([{id: 3}, {id: 2}, {id: 1}]);
-      mgr.unshift([{id: 2}, {id: 1}]);
-
-      expect(mgr.idList).toEqual([2, 1, 3]);
-    });
-
-    it('allows adding a single item', function () {
-      const mgr = new StreamManager(store);
-      mgr.unshift({id: 1});
-
-      expect(mgr.idList).toEqual([1]);
-    });
-  });
-});