PR feedback - coderabbit

Author: petersutter

backend/__tests__/cache.tickets.spec.js

@@ -186,6 +186,39 @@ describe('cache', function () {
       })
     })
 
+    describe('#getIssuesForShoot', function () {
+      it('should return issues matching projectName and name', function () {
+        const result = cache.getIssuesForShoot({ projectName: 'test', name: 'foo' })
+        expect(result).toEqual([firstIssue, secondIssue])
+        expect(emitSpy).not.toHaveBeenCalled()
+      })
+
+      it('should return empty array for unknown shoot', function () {
+        const result = cache.getIssuesForShoot({ projectName: 'test', name: 'unknown' })
+        expect(result).toEqual([])
+      })
+
+      it('should update index when issue is removed', function () {
+        cache.removeIssue({ issue: firstIssue })
+        const result = cache.getIssuesForShoot({ projectName: 'test', name: 'foo' })
+        expect(result).toEqual([secondIssue])
+      })
+
+      it('should update index when issue is updated with new shoot', function () {
+        const updatedIssue = {
+          metadata: {
+            number: 1,
+            updated_at: new Date(Date.now() + 60000).toISOString(),
+            name: 'bar',
+            projectName: 'test',
+          },
+        }
+        cache.addOrUpdateIssue({ issue: updatedIssue })
+        expect(cache.getIssuesForShoot({ projectName: 'test', name: 'foo' })).toEqual([secondIssue])
+        expect(cache.getIssuesForShoot({ projectName: 'test', name: 'bar' })).toEqual([thirdIssue, updatedIssue])
+      })
+    })
+
     describe('#getCommentsForIssue', function () {
       it('should return the all comments for the first issue', function () {
         const comments = cache.getCommentsForIssue({ issueNumber: 1 })

backend/__tests__/hooks.spec.js

@@ -112,6 +112,7 @@ describe('hooks', () => {
         hooks.constructor.createInformers = mockCreateInformers = vi.fn(() => informers)
         cache.initialize = vi.fn()
         cache.indexProjectsByNamespace = vi.fn()
+        cache.indexShootsBySeedName = vi.fn()
         cache.getTicketCache = vi.fn(() => ticketCache)
         io.mockReturnValue(ioInstance)
       })
@@ -136,6 +137,9 @@ describe('hooks', () => {
         expect(cache.indexProjectsByNamespace).toHaveBeenCalledTimes(1)
         expect(cache.indexProjectsByNamespace.mock.calls[0]).toEqual([informers.projects])
 
+        expect(cache.indexShootsBySeedName).toHaveBeenCalledTimes(1)
+        expect(cache.indexShootsBySeedName.mock.calls[0]).toEqual([informers.shoots])
+
         expect(io).toHaveBeenCalledTimes(1)
         expect(io.mock.calls[0]).toEqual([server, expect.anything()])
 

backend/__tests__/io.seedstats.spec.js

@@ -61,7 +61,7 @@ describe('io/seedstats', () => {
       },
     }
 
-    expect(() => getJoinedRooms(nsp)).toThrow(new TypeError('Invalid seedstats room: invalid-room'))
+    expect(() => getJoinedRooms(nsp, { seedName: 'seed-1' })).toThrow(new TypeError('Invalid seedstats room: invalid-room'))
   })
 
   it('should reject unauthorized seedstats subscriptions', async () => {

backend/__tests__/services.seedstats.spec.js

@@ -113,12 +113,16 @@ describe('services/seedstats', () => {
   ]
 
   const ticketCache = {
-    getIssues: vi.fn(() => issues),
+    getIssuesForShoot: vi.fn(({ name, projectName }) =>
+      issues.filter(issue => issue.metadata.projectName === projectName && issue.metadata.name === name),
+    ),
   }
 
   beforeEach(() => {
     vi.resetAllMocks()
-    ticketCache.getIssues.mockReturnValue(issues)
+    ticketCache.getIssuesForShoot.mockImplementation(({ name, projectName }) =>
+      issues.filter(issue => issue.metadata.projectName === projectName && issue.metadata.name === name),
+    )
     config.frontend.ticket = {
       hideClustersWithLabels: ['suppress'],
     }

backend/lib/cache/tickets.js

@@ -8,8 +8,13 @@ import EventEmitter from 'events'
 import _ from 'lodash-es'
 import logger from '../logger/index.js'
 
+function shootKey (projectName, name) {
+  return JSON.stringify([projectName, name])
+}
+
 function init () {
   const issues = new Map()
+  const issuesByShoot = new Map() // Map<shootKey, Map<issueNumber, issue>>
   const commentsForIssues = new Map() // we could also think of getting rid of the comments cache
   const emitter = new EventEmitter()
 
@@ -55,11 +60,19 @@ function init () {
   }
 
   function getIssueNumbersForNameAndProjectName ({ name, projectName }) {
-    return _
-      .chain(getIssues())
-      .filter(_.matches({ metadata: { name, projectName } }))
-      .map('metadata.number')
-      .value()
+    const map = issuesByShoot.get(shootKey(projectName, name))
+    if (!map) {
+      return []
+    }
+    return Array.from(map.keys())
+  }
+
+  function getIssuesForShoot ({ name, projectName }) {
+    const map = issuesByShoot.get(shootKey(projectName, name))
+    if (!map) {
+      return []
+    }
+    return Array.from(map.values())
   }
 
   function getCommentsForIssueCache ({ issueNumber }) {
@@ -76,7 +89,14 @@ function init () {
   }
 
   function addOrUpdateIssue ({ issue }) {
+    const number = issue.metadata.number
+    const oldIssue = issues.get(number)
+    if (oldIssue) {
+      removeFromShootIndex(oldIssue)
+    }
     updateIfNewer('issue', issues, issue)
+    // Re-read from issues map — updateIfNewer may have kept the old item if it was newer
+    addToShootIndex(issues.get(number))
   }
 
   function addOrUpdateComment ({ issueNumber, comment }) {
@@ -88,6 +108,11 @@ function init () {
     const issueNumber = issue.metadata.number
     logger.trace('removing issue', issueNumber, 'and comments')
 
+    const cachedIssue = issues.get(issueNumber)
+    if (cachedIssue) {
+      removeFromShootIndex(cachedIssue)
+    }
+
     const comments = getCommentsForIssueCache({ issueNumber })
 
     issues.delete(issueNumber)
@@ -107,6 +132,36 @@ function init () {
     emitCommmentDeleted(comment)
   }
 
+  function addToShootIndex (issue) {
+    const { projectName, name, number } = issue?.metadata ?? {}
+    if (!projectName || !name) {
+      return
+    }
+    const key = shootKey(projectName, name)
+    let map = issuesByShoot.get(key)
+    if (!map) {
+      map = new Map()
+      issuesByShoot.set(key, map)
+    }
+    map.set(number, issue)
+  }
+
+  function removeFromShootIndex (issue) {
+    const { projectName, name, number } = issue?.metadata ?? {}
+    if (!projectName || !name) {
+      return
+    }
+    const key = shootKey(projectName, name)
+    const map = issuesByShoot.get(key)
+    if (!map) {
+      return
+    }
+    map.delete(number)
+    if (map.size === 0) {
+      issuesByShoot.delete(key)
+    }
+  }
+
   function updateIfNewer (kind, cachedMap, item) {
     const identifier = kind === 'issue'
       ? item.metadata.number
@@ -142,6 +197,7 @@ function init () {
     getCommentsForIssue,
     getIssueNumbers,
     getIssueNumbersForNameAndProjectName,
+    getIssuesForShoot,
     addOrUpdateIssues,
     addOrUpdateIssue,
     addOrUpdateComment,

backend/lib/io/seedstats.js

@@ -76,7 +76,12 @@ function parseRoomName (room) {
   throw new TypeError(`Invalid seedstats room: ${room}`)
 }
 
-function getJoinedRooms (nsp, { seedName } = {}) {
+function getJoinedRooms (nsp, { seedName }) {
+  if (!seedName) {
+    logger.warn('getJoinedRooms called without seedName')
+    return []
+  }
+
   const adapterRooms = nsp?.adapter?.rooms
   if (!adapterRooms) {
     return []

backend/lib/services/seedstats.js

@@ -251,8 +251,7 @@ function getTicketsForShoot (shoot) {
 
   return cache
     .getTicketCache()
-    .getIssues()
-    .filter(issue => issue?.metadata?.projectName === projectName && issue?.metadata?.name === shootName)
+    .getIssuesForShoot({ projectName, name: shootName })
 }
 
 function getProjectNameForShoot (shoot) {

frontend/src/composables/useApi/api.js

@@ -259,6 +259,9 @@ export function getSeedStats ({ unhealthyFilterMask } = {}) {
 }
 
 export function getSeedStat ({ name, unhealthyFilterMask } = {}) {
+  if (!name) {
+    throw new TypeError('getSeedStat requires a name parameter')
+  }
   name = encodeURIComponent(name)
   return getResource(withQuery(`/api/seedstats/${name}`, {
     unhealthyFilterMask,

frontend/src/composables/useSeedItem/index.js

@@ -254,9 +254,9 @@ export function useSeedStats (seedItem) {
   const seedName = computed(() => get(seedItem.value, ['metadata', 'name']))
   const seedStat = computed(() => seedStatStore.statByName(seedName.value))
 
-  const seedShootCount = computed(() => seedStat.value?.counts?.shootCount ?? 0)
-  const seedTotalUnhealthyShoots = computed(() => seedStat.value?.counts?.unhealthyShoots?.total ?? 0)
-  const seedUnhealthyShoots = computed(() => seedStat.value?.counts?.unhealthyShoots?.matching ?? 0)
+  const seedShootCount = computed(() => seedStat.value?.counts?.shootCount)
+  const seedTotalUnhealthyShoots = computed(() => seedStat.value?.counts?.unhealthyShoots?.total)
+  const seedUnhealthyShoots = computed(() => seedStat.value?.counts?.unhealthyShoots?.matching)
 
   return {
     seedShootCount,