diff --git a/src/app/__tests__/api/osrm-routing-validation.test.ts b/src/app/__tests__/api/osrm-routing-validation.test.ts index df812d60..3eb09971 100644 --- a/src/app/__tests__/api/osrm-routing-validation.test.ts +++ b/src/app/__tests__/api/osrm-routing-validation.test.ts @@ -13,6 +13,7 @@ const buildRequest = (query: string): NextRequest => describe('OSRM routing proxy validation', () => { const originalAdminDomain = process.env.ADMIN_DOMAIN; + const originalAdminProxySecret = process.env.ADMIN_PROXY_SECRET; afterEach(() => { jest.restoreAllMocks(); @@ -21,6 +22,11 @@ describe('OSRM routing proxy validation', () => { } else { process.env.ADMIN_DOMAIN = originalAdminDomain; } + if (originalAdminProxySecret === undefined) { + delete process.env.ADMIN_PROXY_SECRET; + } else { + process.env.ADMIN_PROXY_SECRET = originalAdminProxySecret; + } }); it('rejects requests outside the admin domain before proxying upstream', async () => { @@ -59,8 +65,53 @@ describe('OSRM routing proxy validation', () => { expect(fetchSpy).not.toHaveBeenCalled(); }); - it('accepts forwarded admin hosts only from local proxy hops', async () => { + it('rejects forwarded admin hosts from local proxy hops without the proxy secret', async () => { + process.env.ADMIN_DOMAIN = 'admin.example.test'; + const fetchSpy = jest.spyOn(global, 'fetch'); + + const response = await GET( + new NextRequest( + 'http://127.0.0.1:3000/api/routing/osrm?profile=car&fromLat=51.5&fromLng=-0.1&toLat=48.8&toLng=2.3', + { + headers: { + host: '127.0.0.1:3000', + 'x-forwarded-host': 'admin.example.test' + } + } + ) + ); + + await expect(response.json()).resolves.toEqual({ error: 'Admin domain required' }); + expect(response.status).toBe(403); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('rejects forwarded admin hosts from local proxy hops with a mismatched proxy secret', async () => { process.env.ADMIN_DOMAIN = 'admin.example.test'; + process.env.ADMIN_PROXY_SECRET = 'proxy-secret'; + const fetchSpy = jest.spyOn(global, 'fetch'); + + const response = await GET( + new NextRequest( + 'http://127.0.0.1:3000/api/routing/osrm?profile=car&fromLat=51.5&fromLng=-0.1&toLat=48.8&toLng=2.3', + { + headers: { + host: '127.0.0.1:3000', + 'x-forwarded-host': 'admin.example.test', + 'x-travel-tracker-admin-proxy-secret': 'wrong-secret' + } + } + ) + ); + + await expect(response.json()).resolves.toEqual({ error: 'Admin domain required' }); + expect(response.status).toBe(403); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('accepts forwarded admin hosts only from trusted local proxy hops', async () => { + process.env.ADMIN_DOMAIN = 'admin.example.test'; + process.env.ADMIN_PROXY_SECRET = 'proxy-secret'; const fetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue( new Response(JSON.stringify({ routes: [] }), { status: 200, @@ -74,7 +125,8 @@ describe('OSRM routing proxy validation', () => { { headers: { host: '127.0.0.1:3000', - 'x-forwarded-host': 'admin.example.test' + 'x-forwarded-host': 'admin.example.test', + 'x-travel-tracker-admin-proxy-secret': 'proxy-secret' } } ) diff --git a/src/app/__tests__/api/travel-data-auth-cache.test.ts b/src/app/__tests__/api/travel-data-auth-cache.test.ts index 01e520b7..1f52cd09 100644 --- a/src/app/__tests__/api/travel-data-auth-cache.test.ts +++ b/src/app/__tests__/api/travel-data-auth-cache.test.ts @@ -260,6 +260,54 @@ describe('travel-data API auth and cache boundary', () => { expect(mockUpdateTravelData).not.toHaveBeenCalled(); }); + it('normalizes null route point geometry on batch route-point PATCH updates', async () => { + mockIsAdminDomain.mockResolvedValue(true); + mockLoadUnifiedTripData.mockResolvedValue({ + ...buildTrip(), + travelData: { + ...buildTrip().travelData, + routes: [ + { + id: 'route-1', + from: 'A', + to: 'B', + routePoints: [[52.52, 13.405]] + } + ] + } + } as any); // eslint-disable-line @typescript-eslint/no-explicit-any + + const response = await PATCH( + new NextRequest('https://admin.example.test/api/travel-data?id=trip-1', { + method: 'PATCH', + headers: { host: 'admin.example.test' }, + body: JSON.stringify({ + batchRouteUpdate: [ + { + routeId: 'route-1', + routePoints: null + } + ] + }) + }) + ); + const result = await response.json(); + + expect(response.status).toBe(200); + expect(result).toEqual({ + success: true, + message: '1 route(s) updated successfully' + }); + expect(mockUpdateTravelData).toHaveBeenCalledWith('trip-1', expect.objectContaining({ + routes: [ + expect.objectContaining({ + id: 'route-1', + routePoints: [] + }) + ] + })); + }); + it('coerces stale stored route transport types before validating unrelated delta PATCH updates', async () => { mockIsAdminDomain.mockResolvedValue(true); mockLoadUnifiedTripData.mockResolvedValue({ diff --git a/src/app/__tests__/integration/cost-tracking-validation.integration.test.ts b/src/app/__tests__/integration/cost-tracking-validation.integration.test.ts index a9b2873b..9ef27b7d 100644 --- a/src/app/__tests__/integration/cost-tracking-validation.integration.test.ts +++ b/src/app/__tests__/integration/cost-tracking-validation.integration.test.ts @@ -13,6 +13,7 @@ import { Expense, BudgetItem, YnabCategoryMapping, YnabTransaction } from '@/app import { writeFile, unlink } from 'fs/promises'; import { join } from 'path'; import { getDataDir } from '@/app/lib/dataDirectory'; +import { createTransactionHash } from '@/app/lib/ynabUtils'; // Mock the admin domain check jest.mock('@/app/lib/server-domains', () => ({ @@ -100,7 +101,8 @@ describe('Cost Tracking API Validation Integration Tests', () => { }); beforeEach(() => { - jest.clearAllMocks(); + jest.resetAllMocks(); + (mockIsAdminDomain as jest.Mock).mockResolvedValue(true); }); describe('Cost Tracking Main Endpoint', () => { @@ -180,7 +182,7 @@ describe('Cost Tracking API Validation Integration Tests', () => { .mockResolvedValueOnce(mockTrip2); const request = new NextRequest('http://localhost/api/cost-tracking/list'); - const response = await costTrackingListGET(); + const response = await costTrackingListGET(request); const result = await response.json(); expect(response.status).toBe(200); @@ -298,6 +300,116 @@ describe('Cost Tracking API Validation Integration Tests', () => { }) ); }); + + it('skips invalid YNAB dates instead of persisting null expense dates', async () => { + const mockData = createMockUnifiedTripData(false); + const updatedData = { ...mockData, updatedAt: new Date().toISOString() }; + mockLoadUnifiedTripData.mockResolvedValue(mockData); + mockUpdateCostData.mockResolvedValue(updatedData); + + const transaction: YnabTransaction = { + Date: '31/02/2025', + Payee: 'Impossible Date Merchant', + Category: 'Food', + Outflow: '€25.00', + Inflow: '', + Memo: 'Invalid date' + }; + const tempFileId = await createTempYnabFile([transaction]); + + const mappings: YnabCategoryMapping[] = [{ + ynabCategory: 'Food', + mappingType: 'country', + countryName: 'Germany' + }]; + + const request = new NextRequest(`http://localhost/api/cost-tracking/${mockTripId}/ynab-process`, { + method: 'POST', + body: JSON.stringify({ + action: 'import', + tempFileId, + mappings, + selectedTransactions: [{ + transactionHash: createTransactionHash(transaction), + transactionSourceIndex: 0, + expenseCategory: 'Food' + }] + }) + }); + + const response = await ynabProcessPOST(request, { params: Promise.resolve({ id: mockTripId }) }); + const result = await response.json(); + + expect(response.status).toBe(200); + expect(result.importedCount).toBe(0); + expect(result.skippedCount).toBe(1); + expect(mockUpdateCostData).toHaveBeenCalledWith( + mockTripId, + expect.objectContaining({ + expenses: [] + }) + ); + }); + + it('imports an instance-keyed duplicate even when a legacy base hash exists', async () => { + const transaction: YnabTransaction = { + Date: '01/05/2024', + Payee: 'Duplicate Merchant', + Category: 'Food', + Outflow: '€30.00', + Inflow: '', + Memo: 'Duplicate' + }; + const transactionHash = createTransactionHash(transaction); + const mockData = createMockUnifiedTripData(false); + mockData.costData!.ynabImportData = { + mappings: [], + importedTransactionHashes: [transactionHash] + }; + const updatedData = { ...mockData, updatedAt: new Date().toISOString() }; + mockLoadUnifiedTripData.mockResolvedValue(mockData); + mockUpdateCostData.mockResolvedValue(updatedData); + const tempFileId = await createTempYnabFile([transaction, transaction]); + + const mappings: YnabCategoryMapping[] = [{ + ynabCategory: 'Food', + mappingType: 'country', + countryName: 'Germany' + }]; + + const request = new NextRequest(`http://localhost/api/cost-tracking/${mockTripId}/ynab-process`, { + method: 'POST', + body: JSON.stringify({ + action: 'import', + tempFileId, + mappings, + selectedTransactions: [{ + transactionHash, + transactionId: `${transactionHash}-1`, + transactionSourceIndex: 1, + expenseCategory: 'Food' + }] + }) + }); + + const response = await ynabProcessPOST(request, { params: Promise.resolve({ id: mockTripId }) }); + const result = await response.json(); + + expect(response.status).toBe(200); + expect(result.importedCount).toBe(1); + expect(mockUpdateCostData).toHaveBeenCalledWith( + mockTripId, + expect.objectContaining({ + expenses: expect.arrayContaining([ + expect.objectContaining({ + id: `ynab-${transactionHash}-1`, + hash: transactionHash, + description: 'Duplicate Merchant' + }) + ]) + }) + ); + }); }); describe('Validation Endpoint', () => { diff --git a/src/app/__tests__/unit/MultiRouteLinkManager.test.tsx b/src/app/__tests__/unit/MultiRouteLinkManager.test.tsx new file mode 100644 index 00000000..3903b7bc --- /dev/null +++ b/src/app/__tests__/unit/MultiRouteLinkManager.test.tsx @@ -0,0 +1,34 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; + +import MultiRouteLinkManager from '@/app/admin/components/MultiRouteLinkManager'; + +describe('MultiRouteLinkManager', () => { + it('notifies parent when the last existing link is removed', async () => { + const onLinksChange = jest.fn(); + + render( + + ); + + await screen.findByText('A to B'); + onLinksChange.mockClear(); + + fireEvent.click(screen.getByTitle('Remove link')); + + await waitFor(() => { + expect(onLinksChange).toHaveBeenCalledWith([]); + }); + }); +}); diff --git a/src/app/__tests__/unit/TravelItemSelector.loadFailure.test.tsx b/src/app/__tests__/unit/TravelItemSelector.loadFailure.test.tsx new file mode 100644 index 00000000..f790d839 --- /dev/null +++ b/src/app/__tests__/unit/TravelItemSelector.loadFailure.test.tsx @@ -0,0 +1,42 @@ +import { render, waitFor } from '@testing-library/react'; + +import TravelItemSelector from '@/app/admin/components/TravelItemSelector'; + +jest.mock('@/app/hooks/useExpenseLinks', () => ({ + useExpenseLinks: () => ({ + expenseLinks: [], + isLoading: false, + error: null + }) +})); + +describe('TravelItemSelector load failures', () => { + const originalFetch = global.fetch; + + afterEach(() => { + global.fetch = originalFetch; + jest.restoreAllMocks(); + }); + + it('clears the current reference when travel item loading fails', async () => { + const onReferenceChange = jest.fn(); + global.fetch = jest.fn().mockRejectedValue(new Error('network down')) as jest.Mock; + + render( + + ); + + await waitFor(() => { + expect(onReferenceChange).toHaveBeenCalledWith(undefined); + }); + }); +}); diff --git a/src/app/__tests__/unit/weatherService.test.ts b/src/app/__tests__/unit/weatherService.test.ts index 74ea1a60..b71c4011 100644 --- a/src/app/__tests__/unit/weatherService.test.ts +++ b/src/app/__tests__/unit/weatherService.test.ts @@ -14,7 +14,7 @@ const emptySummary: WeatherSummary = { }; describe('weatherService negative cache helpers', () => { - it('treats recent empty cache entries as fresh negative results', () => { + it('does not treat recent empty cache entries as fresh negative results', () => { const now = new Date('2026-06-01T12:00:00.000Z'); expect(weatherServiceTestUtils.isFreshEmptyCacheEntry({ @@ -26,10 +26,10 @@ describe('weatherService negative cache helpers', () => { hasForecast: false, hasRecorded: false } - }, now)).toBe(true); + }, now)).toBe(false); }); - it('expires empty cache entries after the negative cache TTL', () => { + it('returns false for older empty cache entries too', () => { const now = new Date('2026-06-01T12:20:00.000Z'); expect(weatherServiceTestUtils.isFreshEmptyCacheEntry({ diff --git a/src/app/admin/components/MultiRouteLinkManager.tsx b/src/app/admin/components/MultiRouteLinkManager.tsx index db89f5d5..032f69fa 100644 --- a/src/app/admin/components/MultiRouteLinkManager.tsx +++ b/src/app/admin/components/MultiRouteLinkManager.tsx @@ -163,10 +163,6 @@ export default function MultiRouteLinkManager({ if (validation.valid && links.length > 0) { nextLinks = linksWithSplitValues; } else if (links.length === 0) { - if (initialLinks.length > 0) { - return; - } - nextLinks = []; } diff --git a/src/app/admin/components/TravelItemSelector.tsx b/src/app/admin/components/TravelItemSelector.tsx index 9e0558c9..d992cf24 100644 --- a/src/app/admin/components/TravelItemSelector.tsx +++ b/src/app/admin/components/TravelItemSelector.tsx @@ -124,6 +124,10 @@ export default function TravelItemSelector({ selectedItem: '', description: '' }); + const initialValueId = initialValue?.id ?? ''; + const initialValueType = initialValue?.type ?? ''; + const initialValueName = initialValue?.name ?? ''; + const hasInitialValue = Boolean(initialValue); useEffect(() => { onReferenceChangeRef.current = onReferenceChange; @@ -145,10 +149,10 @@ export default function TravelItemSelector({ useEffect(() => { queueMicrotask(() => { // First, try to use initialValue if provided - if (initialValue) { - setSelectedType(initialValue.type); - setSelectedItem(initialValue.id); - setDescription(initialValue.name || ''); + if (initialValueType && initialValueId) { + setSelectedType(initialValueType); + setSelectedItem(initialValueId); + setDescription(initialValueName); return; } @@ -180,7 +184,9 @@ export default function TravelItemSelector({ }, [ expenseLinksForHydration, expenseId, - initialValue, + initialValueId, + initialValueName, + initialValueType, loadExistingLink ]); @@ -311,6 +317,17 @@ export default function TravelItemSelector({ } console.error('Error loading travel items:', error); setTravelItems([]); + if ( + hasInitialValue || + selectionStateRef.current.selectedType || + selectionStateRef.current.selectedItem || + selectionStateRef.current.description + ) { + setSelectedType(''); + setSelectedItem(''); + setDescription(''); + onReferenceChangeRef.current(undefined); + } if (error instanceof Error && error.name === 'AbortError') { setLoadError('Loading travel items timed out. Please try again.'); } else { @@ -335,7 +352,7 @@ export default function TravelItemSelector({ window.clearTimeout(timeoutId); } }; - }, [tripId, reloadToken]); + }, [hasInitialValue, tripId, reloadToken]); const normalizedTransactionDates = useMemo(() => { const values: Array = [transactionDate]; diff --git a/src/app/api/cost-tracking/[id]/ynab-process/route.ts b/src/app/api/cost-tracking/[id]/ynab-process/route.ts index c7e517ff..cfe09c62 100644 --- a/src/app/api/cost-tracking/[id]/ynab-process/route.ts +++ b/src/app/api/cost-tracking/[id]/ynab-process/route.ts @@ -46,6 +46,10 @@ function isValidTempFileId(tempFileId: unknown): tempFileId is string { ); } +function getValidYnabISODate(ynabDate: string): string | null { + return convertYnabDateToISO(ynabDate) || null; +} + export async function POST(request: NextRequest, { params }: { params: Promise<{ id: string }> }) { try { // Check if request is from admin domain @@ -143,6 +147,7 @@ async function handleProcessTransactions( const uniqueTransactions: ProcessedYnabTransaction[] = []; const uniqueTransactionKeys = new Set(); let alreadyImportedCount = 0; + let invalidDateCount = 0; const mappedCategories = new Set(mappings.map(m => m.ynabCategory)); for (const [index, transaction] of tempData.transactions.entries()) { @@ -150,15 +155,21 @@ async function handleProcessTransactions( continue; // Skip unmapped categories } + const mapping = mappings.find(m => m.ynabCategory === transaction.Category); + if (!mapping || mapping.mappingType === 'none') continue; // Skip 'none' mappings + + const isoDate = getValidYnabISODate(transaction.Date); + if (!isoDate) { + invalidDateCount += 1; + continue; + } + const hash = createTransactionHash(transaction); const instanceId = `${hash}-${index}`; const hasImportedInstance = existingHashSet.has(instanceId); const hasLegacyImportedHash = consumeLegacyImportedHash(legacyImportedHashCounts, hash); const isAlreadyImported = hasImportedInstance || hasLegacyImportedHash; - const mapping = mappings.find(m => m.ynabCategory === transaction.Category); - if (!mapping || mapping.mappingType === 'none') continue; // Skip 'none' mappings - // Calculate amount - handle both outflows and inflows let amount = 0; const outflowStr = transaction.Outflow.replace('€', '').replace(',', '.'); @@ -175,7 +186,7 @@ async function handleProcessTransactions( const processedTxn: ProcessedYnabTransaction = { originalTransaction: transaction, amount: amount, - date: convertYnabDateToISO(transaction.Date), + date: isoDate, description: transaction.Payee, memo: transaction.Memo, mappedCountry: mapping.mappingType === 'general' ? '' : (mapping.countryName || ''), @@ -235,6 +246,7 @@ async function handleProcessTransactions( totalCount: 0, alreadyImportedCount, filteredCount, + invalidDateCount, lastImportedTransactionFound: showAll ? false : lastImportedTransactionFound || filteredResult.lastTransactionFound, totalTransactions: processedTransactions.length, message: 'No transactions available for import. This may be because all categories are mapped to "None" or all transactions have already been imported.' @@ -246,6 +258,7 @@ async function handleProcessTransactions( totalCount: filteredResult.newTransactions.length, alreadyImportedCount, filteredCount, + invalidDateCount, lastImportedTransactionFound: showAll ? false : lastImportedTransactionFound || filteredResult.lastTransactionFound, totalTransactions: processedTransactions.length }); @@ -333,6 +346,7 @@ async function handleImportTransactions( for (const selectedTxn of selectedTransactions) { const { transactionHash, transactionId, transactionSourceIndex, expenseCategory, travelLinkInfo } = selectedTxn; const targetIndex = typeof transactionSourceIndex === 'number' ? transactionSourceIndex : undefined; + const hasSpecificSelection = Boolean(transactionId) || targetIndex !== undefined; if (transactionId && importedKeySet.has(transactionId)) { consumeLegacyImportedHash(legacyImportedHashCounts, transactionHash); @@ -343,7 +357,7 @@ async function handleImportTransactions( continue; } - if (consumeLegacyImportedHash(legacyImportedHashCounts, transactionHash)) { + if (!hasSpecificSelection && consumeLegacyImportedHash(legacyImportedHashCounts, transactionHash)) { continue; } @@ -370,6 +384,11 @@ async function handleImportTransactions( continue; } + const isoDate = getValidYnabISODate(originalTxn.Date); + if (!isoDate) { + continue; + } + const importKey = getTransactionImportKey({ hash: transactionHash, instanceId: transactionId, @@ -406,7 +425,7 @@ async function handleImportTransactions( const processedTxn: ProcessedYnabTransaction = { originalTransaction: originalTxn, amount: amount, - date: convertYnabDateToISO(originalTxn.Date), + date: isoDate, description: originalTxn.Payee, memo: originalTxn.Memo, mappedCountry: mapping.mappingType === 'general' ? '' : (mapping.countryName || ''), @@ -420,7 +439,7 @@ async function handleImportTransactions( const travelReference = buildTravelReference(travelLinkInfo); const expense: Expense = { id: `ynab-${importKey}`, - date: new Date(convertYnabDateToISO(originalTxn.Date)), + date: new Date(isoDate), amount: amount, currency: costData.currency, category: expenseCategory, diff --git a/src/app/api/routing/osrm/route.ts b/src/app/api/routing/osrm/route.ts index 0df8a3f8..5e6ad543 100644 --- a/src/app/api/routing/osrm/route.ts +++ b/src/app/api/routing/osrm/route.ts @@ -61,13 +61,26 @@ const getForwardedHost = (request: NextRequest): string | null => { return forwardedHost?.split(',')[0]?.trim() || null; }; +const hasTrustedForwardedHostSecret = (request: NextRequest): boolean => { + const expectedSecret = process.env.ADMIN_PROXY_SECRET?.trim(); + if (!expectedSecret) { + return false; + } + + return request.headers.get('x-travel-tracker-admin-proxy-secret') === expectedSecret; +}; + const isAdminOsrmRequest = (request: NextRequest): boolean => { const host = request.headers.get('host'); if (isAdminHost(host)) { return true; } - return isLocalProxyHost(host) && isAdminHost(getForwardedHost(request)); + return ( + isLocalProxyHost(host) && + hasTrustedForwardedHostSecret(request) && + isAdminHost(getForwardedHost(request)) + ); }; export async function GET(request: NextRequest): Promise { diff --git a/src/app/api/travel-data/route.ts b/src/app/api/travel-data/route.ts index 0e471b56..4efdd0fc 100644 --- a/src/app/api/travel-data/route.ts +++ b/src/app/api/travel-data/route.ts @@ -455,6 +455,7 @@ export async function PATCH(request: NextRequest) { // Handle batch route point updates if (updateRequest.batchRouteUpdate) { const updates = updateRequest.batchRouteUpdate as Array<{routeId: string, routePoints: [number, number][]}>; + const validatedUpdates: Array<{ routeId: string; routePoints: [number, number][] }> = []; let totalRoutePoints = 0; for (const update of updates) { const validation = validateRoutePoints(update.routePoints, `Route ${update.routeId} routePoints`); @@ -466,6 +467,10 @@ export async function PATCH(request: NextRequest) { } totalRoutePoints += validation.points.length; + validatedUpdates.push({ + routeId: update.routeId, + routePoints: validation.points + }); if (totalRoutePoints > MAX_ROUTE_POINTS_PER_UPDATE) { return NextResponse.json( { error: `Route update cannot contain more than ${MAX_ROUTE_POINTS_PER_UPDATE} total route points` }, @@ -474,7 +479,7 @@ export async function PATCH(request: NextRequest) { } } - console.log('[PATCH] Processing batch route update for trip %s with %d routes', id, updates.length); + console.log('[PATCH] Processing batch route update for trip %s with %d routes', id, validatedUpdates.length); // Load current unified data const unifiedData = await loadUnifiedTripData(id); @@ -492,7 +497,7 @@ export async function PATCH(request: NextRequest) { if (unifiedData.travelData && unifiedData.travelData.routes) { let updatedCount = 0; - for (const update of updates) { + for (const update of validatedUpdates) { const routeIndex = unifiedData.travelData.routes.findIndex((route: { id: string }) => route.id === update.routeId); if (routeIndex >= 0) { console.log( @@ -575,7 +580,7 @@ export async function PATCH(request: NextRequest) { if (unifiedData.travelData && unifiedData.travelData.routes) { const routeIndex = unifiedData.travelData.routes.findIndex((route: { id: string }) => route.id === routeId); if (routeIndex >= 0) { - (unifiedData.travelData.routes[routeIndex] as { routePoints?: [number, number][] }).routePoints = routePoints; + (unifiedData.travelData.routes[routeIndex] as { routePoints?: [number, number][] }).routePoints = validation.points; const routePointValidation = validateSubmittedRoutePoints( unifiedData.travelData.routes as unknown as RoutePayload[], diff --git a/src/app/services/weatherService.ts b/src/app/services/weatherService.ts index 262f98ee..9b82eecd 100644 --- a/src/app/services/weatherService.ts +++ b/src/app/services/weatherService.ts @@ -27,7 +27,6 @@ const RATE_LIMIT_BASE_BACKOFF_MS = 2000; const RATE_LIMIT_MAX_BACKOFF_MS = 60_000; const RATE_LIMIT_MAX_RETRIES = 4; const HISTORICAL_AVERAGE_FORECAST_REFRESH_MS = 6 * 60 * 60 * 1000; -const EMPTY_WEATHER_CACHE_TTL_MS = 15 * 60 * 1000; let lastRequestTime = 0; let rateLimitBackoffUntil = 0; let rateLimitQueue: Promise = Promise.resolve(); @@ -432,12 +431,11 @@ function needsForecastRefresh(summary: WeatherSummary): boolean { }); } -function isFreshEmptyCacheEntry(cached: CacheEntry, now: Date): boolean { - if (cached.summary.dailyWeather.length > 0) return false; - if (cached.fetchedAt === LEGACY_FETCHED_AT) return false; - - const fetchedAt = parseISO(cached.fetchedAt); - return isValid(fetchedAt) && now.getTime() - fetchedAt.getTime() < EMPTY_WEATHER_CACHE_TTL_MS; +function isFreshEmptyCacheEntry(_cached: CacheEntry, _now: Date): boolean { + // Negative caching for empty weather lookups is disabled. + void _cached; + void _now; + return false; } /** @@ -773,14 +771,6 @@ class WeatherService { const forecastRefreshNeeded = needsForecastRefresh(cached.summary); if (forecastRefreshNeeded) reasons.push('needs-forecast'); - if (isFreshEmptyCacheEntry(cached, now)) { - log('cache:negative-hit', { key }); - return { - ...cached.summary, - summary: summarize(cached.summary.dailyWeather) - }; - } - if (preferCache && hasData) { log('cache:prefer-hit', { key, count: cached.summary.dailyWeather.length }); return { @@ -826,9 +816,6 @@ class WeatherService { } } - if (fetched.length === 0) { - log('fetch:failed-no-data', { key }); - } const summary: WeatherSummary = { locationId: location.id, startDate: startISO, @@ -836,8 +823,12 @@ class WeatherService { dailyWeather: fetched, summary: summarize(fetched) }; - await writeCache(key, summary); - log(fetched.length > 0 ? 'cache:write' : 'cache:write-empty', { key, count: fetched.length }); + if (fetched.length > 0) { + await writeCache(key, summary); + log('cache:write', { key, count: fetched.length }); + } else { + log('fetch:failed-no-data', { key }); + } return summary; }