From 0df650a4bd73e177516f7caa2d1fad9b753fbc7f Mon Sep 17 00:00:00 2001 From: anilb Date: Mon, 21 Sep 2026 13:27:42 +0200 Subject: [PATCH 1/3] feat: add active days endpoint under v1-alpha IN-1334 Signed-off-by: anilb --- .../v1-alpha/development/active-days.ts | 162 ++++++ api/tests/development-active-days.test.ts | 516 ++++++++++++++++++ 2 files changed, 678 insertions(+) create mode 100644 api/src/versions/v1-alpha/development/active-days.ts create mode 100644 api/tests/development-active-days.test.ts diff --git a/api/src/versions/v1-alpha/development/active-days.ts b/api/src/versions/v1-alpha/development/active-days.ts new file mode 100644 index 000000000..3d174f7fc --- /dev/null +++ b/api/src/versions/v1-alpha/development/active-days.ts @@ -0,0 +1,162 @@ +// Copyright (c) 2025 The Linux Foundation and each contributor. +// SPDX-License-Identifier: MIT +import type { FastifyPluginAsyncTypebox } from '@fastify/type-provider-typebox'; +import { + TinybirdProjectNotFoundError, + type TinybirdClient, + type TinybirdQuery, +} from '@lfx-insights/tinybird-client'; +import { Type } from '@sinclair/typebox'; +import { getTinybirdClient } from '../../../clients/tinybird.js'; +import { UpstreamUnavailableError } from '../../../lib/errors.js'; +import { getPreviousDates, toPeriodSummary, type DateRange } from '../../../lib/period.js'; +import { + DateRangeQuery, + Granularity, + PeriodSummary, + ProjectSlugParams, +} from '../../../schemas/common.js'; + +const pipePath = '/v0/pipes/active_days.json'; + +interface SummaryRow { + activeDaysCount: number; + avgContributionsPerDay: number | null; +} + +interface SeriesRow { + startDate: string; + endDate: string; + activityCount: number; +} + +const ActiveDaysQuery = Type.Composite([ + DateRangeQuery, + Type.Object({ + granularity: { ...Granularity, description: 'Width of each bucket in `data`.' }, + includeCollaborations: Type.Optional( + Type.Boolean({ + default: false, + description: + 'Also count collaboration activity (reviews, comments and similar) as contributions, alongside code contributions.', + }), + ), + }), +]); + +const ActiveDaysBucket = Type.Object({ + startDate: Type.String({ + format: 'date-time', + description: 'First day of the bucket, at 00:00:00 UTC.', + }), + endDate: Type.String({ + format: 'date-time', + description: 'Last calendar day of the bucket, at 00:00:00 UTC.', + }), + contributions: Type.Integer({ description: 'Number of contributions in the bucket (count).' }), +}); + +const ActiveDays = Type.Object({ + summary: { + ...PeriodSummary, + description: + 'Active days in the current period against the previous one. current, previous and changeValue are counts of days.', + }, + avgContributionsPerDay: Type.Number({ + description: + 'Average number of contributions per active day in the current period (count per day). 0 when there are no active days.', + }), + data: Type.Array(ActiveDaysBucket, { + description: 'One entry per granularity bucket in the current period.', + }), +}); + +// The pipe takes DateTime parameters and returns its Date columns as YYYY-MM-DD. +const toTinybirdRange = (range: DateRange) => ({ + startDate: `${range.startDate} 00:00:00`, + endDate: `${range.endDate} 00:00:00`, +}); +const toUtcDateTime = (day: string) => `${day}T00:00:00Z`; + +async function fetchRows(client: TinybirdClient, params: TinybirdQuery): Promise { + try { + const { data } = await client.fetch(pipePath, params); + return data; + } catch (err: unknown) { + // The client reports a slug with no bucket as not found. A metric endpoint answers an + // unknown project with empty data, which is why ProjectSlugParams lets any slug through. + if (err instanceof TinybirdProjectNotFoundError) { + return []; + } + throw err; + } +} + +const activeDaysRoutes: FastifyPluginAsyncTypebox = async (scope) => { + scope.get( + '/projects/:slug/development/active-days', + { + schema: { + tags: ['Development'], + summary: 'Get active days', + description: + 'Returns the number of days with at least one development activity in the period against the previous period, the average contributions per active day, and the contributions per bucket. The previous period has the same length and ends the day before `startDate`.', + params: ProjectSlugParams, + querystring: ActiveDaysQuery, + response: { 200: ActiveDays }, + }, + }, + async (request) => { + const { slug } = request.params; + const { + repos, + startDate, + endDate, + granularity, + includeCollaborations = false, + } = request.query; + const dates = getPreviousDates(startDate, endDate); + + // Ajv turns a bare `repos=` into [''] and the client sends an empty array as `repos=`, + // which the pipe would apply as a filter. The Nuxt handler drops an empty value too. + const repoFilter = repos?.filter(Boolean); + const shared = { + project: slug, + repos: repoFilter?.length ? repoFilter : undefined, + includeCodeContributions: true, + includeCollaborations, + }; + const currentRange = toTinybirdRange(dates.current); + const previousRange = toTinybirdRange(dates.previous); + + // A missing API_TB_* variable throws here, outside the catch, so it stays a 500. + const client = getTinybirdClient(); + const [currentRows, previousRows, seriesRows] = await Promise.all([ + fetchRows(client, { ...shared, ...currentRange }), + fetchRows(client, { ...shared, ...previousRange }), + fetchRows(client, { ...shared, ...currentRange, granularity }), + ]).catch((err: unknown) => { + request.log.error({ err }, 'Tinybird active_days request failed'); + throw new UpstreamUnavailableError(); + }); + + const current = currentRows[0]; + const previous = previousRows[0]; + return { + summary: toPeriodSummary( + current?.activeDaysCount ?? 0, + previous?.activeDaysCount ?? 0, + dates.current, + ), + avgContributionsPerDay: current?.avgContributionsPerDay ?? 0, + data: seriesRows.map((row) => ({ + startDate: toUtcDateTime(row.startDate), + endDate: toUtcDateTime(row.endDate), + contributions: row.activityCount, + })), + }; + }, + ); +}; + +export default activeDaysRoutes; diff --git a/api/tests/development-active-days.test.ts b/api/tests/development-active-days.test.ts new file mode 100644 index 000000000..b566260b5 --- /dev/null +++ b/api/tests/development-active-days.test.ts @@ -0,0 +1,516 @@ +// Copyright (c) 2025 The Linux Foundation and each contributor. +// SPDX-License-Identifier: MIT +import type { FastifyInstance } from 'fastify'; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { buildApp } from '../src/app.js'; + +const tinybirdHost = 'https://tinybird.test'; +const mockFetch = vi.fn(); +vi.stubGlobal('fetch', mockFetch); + +const pipePath = '/v0/pipes/active_days.json'; +const bucketPath = '/v0/pipes/project_buckets.json'; +const specPath = '/v1-alpha/projects/{slug}/development/active-days'; + +const startDate = '2025-01-01'; +const endDate = '2025-03-31'; +// getPreviousDates gives the previous period the same length, ending the day before startDate. +const previousStart = '2024-10-01'; +const previousEnd = '2024-12-31'; +const atMidnight = (day: string) => `${day} 00:00:00`; +const isoDay = (day: string) => `${day}T00:00:00Z`; + +const k8sRepo = 'https://github.com/kubernetes/kubernetes'; +const websiteRepo = 'https://github.com/kubernetes/website'; + +const currentRow = { activeDaysCount: 80, avgContributionsPerDay: 21.5 }; +const previousRow = { activeDaysCount: 64, avgContributionsPerDay: 18 }; +const seriesRows = [ + { startDate: '2025-01-01', endDate: '2025-01-31', activityCount: 698 }, + { startDate: '2025-02-01', endDate: '2025-02-28', activityCount: 579 }, + { startDate: '2025-03-01', endDate: '2025-03-31', activityCount: 472 }, +]; + +const expectedBody = { + summary: { + current: 80, + previous: 64, + percentageChange: 25, + changeValue: 16, + periodFrom: isoDay(startDate), + periodTo: isoDay(endDate), + }, + avgContributionsPerDay: 21.5, + data: [ + { startDate: isoDay('2025-01-01'), endDate: isoDay('2025-01-31'), contributions: 698 }, + { startDate: isoDay('2025-02-01'), endDate: isoDay('2025-02-28'), contributions: 579 }, + { startDate: isoDay('2025-03-01'), endDate: isoDay('2025-03-31'), contributions: 472 }, + ], +}; + +const zeroBody = { + summary: { + current: 0, + previous: 0, + percentageChange: 0, + changeValue: 0, + periodFrom: isoDay(startDate), + periodTo: isoDay(endDate), + }, + avgContributionsPerDay: 0, + data: [], +}; + +const tinybirdResponse = (rows: object[]) => + new Response( + JSON.stringify({ + data: rows, + meta: [], + rows: rows.length, + statistics: { elapsed: 0.01, rows_read: 1, bytes_read: 1 }, + }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ); + +interface PipeRows { + bucket?: object[]; + current?: object[]; + previous?: object[]; + series?: object[]; +} + +// The client resolves the slug to a bucket before every pipe call. The series call is the one +// that carries granularity; the two summary calls differ by their startDate. +const routeTinybird = + ({ + bucket = [{ bucketId: 7 }], + current = [currentRow], + previous = [previousRow], + series = seriesRows, + }: PipeRows = {}) => + async (input: unknown) => { + const url = new URL(String(input)); + if (url.pathname === bucketPath) { + return tinybirdResponse(bucket); + } + if (url.searchParams.has('granularity')) { + return tinybirdResponse(series); + } + const isPrevious = url.searchParams.get('startDate') === atMidnight(previousStart); + return tinybirdResponse(isPrevious ? previous : current); + }; + +const pipeCalls = () => + mockFetch.mock.calls + .map((call) => new URL(String(call[0]))) + .filter((url) => url.pathname === pipePath); + +const defaultQuery = { startDate, endDate, granularity: 'monthly' }; + +function url(params: Record = {}, slug = 'kubernetes') { + const search = new URLSearchParams(); + for (const [key, value] of Object.entries({ ...defaultQuery, ...params })) { + for (const item of [value].flat()) { + if (item !== undefined) { + search.append(key, item); + } + } + } + return `/v1-alpha/projects/${slug}/development/active-days?${search}`; +} + +interface OpenApiSchema { + type?: string; + description?: string; + default?: unknown; + enum?: string[]; + required?: string[]; + properties?: Record; + items?: OpenApiSchema; +} + +interface OpenApiParameter { + name: string; + in: string; + required?: boolean; + description?: string; + schema: OpenApiSchema; +} + +interface OpenApiOperation { + tags?: string[]; + parameters?: OpenApiParameter[]; + responses: Record }>; +} + +interface OpenApiDoc { + paths: Record; +} + +let app: FastifyInstance; + +beforeAll(async () => { + vi.stubEnv('API_TB_HOST', tinybirdHost); + vi.stubEnv('API_TB_TOKEN', 'test-token'); + app = await buildApp(); + await app.ready(); +}); + +afterAll(async () => { + await app.close(); + vi.unstubAllEnvs(); +}); + +beforeEach(() => { + mockFetch.mockReset().mockImplementation(routeTinybird()); +}); + +const get = (path: string) => app.inject({ method: 'GET', url: path }); + +describe('GET /v1-alpha/projects/{slug}/development/active-days (AC1)', () => { + it('returns the active days summary, the daily average and one bucket per granularity step', async () => { + const res = await get(url()); + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual(expectedBody); + }); + + it('returns only the documented keys, dropping extra pipe fields and the day index', async () => { + mockFetch.mockImplementation( + routeTinybird({ + current: [{ ...currentRow, avgContributions: 1, extra: 'x' }], + series: seriesRows.map((row, index) => ({ ...row, day: index + 1, extra: 'x' })), + }), + ); + const res = await get(url()); + expect(res.statusCode).toBe(200); + const body = res.json<{ data: object[] }>(); + expect(Object.keys(body).sort()).toEqual(['avgContributionsPerDay', 'data', 'summary']); + for (const bucket of body.data) { + expect(Object.keys(bucket).sort()).toEqual(['contributions', 'endDate', 'startDate']); + } + }); +}); + +describe('Tinybird calls (AC2)', () => { + it('makes three active_days calls with the slug as project and the shared filters', async () => { + await get(url({ repos: [k8sRepo, websiteRepo] })); + const calls = pipeCalls(); + expect(calls).toHaveLength(3); + for (const call of calls) { + expect(call.origin).toBe(tinybirdHost); + expect(call.searchParams.get('project')).toBe('kubernetes'); + expect(call.searchParams.get('repos')).toBe(`${k8sRepo},${websiteRepo}`); + expect(call.searchParams.get('includeCodeContributions')).toBe('true'); + expect(call.searchParams.get('includeCollaborations')).toBe('false'); + } + }); + + it('sends the current range twice, once with the granularity, and the previous range once', async () => { + await get(url()); + const ranges = pipeCalls().map((call) => [ + call.searchParams.get('startDate'), + call.searchParams.get('endDate'), + call.searchParams.get('granularity'), + ]); + expect(ranges).toHaveLength(3); + expect(ranges).toEqual( + expect.arrayContaining([ + [atMidnight(startDate), atMidnight(endDate), null], + [atMidnight(startDate), atMidnight(endDate), 'monthly'], + [atMidnight(previousStart), atMidnight(previousEnd), null], + ]), + ); + }); + + it('omits repos from the pipe calls when the caller sends none', async () => { + await get(url()); + const calls = pipeCalls(); + expect(calls).toHaveLength(3); + for (const call of calls) { + expect(call.searchParams.has('repos')).toBe(false); + } + }); + + it('drops an empty repos value instead of sending it to the pipe as a filter', async () => { + await get(url({ repos: '' })); + const calls = pipeCalls(); + expect(calls).toHaveLength(3); + for (const call of calls) { + expect(call.searchParams.has('repos')).toBe(false); + } + }); + + it('issues the three pipe calls concurrently', async () => { + let inFlight = 0; + let maxInFlight = 0; + const respond = routeTinybird(); + mockFetch.mockImplementation(async (input: unknown) => { + if (new URL(String(input)).pathname === bucketPath) { + return respond(input); + } + inFlight += 1; + maxInFlight = Math.max(maxInFlight, inFlight); + await new Promise((resolve) => setTimeout(resolve, 5)); + inFlight -= 1; + return respond(input); + }); + const res = await get(url()); + expect(res.statusCode).toBe(200); + expect(maxInFlight).toBe(3); + }); +}); + +describe('includeCollaborations (AC3)', () => { + it('passes includeCollaborations=true to all three pipe calls', async () => { + const res = await get(url({ includeCollaborations: 'true' })); + expect(res.statusCode).toBe(200); + const calls = pipeCalls(); + expect(calls).toHaveLength(3); + for (const call of calls) { + expect(call.searchParams.get('includeCollaborations')).toBe('true'); + expect(call.searchParams.get('includeCodeContributions')).toBe('true'); + } + }); + + it('rejects a value that is not a boolean', async () => { + const res = await get(url({ includeCollaborations: 'maybe' })); + expect(res.statusCode).toBe(400); + expect(mockFetch).not.toHaveBeenCalled(); + }); +}); + +describe('percentageChange (AC4)', () => { + it('is negative when the current period has fewer active days', async () => { + mockFetch.mockImplementation( + routeTinybird({ + current: [{ activeDaysCount: 50, avgContributionsPerDay: 3 }], + previous: [{ activeDaysCount: 100, avgContributionsPerDay: 4 }], + }), + ); + const res = await get(url()); + expect(res.json().summary).toMatchObject({ + current: 50, + previous: 100, + percentageChange: -50, + changeValue: -50, + }); + }); + + it('is null when the previous period had no active days and the current one has some', async () => { + mockFetch.mockImplementation( + routeTinybird({ previous: [{ activeDaysCount: 0, avgContributionsPerDay: null }] }), + ); + const res = await get(url()); + expect(res.statusCode).toBe(200); + expect(res.json().summary).toMatchObject({ current: 80, previous: 0, percentageChange: null }); + }); +}); + +describe('empty results (AC5)', () => { + it('returns zeros and an empty series when the pipe has no rows', async () => { + mockFetch.mockImplementation(routeTinybird({ current: [], previous: [], series: [] })); + const res = await get(url()); + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual(zeroBody); + }); + + it('returns 0 for a null avgContributionsPerDay', async () => { + mockFetch.mockImplementation( + routeTinybird({ current: [{ activeDaysCount: 0, avgContributionsPerDay: null }] }), + ); + const res = await get(url()); + expect(res.statusCode).toBe(200); + expect(res.json().avgContributionsPerDay).toBe(0); + }); +}); + +describe('unknown slug (AC6)', () => { + beforeEach(() => { + // The Tinybird client warns about the missing bucket; keep the test output readable. + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('returns 200 with zeros when the bucket lookup has no row for the slug', async () => { + mockFetch.mockImplementation(routeTinybird({ bucket: [] })); + const res = await get(url({}, 'no-such-project')); + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual(zeroBody); + }); +}); + +describe('request validation (AC7)', () => { + it('rejects a timestamp in startDate', async () => { + const res = await get(url({ startDate: '2025-01-01T00:00:00Z' })); + expect(res.statusCode).toBe(400); + }); + + it('rejects an inverted range with invalid_request before calling Tinybird', async () => { + const res = await get(url({ startDate: endDate, endDate: startDate })); + expect(res.statusCode).toBe(400); + expect(res.json().code).toBe('invalid_request'); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('rejects a missing granularity', async () => { + const res = await get(url({ granularity: undefined })); + expect(res.statusCode).toBe(400); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('rejects granularity=hourly', async () => { + const res = await get(url({ granularity: 'hourly' })); + expect(res.statusCode).toBe(400); + }); + + it('defaults the range to 2010-01-01 through today when both dates are omitted', async () => { + const today = new Date().toISOString().slice(0, 10); + const res = await get(url({ startDate: undefined, endDate: undefined })); + expect(res.statusCode).toBe(200); + expect(res.json().summary).toMatchObject({ + periodFrom: isoDay('2010-01-01'), + periodTo: isoDay(today), + }); + const series = pipeCalls().find((call) => call.searchParams.has('granularity')); + expect(series?.searchParams.get('startDate')).toBe(atMidnight('2010-01-01')); + expect(series?.searchParams.get('endDate')).toBe(atMidnight(today)); + }); +}); + +describe('Tinybird failures (AC8)', () => { + const upstreamDetail = 'tinybird internal detail'; + const failing = (status: number, statusText: string, path = pipePath) => { + const respond = routeTinybird(); + return async (input: unknown) => + new URL(String(input)).pathname === path + ? new Response(upstreamDetail, { status, statusText }) + : respond(input); + }; + + beforeEach(() => { + // The Tinybird client logs every failed request; keep the test output readable. + vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + // 404 is in the table because only the client's bucket miss may read as empty data; a 404 + // from the pipe itself is an outage. + it.each([ + [500, 'Internal Server Error'], + [404, 'Not Found'], + [401, 'Unauthorized'], + [429, 'Too Many Requests'], + ])('maps a pipe %i to 503 upstream_unavailable', async (status, statusText) => { + mockFetch.mockImplementation(failing(status, statusText)); + const res = await get(url()); + expect(res.statusCode).toBe(503); + expect(res.json().code).toBe('upstream_unavailable'); + expect(res.body).not.toContain(upstreamDetail); + }); + + it('maps a failing bucket lookup to 503 upstream_unavailable', async () => { + mockFetch.mockImplementation(failing(500, 'Internal Server Error', bucketPath)); + const res = await get(url()); + expect(res.statusCode).toBe(503); + expect(res.json().code).toBe('upstream_unavailable'); + expect(res.body).not.toContain(upstreamDetail); + }); + + it('maps a network error to 503 upstream_unavailable', async () => { + mockFetch.mockRejectedValue(new TypeError(`fetch failed: ${upstreamDetail}`)); + const res = await get(url()); + expect(res.statusCode).toBe(503); + expect(res.json().code).toBe('upstream_unavailable'); + expect(res.body).not.toContain(upstreamDetail); + }); + + it('maps a response that is not JSON to 503 upstream_unavailable', async () => { + mockFetch.mockImplementation(async () => new Response('oops', { status: 200 })); + const res = await get(url()); + expect(res.statusCode).toBe(503); + expect(res.json().code).toBe('upstream_unavailable'); + }); +}); + +describe('caching headers (AC9)', () => { + it('sets Cache-Control: private, max-age=0 on a successful response', async () => { + const res = await get(url()); + expect(res.statusCode).toBe(200); + expect(res.headers['cache-control']).toBe('private, max-age=0'); + }); + + it('sets the same header on a 503', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + mockFetch.mockRejectedValue(new TypeError('fetch failed')); + const res = await get(url()); + expect(res.statusCode).toBe(503); + expect(res.headers['cache-control']).toBe('private, max-age=0'); + vi.restoreAllMocks(); + }); +}); + +describe('OpenAPI (AC10)', () => { + async function getOperation(): Promise { + const res = await get('/v1-alpha/openapi.json'); + expect(res.statusCode).toBe(200); + return res.json().paths[specPath]?.get; + } + + it('lists the route in /v1-alpha/openapi.json tagged Development', async () => { + const operation = await getOperation(); + expect(operation?.tags).toEqual(['Development']); + }); + + it('documents every field of the 200 response', async () => { + const operation = await getOperation(); + const schema = operation?.responses['200']?.content['application/json']?.schema; + const fields = ['summary', 'avgContributionsPerDay', 'data']; + expect(schema?.required).toEqual(expect.arrayContaining(fields)); + for (const field of fields) { + expect(schema?.properties?.[field]?.description, `${field} has no description`).toBeTruthy(); + } + const bucket = schema?.properties?.data?.items; + const bucketFields = ['startDate', 'endDate', 'contributions']; + expect(bucket?.required).toEqual(expect.arrayContaining(bucketFields)); + for (const field of bucketFields) { + expect(bucket?.properties?.[field]?.description, `${field} has no description`).toBeTruthy(); + } + }); + + it('documents the query params, with granularity required and includeCollaborations defaulting to false', async () => { + const operation = await getOperation(); + // The operation also lists the slug path parameter; only the query params are under test. + const params = new Map( + operation?.parameters + ?.filter((param) => param.in === 'query') + .map((param) => [param.name, param]), + ); + expect([...params.keys()].sort()).toEqual( + ['endDate', 'granularity', 'includeCollaborations', 'repos', 'startDate'].sort(), + ); + const granularity = params.get('granularity'); + expect(granularity?.required).toBe(true); + expect(granularity?.schema.enum).toEqual(['daily', 'weekly', 'monthly', 'quarterly', 'yearly']); + expect(granularity?.description ?? granularity?.schema.description).toBeTruthy(); + const includeCollaborations = params.get('includeCollaborations'); + expect(includeCollaborations?.required).toBeFalsy(); + expect(includeCollaborations?.schema).toMatchObject({ type: 'boolean', default: false }); + expect( + includeCollaborations?.description ?? includeCollaborations?.schema.description, + ).toBeTruthy(); + }); + + it('keeps the route out of /v1', async () => { + const spec = await get('/v1/openapi.json'); + expect(Object.keys(spec.json().paths)).not.toContain(specPath); + const res = await get('/v1/projects/kubernetes/development/active-days?granularity=monthly'); + expect(res.statusCode).toBe(404); + }); +}); From df2ce449150f717a9a4dff52f819295a426d0d3e Mon Sep 17 00:00:00 2001 From: anilb Date: Mon, 21 Sep 2026 13:45:27 +0200 Subject: [PATCH 2/3] fix: describe active days summary fields IN-1334 Signed-off-by: anilb --- .../v1-alpha/development/active-days.ts | 23 +++++++--- api/tests/development-active-days.test.ts | 46 ++++++++++++++----- 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/api/src/versions/v1-alpha/development/active-days.ts b/api/src/versions/v1-alpha/development/active-days.ts index 3d174f7fc..823e2bad7 100644 --- a/api/src/versions/v1-alpha/development/active-days.ts +++ b/api/src/versions/v1-alpha/development/active-days.ts @@ -56,12 +56,23 @@ const ActiveDaysBucket = Type.Object({ contributions: Type.Integer({ description: 'Number of contributions in the bucket (count).' }), }); -const ActiveDays = Type.Object({ - summary: { - ...PeriodSummary, - description: - 'Active days in the current period against the previous one. current, previous and changeValue are counts of days.', +// The shared PeriodSummary leaves current, previous and changeValue undescribed; the unit is +// per metric, so it is stated here. +const ActiveDaysSummary = Type.Object( + { + ...PeriodSummary.properties, + current: Type.Number({ description: 'Active days in the current period (count of days).' }), + previous: Type.Number({ description: 'Active days in the previous period (count of days).' }), + changeValue: Type.Number({ description: 'current minus previous (count of days).' }), + }, + { + title: 'PeriodSummary', + description: 'Active days in the current period against the previous one.', }, +); + +const ActiveDays = Type.Object({ + summary: ActiveDaysSummary, avgContributionsPerDay: Type.Number({ description: 'Average number of contributions per active day in the current period (count per day). 0 when there are no active days.', @@ -100,7 +111,7 @@ const activeDaysRoutes: FastifyPluginAsyncTypebox = async (scope) => { tags: ['Development'], summary: 'Get active days', description: - 'Returns the number of days with at least one development activity in the period against the previous period, the average contributions per active day, and the contributions per bucket. The previous period has the same length and ends the day before `startDate`.', + 'Returns the number of days with at least one development activity in the period against the previous period, the average contributions per active day, and the contributions per bucket. The previous period ends the day before `startDate` and covers the same calendar span as the current period, counted in whole months plus remaining days the way the Insights UI does, so its number of days can differ around month ends.', params: ProjectSlugParams, querystring: ActiveDaysQuery, response: { 200: ActiveDays }, diff --git a/api/tests/development-active-days.test.ts b/api/tests/development-active-days.test.ts index b566260b5..f5cd0567e 100644 --- a/api/tests/development-active-days.test.ts +++ b/api/tests/development-active-days.test.ts @@ -14,7 +14,8 @@ const specPath = '/v1-alpha/projects/{slug}/development/active-days'; const startDate = '2025-01-01'; const endDate = '2025-03-31'; -// getPreviousDates gives the previous period the same length, ending the day before startDate. +// getPreviousDates shifts the range back by its calendar span (2 months 30 days here), ending +// the day before startDate. const previousStart = '2024-10-01'; const previousEnd = '2024-12-31'; const atMidnight = (day: string) => `${day} 00:00:00`; @@ -367,16 +368,23 @@ describe('request validation (AC7)', () => { }); it('defaults the range to 2010-01-01 through today when both dates are omitted', async () => { - const today = new Date().toISOString().slice(0, 10); - const res = await get(url({ startDate: undefined, endDate: undefined })); - expect(res.statusCode).toBe(200); - expect(res.json().summary).toMatchObject({ - periodFrom: isoDay('2010-01-01'), - periodTo: isoDay(today), - }); - const series = pipeCalls().find((call) => call.searchParams.has('granularity')); - expect(series?.searchParams.get('startDate')).toBe(atMidnight('2010-01-01')); - expect(series?.searchParams.get('endDate')).toBe(atMidnight(today)); + // Only Date is faked, so the Tinybird client's real timers keep running and the request + // cannot straddle a UTC midnight between the handler and the assertion. + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(new Date('2025-09-21T12:00:00Z')); + try { + const res = await get(url({ startDate: undefined, endDate: undefined })); + expect(res.statusCode).toBe(200); + expect(res.json().summary).toMatchObject({ + periodFrom: isoDay('2010-01-01'), + periodTo: isoDay('2025-09-21'), + }); + const series = pipeCalls().find((call) => call.searchParams.has('granularity')); + expect(series?.searchParams.get('startDate')).toBe(atMidnight('2010-01-01')); + expect(series?.searchParams.get('endDate')).toBe(atMidnight('2025-09-21')); + } finally { + vi.useRealTimers(); + } }); }); @@ -482,6 +490,22 @@ describe('OpenAPI (AC10)', () => { for (const field of bucketFields) { expect(bucket?.properties?.[field]?.description, `${field} has no description`).toBeTruthy(); } + const summary = schema?.properties?.summary; + const summaryFields = [ + 'current', + 'previous', + 'percentageChange', + 'changeValue', + 'periodFrom', + 'periodTo', + ]; + expect(summary?.required).toEqual(expect.arrayContaining(summaryFields)); + for (const field of summaryFields) { + expect( + summary?.properties?.[field]?.description, + `summary.${field} has no description`, + ).toBeTruthy(); + } }); it('documents the query params, with granularity required and includeCollaborations defaulting to false', async () => { From 40d1111b3533c00ae32eb079737a67f3a471e566 Mon Sep 17 00:00:00 2001 From: anilb Date: Mon, 21 Sep 2026 16:47:10 +0200 Subject: [PATCH 3/3] fix: resolve bucket once, drop null buckets IN-1334 Signed-off-by: anilb --- .../v1-alpha/development/active-days.ts | 100 +++++++++------ api/tests/development-active-days.test.ts | 115 +++++++++++++++--- 2 files changed, 158 insertions(+), 57 deletions(-) diff --git a/api/src/versions/v1-alpha/development/active-days.ts b/api/src/versions/v1-alpha/development/active-days.ts index 823e2bad7..7b67ce82c 100644 --- a/api/src/versions/v1-alpha/development/active-days.ts +++ b/api/src/versions/v1-alpha/development/active-days.ts @@ -1,12 +1,9 @@ // Copyright (c) 2025 The Linux Foundation and each contributor. // SPDX-License-Identifier: MIT import type { FastifyPluginAsyncTypebox } from '@fastify/type-provider-typebox'; -import { - TinybirdProjectNotFoundError, - type TinybirdClient, - type TinybirdQuery, -} from '@lfx-insights/tinybird-client'; +import type { TinybirdQuery } from '@lfx-insights/tinybird-client'; import { Type } from '@sinclair/typebox'; +import type { FastifyBaseLogger } from 'fastify'; import { getTinybirdClient } from '../../../clients/tinybird.js'; import { UpstreamUnavailableError } from '../../../lib/errors.js'; import { getPreviousDates, toPeriodSummary, type DateRange } from '../../../lib/period.js'; @@ -24,12 +21,15 @@ interface SummaryRow { avgContributionsPerDay: number | null; } +// The pipe declares both bucket bounds as Nullable(Date). interface SeriesRow { - startDate: string; - endDate: string; - activityCount: number; + startDate: string | null; + endDate: string | null; + activityCount?: number | null; } +type BoundedSeriesRow = SeriesRow & { startDate: string; endDate: string }; + const ActiveDaysQuery = Type.Composite([ DateRangeQuery, Type.Object({ @@ -66,7 +66,7 @@ const ActiveDaysSummary = Type.Object( changeValue: Type.Number({ description: 'current minus previous (count of days).' }), }, { - title: 'PeriodSummary', + title: 'ActiveDaysSummary', description: 'Active days in the current period against the previous one.', }, ); @@ -78,7 +78,8 @@ const ActiveDays = Type.Object({ 'Average number of contributions per active day in the current period (count per day). 0 when there are no active days.', }), data: Type.Array(ActiveDaysBucket, { - description: 'One entry per granularity bucket in the current period.', + description: + 'One entry per granularity bucket in the current period. A bucket the pipe reports without both bounds is omitted.', }), }); @@ -87,19 +88,27 @@ const toTinybirdRange = (range: DateRange) => ({ startDate: `${range.startDate} 00:00:00`, endDate: `${range.endDate} 00:00:00`, }); -const toUtcDateTime = (day: string) => `${day}T00:00:00Z`; -async function fetchRows(client: TinybirdClient, params: TinybirdQuery): Promise { +// Tinybird returns bucket bounds as Date columns (YYYY-MM-DD). Keeping the day means a DateTime +// column formats the same way, and the shape matches periodFrom/periodTo from toPeriodSummary. +const toUtcMidnight = (day: string) => `${day.slice(0, 10)}T00:00:00Z`; + +// A bucket missing either bound has no place on the time axis and cannot meet the date-time +// contract, so it is left out instead of being formatted from null. +const hasBounds = (row: SeriesRow): row is BoundedSeriesRow => + typeof row.startDate === 'string' && typeof row.endDate === 'string'; + +// Every Tinybird failure becomes a 503, so Tinybird's own status never reaches the caller. +async function fromTinybird( + log: FastifyBaseLogger, + pipe: string, + call: () => Promise, +): Promise { try { - const { data } = await client.fetch(pipePath, params); - return data; + return await call(); } catch (err: unknown) { - // The client reports a slug with no bucket as not found. A metric endpoint answers an - // unknown project with empty data, which is why ProjectSlugParams lets any slug through. - if (err instanceof TinybirdProjectNotFoundError) { - return []; - } - throw err; + log.error({ err }, `Tinybird ${pipe} request failed`); + throw new UpstreamUnavailableError(); } } @@ -111,7 +120,7 @@ const activeDaysRoutes: FastifyPluginAsyncTypebox = async (scope) => { tags: ['Development'], summary: 'Get active days', description: - 'Returns the number of days with at least one development activity in the period against the previous period, the average contributions per active day, and the contributions per bucket. The previous period ends the day before `startDate` and covers the same calendar span as the current period, counted in whole months plus remaining days the way the Insights UI does, so its number of days can differ around month ends.', + 'Returns the number of days with at least one development activity in the period against the previous period, the average contributions per active day, and the contributions per bucket. The previous period ends the day before `startDate` and covers the same calendar span as the current period, counted in whole months plus remaining days the way the Insights UI does, so its number of days can differ around month ends. Without dates the period runs from 2010-01-01 to today. An unknown project returns zeros and an empty `data` list.', params: ProjectSlugParams, querystring: ActiveDaysQuery, response: { 200: ActiveDays }, @@ -127,32 +136,47 @@ const activeDaysRoutes: FastifyPluginAsyncTypebox = async (scope) => { includeCollaborations = false, } = request.query; const dates = getPreviousDates(startDate, endDate); + const log = request.log; + + // A missing API_TB_* variable throws here, outside fromTinybird, so it stays a 500. + const client = getTinybirdClient(); + + // Resolving the bucket here saves the client one lookup per pipe call. A project without a + // bucket is unknown to Tinybird, which metric endpoints answer with empty data. + const bucketId = await fromTinybird(log, 'project_buckets', () => + client.getBucketIdForProject(slug), + ); + if (bucketId === null) { + return { + summary: toPeriodSummary(0, 0, dates.current), + avgContributionsPerDay: 0, + data: [], + }; + } // Ajv turns a bare `repos=` into [''] and the client sends an empty array as `repos=`, // which the pipe would apply as a filter. The Nuxt handler drops an empty value too. const repoFilter = repos?.filter(Boolean); - const shared = { + const shared: TinybirdQuery = { project: slug, + bucketId, repos: repoFilter?.length ? repoFilter : undefined, includeCodeContributions: true, includeCollaborations, }; const currentRange = toTinybirdRange(dates.current); const previousRange = toTinybirdRange(dates.previous); + const activeDays = (query: TinybirdQuery) => + fromTinybird(log, 'active_days', () => client.fetch(pipePath, query)); - // A missing API_TB_* variable throws here, outside the catch, so it stays a 500. - const client = getTinybirdClient(); const [currentRows, previousRows, seriesRows] = await Promise.all([ - fetchRows(client, { ...shared, ...currentRange }), - fetchRows(client, { ...shared, ...previousRange }), - fetchRows(client, { ...shared, ...currentRange, granularity }), - ]).catch((err: unknown) => { - request.log.error({ err }, 'Tinybird active_days request failed'); - throw new UpstreamUnavailableError(); - }); - - const current = currentRows[0]; - const previous = previousRows[0]; + activeDays({ ...shared, ...currentRange }), + activeDays({ ...shared, ...previousRange }), + activeDays({ ...shared, ...currentRange, granularity }), + ]); + + const current = currentRows.data[0]; + const previous = previousRows.data[0]; return { summary: toPeriodSummary( current?.activeDaysCount ?? 0, @@ -160,10 +184,10 @@ const activeDaysRoutes: FastifyPluginAsyncTypebox = async (scope) => { dates.current, ), avgContributionsPerDay: current?.avgContributionsPerDay ?? 0, - data: seriesRows.map((row) => ({ - startDate: toUtcDateTime(row.startDate), - endDate: toUtcDateTime(row.endDate), - contributions: row.activityCount, + data: seriesRows.data.filter(hasBounds).map((row) => ({ + startDate: toUtcMidnight(row.startDate), + endDate: toUtcMidnight(row.endDate), + contributions: row.activityCount ?? 0, })), }; }, diff --git a/api/tests/development-active-days.test.ts b/api/tests/development-active-days.test.ts index f5cd0567e..3b4afc875 100644 --- a/api/tests/development-active-days.test.ts +++ b/api/tests/development-active-days.test.ts @@ -80,8 +80,8 @@ interface PipeRows { series?: object[]; } -// The client resolves the slug to a bucket before every pipe call. The series call is the one -// that carries granularity; the two summary calls differ by their startDate. +// The handler resolves the slug to a bucket once, then makes the three pipe calls. The series +// call is the one that carries granularity; the two summary calls differ by their startDate. const routeTinybird = ({ bucket = [{ bucketId: 7 }], @@ -101,10 +101,11 @@ const routeTinybird = return tinybirdResponse(isPrevious ? previous : current); }; -const pipeCalls = () => +const callsTo = (path: string) => mockFetch.mock.calls .map((call) => new URL(String(call[0]))) - .filter((url) => url.pathname === pipePath); + .filter((url) => url.pathname === path); +const pipeCalls = () => callsTo(pipePath); const defaultQuery = { startDate, endDate, granularity: 'monthly' }; @@ -206,6 +207,30 @@ describe('Tinybird calls (AC2)', () => { } }); + it('resolves the project bucket once and sends its id on every pipe call', async () => { + await get(url()); + const lookups = callsTo(bucketPath); + expect(lookups).toHaveLength(1); + expect(lookups[0]?.searchParams.get('project')).toBe('kubernetes'); + const calls = pipeCalls(); + expect(calls).toHaveLength(3); + for (const call of calls) { + expect(call.searchParams.get('bucketId')).toBe('7'); + } + }); + + it('treats bucket id 0 as a real bucket and forwards it', async () => { + mockFetch.mockImplementation(routeTinybird({ bucket: [{ bucketId: 0 }] })); + const res = await get(url()); + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual(expectedBody); + const calls = pipeCalls(); + expect(calls).toHaveLength(3); + for (const call of calls) { + expect(call.searchParams.get('bucketId')).toBe('0'); + } + }); + it('sends the current range twice, once with the granularity, and the previous range once', async () => { await get(url()); const ranges = pipeCalls().map((call) => [ @@ -323,6 +348,37 @@ describe('empty results (AC5)', () => { expect(res.statusCode).toBe(200); expect(res.json().avgContributionsPerDay).toBe(0); }); + + // The pipe types both bucket bounds Nullable(Date); a null must never reach the formatter. + it('drops a series row with a null bucket bound instead of formatting it', async () => { + mockFetch.mockImplementation( + routeTinybird({ + series: [ + { startDate: null, endDate: null, activityCount: 5 }, + { startDate: '2025-01-01', endDate: '2025-01-31', activityCount: 698 }, + { startDate: '2025-02-01', endDate: null, activityCount: 9 }, + { startDate: null, endDate: '2025-03-31', activityCount: 4 }, + ], + }), + ); + const res = await get(url()); + expect(res.statusCode).toBe(200); + expect(res.json().data).toEqual([ + { startDate: isoDay('2025-01-01'), endDate: isoDay('2025-01-31'), contributions: 698 }, + ]); + expect(res.body).not.toContain('nullT00:00:00Z'); + }); + + it('reports 0 contributions for a bucket row without an activityCount', async () => { + mockFetch.mockImplementation( + routeTinybird({ series: [{ startDate: '2025-01-01', endDate: '2025-01-31' }] }), + ); + const res = await get(url()); + expect(res.statusCode).toBe(200); + expect(res.json().data).toEqual([ + { startDate: isoDay('2025-01-01'), endDate: isoDay('2025-01-31'), contributions: 0 }, + ]); + }); }); describe('unknown slug (AC6)', () => { @@ -335,11 +391,14 @@ describe('unknown slug (AC6)', () => { vi.restoreAllMocks(); }); - it('returns 200 with zeros when the bucket lookup has no row for the slug', async () => { + it('returns 200 with zeros after only the bucket lookup when the slug has no bucket', async () => { mockFetch.mockImplementation(routeTinybird({ bucket: [] })); const res = await get(url({}, 'no-such-project')); expect(res.statusCode).toBe(200); expect(res.json()).toEqual(zeroBody); + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(callsTo(bucketPath)[0]?.searchParams.get('project')).toBe('no-such-project'); + expect(pipeCalls()).toHaveLength(0); }); }); @@ -390,13 +449,15 @@ describe('request validation (AC7)', () => { describe('Tinybird failures (AC8)', () => { const upstreamDetail = 'tinybird internal detail'; - const failing = (status: number, statusText: string, path = pipePath) => { + // Fails only the calls to `path`; every other Tinybird call answers normally. + const failing = (respondWith: () => Response | Promise, path = pipePath) => { const respond = routeTinybird(); return async (input: unknown) => - new URL(String(input)).pathname === path - ? new Response(upstreamDetail, { status, statusText }) - : respond(input); + new URL(String(input)).pathname === path ? respondWith() : respond(input); }; + const httpError = (status: number, statusText: string) => () => + new Response(upstreamDetail, { status, statusText }); + const networkError = () => Promise.reject(new TypeError(`fetch failed: ${upstreamDetail}`)); beforeEach(() => { // The Tinybird client logs every failed request; keep the test output readable. @@ -408,42 +469,56 @@ describe('Tinybird failures (AC8)', () => { vi.restoreAllMocks(); }); - // 404 is in the table because only the client's bucket miss may read as empty data; a 404 - // from the pipe itself is an outage. + // 404 is in the table because the empty-data answer comes from the handler's null bucket id; + // a 404 from the pipe itself is an outage. it.each([ [500, 'Internal Server Error'], [404, 'Not Found'], [401, 'Unauthorized'], [429, 'Too Many Requests'], ])('maps a pipe %i to 503 upstream_unavailable', async (status, statusText) => { - mockFetch.mockImplementation(failing(status, statusText)); + mockFetch.mockImplementation(failing(httpError(status, statusText))); const res = await get(url()); expect(res.statusCode).toBe(503); expect(res.json().code).toBe('upstream_unavailable'); expect(res.body).not.toContain(upstreamDetail); }); - it('maps a failing bucket lookup to 503 upstream_unavailable', async () => { - mockFetch.mockImplementation(failing(500, 'Internal Server Error', bucketPath)); + it('maps a failing bucket lookup to 503 upstream_unavailable without calling the pipe', async () => { + mockFetch.mockImplementation(failing(httpError(500, 'Internal Server Error'), bucketPath)); const res = await get(url()); expect(res.statusCode).toBe(503); expect(res.json().code).toBe('upstream_unavailable'); expect(res.body).not.toContain(upstreamDetail); + expect(pipeCalls()).toHaveLength(0); }); - it('maps a network error to 503 upstream_unavailable', async () => { - mockFetch.mockRejectedValue(new TypeError(`fetch failed: ${upstreamDetail}`)); + it('maps a pipe network error to 503 upstream_unavailable', async () => { + mockFetch.mockImplementation(failing(networkError)); const res = await get(url()); expect(res.statusCode).toBe(503); expect(res.json().code).toBe('upstream_unavailable'); expect(res.body).not.toContain(upstreamDetail); + // The client retries a network error, so only the path matters: the lookup succeeded and + // the failure came from the pipe. + expect(pipeCalls()).not.toHaveLength(0); }); - it('maps a response that is not JSON to 503 upstream_unavailable', async () => { - mockFetch.mockImplementation(async () => new Response('oops', { status: 200 })); + it('maps a bucket lookup network error to 503 upstream_unavailable', async () => { + mockFetch.mockImplementation(failing(networkError, bucketPath)); const res = await get(url()); expect(res.statusCode).toBe(503); expect(res.json().code).toBe('upstream_unavailable'); + expect(res.body).not.toContain(upstreamDetail); + expect(pipeCalls()).toHaveLength(0); + }); + + it('maps a pipe response that is not JSON to 503 upstream_unavailable', async () => { + mockFetch.mockImplementation(failing(() => new Response('oops', { status: 200 }))); + const res = await get(url()); + expect(res.statusCode).toBe(503); + expect(res.json().code).toBe('upstream_unavailable'); + expect(res.body).not.toContain('oops'); }); }); @@ -533,7 +608,9 @@ describe('OpenAPI (AC10)', () => { it('keeps the route out of /v1', async () => { const spec = await get('/v1/openapi.json'); - expect(Object.keys(spec.json().paths)).not.toContain(specPath); + expect(Object.keys(spec.json().paths)).not.toContain( + '/v1/projects/{slug}/development/active-days', + ); const res = await get('/v1/projects/kubernetes/development/active-days?granularity=monthly'); expect(res.statusCode).toBe(404); });