diff --git a/server/api.ts b/server/api.ts index b9fcada8..7d3e66f4 100644 --- a/server/api.ts +++ b/server/api.ts @@ -10,9 +10,9 @@ import { MapperKind, mapSchema } from '@graphql-tools/utils'; import { MultiSamlStrategy } from '@node-saml/passport-saml'; import { SpanStatusCode } from '@opentelemetry/api'; import { - SEMATTRS_EXCEPTION_MESSAGE, - SEMATTRS_EXCEPTION_STACKTRACE, - SEMATTRS_EXCEPTION_TYPE, + ATTR_EXCEPTION_MESSAGE, + ATTR_EXCEPTION_STACKTRACE, + ATTR_EXCEPTION_TYPE, } from '@opentelemetry/semantic-conventions'; import connectPgSimple from 'connect-pg-simple'; import cors from 'cors'; @@ -276,7 +276,7 @@ export default async function makeApiServer(deps: Dependencies) { }, ); - passport.serializeUser((user: any, done) => { + passport.serializeUser((user, done) => { done(null, user.id); }); @@ -422,11 +422,11 @@ export default async function makeApiServer(deps: Dependencies) { span.setStatus({ code: SpanStatusCode.ERROR, message: err.message }); // I don't know if these attributes are necessary, with recordException - span.setAttribute(SEMATTRS_EXCEPTION_MESSAGE, err.message); + span.setAttribute(ATTR_EXCEPTION_MESSAGE, err.message); if (err.stack) { - span.setAttribute(SEMATTRS_EXCEPTION_STACKTRACE, err.stack); + span.setAttribute(ATTR_EXCEPTION_STACKTRACE, err.stack); } - span.setAttribute(SEMATTRS_EXCEPTION_TYPE, err.name); + span.setAttribute(ATTR_EXCEPTION_TYPE, err.name); const errors = (() => { if (err instanceof AggregateError) { diff --git a/server/bin/run-worker-or-job.ts b/server/bin/run-worker-or-job.ts index d0b6dae8..a07617d5 100644 --- a/server/bin/run-worker-or-job.ts +++ b/server/bin/run-worker-or-job.ts @@ -1,14 +1,16 @@ #!/usr/bin/env node import _ from 'lodash'; -import getBottle from '../iocContainer/index.js'; +import getBottle, { type Dependencies } from '../iocContainer/index.js'; import { logErrorJson } from '../utils/logging.js'; import { type WorkerOrJob } from '../workers_jobs/index.js'; const { container } = await getBottle(); const workerOrJobName = process.argv[2]; -const workerOrJob = (container as any)[workerOrJobName] as WorkerOrJob; +const workerOrJob = container[ + workerOrJobName as keyof Dependencies +] as WorkerOrJob; const controller = new AbortController(); // When the worker/job finishes naturally (which only applies to jobs, as diff --git a/server/condition_evaluator/conditionSet.test.ts b/server/condition_evaluator/conditionSet.test.ts index 77b710ca..7784ee4a 100644 --- a/server/condition_evaluator/conditionSet.test.ts +++ b/server/condition_evaluator/conditionSet.test.ts @@ -85,8 +85,12 @@ describe('Condition Evaluation', () => { await getConditionSetResults( { conditions, conjunction: ConditionConjunction.OR }, + // Tests only exercise getSignalCost / tracer.getActiveSpan; + // a full RuleEvaluationContext / SafeTracer is unnecessary. + /* eslint-disable @typescript-eslint/no-explicit-any */ { getSignalCost } as any, jest.fn() as any, + /* eslint-enable @typescript-eslint/no-explicit-any */ stubRunLeafCondition, ); diff --git a/server/condition_evaluator/conditionSet.ts b/server/condition_evaluator/conditionSet.ts index 8ff93faa..a827aded 100644 --- a/server/condition_evaluator/conditionSet.ts +++ b/server/condition_evaluator/conditionSet.ts @@ -69,12 +69,19 @@ export async function getConditionSetResults( runLeafCondition = defaultRunLeafCondition, ): Promise> { const { conjunction } = conditionSet; - const result: ConditionSetWithResult = { - conjunction, - conditions: [] as unknown as - | NonEmptyArray - | NonEmptyArray, - }; + // We collect mixed condition results during the loop, then narrow back to + // the public "ConditionSetWithResult.conditions" shape at return time. The + // public type is a discriminated union of "all leaves" or "all sets", which + // can't be safely pushed into; the array's runtime contents stay uniform + // because the input "conditionSet.conditions" is uniform. + // + // The element type is "ReadonlyDeep<...>" here because the inputs we push + // come from "ReadonlyDeep". We cast back to the mutable + // shape at the return site below; the cast is sound because the data is + // freshly built and never mutated by callers. + const conditionsWithResult: Array< + ReadonlyDeep + > = []; // The conditions, sorted with lowest cost ones first. const getSignalCost = evaluationContext.getSignalCost.bind(evaluationContext); @@ -99,17 +106,17 @@ export async function getConditionSetResults( // if we can determine it before evaluating all conditions. let finalOutcome: ConditionOutcome | undefined; - // This is basically mapping `conditions` to ConditionWithResult, and putting - // the mapped array in result.conditions. But we don't use `map` because we - // want to run the conditions in sequence (i.e., with an `await` on each loop + // This is basically mapping "conditions" to ConditionWithResult, and pushing + // them onto "conditionsWithResult". But we don't use "map" because we want to + // run the conditions in sequence (i.e., with an "await" on each loop // iteration), and map would run them in parallel. for (const condition of sortedConditions) { // If we already know the final outcome for the condition set, then we can // skip all subsequent conditions, and a condition that's skipped has an // identical representation for its ConditionWithResult, so we just push - // the skipped condition into result.conditions. + // the skipped condition through. if (finalOutcome !== undefined) { - result.conditions.push(condition as any); + conditionsWithResult.push(condition); continue; } @@ -133,14 +140,14 @@ export async function getConditionSetResults( ), }; - result.conditions.push(conditionWithResult as any); + conditionsWithResult.push(conditionWithResult); // console.log('RESULT ' + conditionWithResult.result.outcome); // Attempt to determine the result for the whole condition set from the // outcomes so far. If we can, save that result to skip running each // condition for the rest of the loop const conditionSetOutcome = tryGetOutcomeFromPartialOutcomes( - result.conditions.map((c) => c.result!.outcome), + conditionsWithResult.map((c) => c.result!.outcome), conjunction, ); @@ -150,12 +157,14 @@ export async function getConditionSetResults( } return { - ...result, + conjunction, + conditions: + conditionsWithResult as ConditionSetWithResult['conditions'], result: { outcome: finalOutcome ?? getConditionSetOutcome( - result.conditions.map((c) => c.result!.outcome), + conditionsWithResult.map((c) => c.result!.outcome), conjunction, ), }, diff --git a/server/condition_evaluator/leafCondition.ts b/server/condition_evaluator/leafCondition.ts index f7bbc1dd..c8792220 100644 --- a/server/condition_evaluator/leafCondition.ts +++ b/server/condition_evaluator/leafCondition.ts @@ -203,7 +203,7 @@ export async function runLeafCondition( }); }), ) - : signalInputValues.map((it): SignalResult => { + : signalInputValues.map((it): SignalResult => { // If the condition specified no signal, then we should act as though // the "identity" signal was used; i.e., the value that the condition // picked out (with `condition.input`) should be returned as-is. @@ -259,7 +259,7 @@ export async function runLeafCondition( it.score, condition.threshold, condition.comparator, - it.outputType as SignalOutputType, + it.outputType, ) : (it as SignalResult<{ scalarType: ScalarTypes['BOOLEAN'] }>).score, ), diff --git a/server/decs.d.ts b/server/decs.d.ts index c1803752..3922c845 100644 --- a/server/decs.d.ts +++ b/server/decs.d.ts @@ -1,3 +1,11 @@ +declare namespace Express { + // Extend Express.User so passport callbacks (serializeUser, etc.) see the + // fields we actually use without per-call `as any` casts. + interface User { + id: string; + } +} + declare module 'homoglyph-search'; declare module 'nilsimsa'; diff --git a/server/graphql/datasources/UserApi.ts b/server/graphql/datasources/UserApi.ts index 5c88f8ad..38329dd0 100644 --- a/server/graphql/datasources/UserApi.ts +++ b/server/graphql/datasources/UserApi.ts @@ -14,6 +14,10 @@ import { } from '../../utils/errors.js'; import { safePick } from '../../utils/misc.js'; import { WEEK_MS } from '../../utils/time.js'; +import { + type GQLMutationLoginArgs, + type GQLMutationSignUpArgs, +} from '../generated.js'; import { type PassportGqlContext } from '../utils/passportContext.js'; import { buildGraphqlRuleParent } from './buildGraphqlRuleParent.js'; import { type GraphQLRuleParent } from './ruleKyselyPersistence.js'; @@ -74,7 +78,7 @@ class UserAPI { return kyselyUserFindByIds(this.kyselyPg, ids); } - async login(params: any, context: PassportGqlContext) { + async login(params: GQLMutationLoginArgs, context: PassportGqlContext) { const { email, password } = safePick(params.input, ['email', 'password']); // Reject missing/empty credentials as a bad request before verifying. @@ -118,7 +122,10 @@ class UserAPI { } } - async signUp(params: any, _: any): Promise { + async signUp( + params: GQLMutationSignUpArgs, + _: unknown, + ): Promise { const { role } = params.input; const { email, diff --git a/server/graphql/modules/apiKey.ts b/server/graphql/modules/apiKey.ts index 394c8276..c1a370a6 100644 --- a/server/graphql/modules/apiKey.ts +++ b/server/graphql/modules/apiKey.ts @@ -2,17 +2,7 @@ import { ErrorType, CoopError } from '../../utils/errors.js'; import { logErrorJson } from '../../utils/logging.js'; import { gqlErrorResult, gqlSuccessResult } from '../utils/gqlResult.js'; import { forbiddenError } from '../utils/errors.js'; - -/** Context shape required by rotateWebhookSigningKey (avoids importing resolvers). */ -type RotateWebhookSigningKeyContext = { - getUser: () => { - orgId: string; - getPermissions: () => readonly string[]; - } | null | undefined; - dataSources: { - orgAPI: { rotateWebhookSigningKey: (orgId: string) => Promise }; - }; -}; +import { type GQLMutationResolvers, type GQLQueryResolvers } from '../generated.js'; const typeDefs = /* GraphQL */ ` type ApiKey { @@ -73,8 +63,8 @@ const typeDefs = /* GraphQL */ ` } `; -const Query: any = { - async apiKey(_: any, __: any, context: any) { +const Query: GQLQueryResolvers = { + async apiKey(_, __, context) { const user = context.getUser(); if (!user || !user.orgId) { throw forbiddenError('User does not have permission to check if key exists'); @@ -89,8 +79,8 @@ const Query: any = { }, }; -const Mutation: any = { - async rotateApiKey(_: any, { input }: any, context: any) { +const Mutation: GQLMutationResolvers = { + async rotateApiKey(_, { input }, context) { const user = context.getUser(); if (!user || !user.orgId) { throw forbiddenError('User does not have permission to rotate the API key'); @@ -103,7 +93,7 @@ const Mutation: any = { const { apiKey, record } = await context.services.ApiKeyService.rotateApiKey( user.orgId, input.name, - input.description || null, + input.description ?? null, user.id ); @@ -116,7 +106,7 @@ const Mutation: any = { description: record.description, isActive: record.isActive, createdAt: record.createdAt.toISOString(), - lastUsedAt: record.lastUsedAt?.toISOString() || null, + lastUsedAt: record.lastUsedAt?.toISOString() ?? null, createdBy: record.createdBy, }, }, @@ -132,17 +122,13 @@ const Mutation: any = { type: [ErrorType.InternalServerError], title: 'Failed to rotate API key', detail: 'An error occurred while rotating the API key', - name: 'InternalServerError', + name: 'RotateApiKeyError', shouldErrorSpan: true, }), ); } }, - async rotateWebhookSigningKey( - _: unknown, - __: Record, - context: RotateWebhookSigningKeyContext, - ) { + async rotateWebhookSigningKey(_, __, context) { const user = context.getUser(); if (!user || !user.orgId) { throw forbiddenError('User does not have permission to rotate the webhook signing key'); @@ -171,7 +157,7 @@ const Mutation: any = { type: [ErrorType.InternalServerError], title: 'Failed to rotate webhook signing key', detail: 'An error occurred while rotating the webhook signing key', - name: 'InternalServerError', + name: 'RotateWebhookSigningKeyError', shouldErrorSpan: true, }), ); diff --git a/server/graphql/modules/insights.ts b/server/graphql/modules/insights.ts index 89739f1d..952b1940 100644 --- a/server/graphql/modules/insights.ts +++ b/server/graphql/modules/insights.ts @@ -180,10 +180,10 @@ const RuleInsights: GQLRuleInsightsResolvers = { }, ); - // TODO: remove any cast. It's hiding that there are legit fields missing - // here, meaning that graphql queries would throw an exception if asking for - // those fields. To provide them, we'll have to update (and add better - // typings for) getRulePassingContentSamples. + // TODO: type this properly. graphql queries would throw an exception if + // asking for fields that aren't populated here. To provide them, we'll + // have to update (and add better typings for) getRulePassingContentSamples. + /* eslint-disable @typescript-eslint/no-explicit-any -- see TODO above */ return samples.map((it) => ({ ...it, itemId: it.contentId, @@ -193,6 +193,7 @@ const RuleInsights: GQLRuleInsightsResolvers = { ruleId: rule.id, ruleName: rule.name, })) as any[]; + /* eslint-enable @typescript-eslint/no-explicit-any */ }, async priorVersionSamples(rule, _args, context) { const user = context.getUser(); @@ -210,10 +211,8 @@ const RuleInsights: GQLRuleInsightsResolvers = { }, ); - // TODO: remove any cast. It's hiding that there are legit fields missing - // here, meaning that graphql queries would throw an exception if asking for - // those fields. To provide them, we'll have to update (and add better - // typings for) getRulePassingContentSamples. + // TODO: type this properly. See latestVersionSamples for context. + /* eslint-disable @typescript-eslint/no-explicit-any -- see TODO above */ return samples.map((it) => ({ ...it, itemId: it.contentId, @@ -223,6 +222,7 @@ const RuleInsights: GQLRuleInsightsResolvers = { ruleId: rule.id, ruleName: rule.name, })) as any[]; + /* eslint-enable @typescript-eslint/no-explicit-any */ }, }; @@ -255,10 +255,10 @@ const ReportingRuleInsights: GQLReportingRuleInsightsResolvers = { }, ); - // TODO: remove any cast. It's hiding that there are legit fields missing - // here, meaning that graphql queries would throw an exception if asking for - // those fields. To provide them, we'll have to update (and add better - // typings for) getRulePassingContentSamples. + // TODO: type this properly. graphql queries would throw an exception if + // asking for fields that aren't populated here. To provide them, we'll + // have to update (and add better typings for) getRulePassingContentSamples. + /* eslint-disable @typescript-eslint/no-explicit-any -- see TODO above */ return samples.map((it) => ({ ...it, itemId: it.contentId, @@ -268,6 +268,7 @@ const ReportingRuleInsights: GQLReportingRuleInsightsResolvers = { ruleId: rule.id, ruleName: rule.name, })) as any[]; + /* eslint-enable @typescript-eslint/no-explicit-any */ }, async priorVersionSamples(rule, _args, context) { const user = context.getUser(); @@ -285,10 +286,8 @@ const ReportingRuleInsights: GQLReportingRuleInsightsResolvers = { }, ); - // TODO: remove any cast. It's hiding that there are legit fields missing - // here, meaning that graphql queries would throw an exception if asking for - // those fields. To provide them, we'll have to update (and add better - // typings for) getRulePassingContentSamples. + // TODO: type this properly. See latestVersionSamples for context. + /* eslint-disable @typescript-eslint/no-explicit-any -- see TODO above */ return samples.map((it) => ({ ...it, itemId: it.contentId, @@ -298,6 +297,7 @@ const ReportingRuleInsights: GQLReportingRuleInsightsResolvers = { ruleId: rule.id, ruleName: rule.name, })) as any[]; + /* eslint-enable @typescript-eslint/no-explicit-any */ }, }; @@ -346,12 +346,11 @@ const Query: GQLQueryResolvers = { const result = samples[0]; + // TODO: type this properly. graphql queries would throw an exception if + // asking for fields that aren't populated here. To provide them, we'll + // have to update (and add better typings for) getRulePassingContentSamples. + /* eslint-disable @typescript-eslint/no-explicit-any -- see TODO above */ return gqlSuccessResult( - // TODO: remove any cast. It's hiding that there are legit fields missing - // here, meaning that graphql queries would throw an exception if asking for - // those fields. To provide them, we'll have to update (and add better - // typings for) getRulePassingContentSamples. - { ...result, itemId: result.contentId, @@ -363,6 +362,7 @@ const Query: GQLQueryResolvers = { } as any, 'RuleExecutionResult', ); + /* eslint-enable @typescript-eslint/no-explicit-any */ } catch (e) { if (isCoopErrorOfType(e, 'NotFoundError')) { return gqlErrorResult(e); diff --git a/server/graphql/resolvers.ts b/server/graphql/resolvers.ts index fdb600de..db9cd69a 100644 --- a/server/graphql/resolvers.ts +++ b/server/graphql/resolvers.ts @@ -97,10 +97,13 @@ const Query: GQLQueryResolvers = { } try { - // TODO: this response type actually isn't right; remove cast and fix errors. + // TODO: this response type actually isn't right; remove the cast below + // and fix the underlying mismatch in getAllRuleInsights' return shape. + /* eslint-disable @typescript-eslint/no-explicit-any -- see TODO above */ return (await context.dataSources.ruleAPI.getAllRuleInsights( user.orgId, )) as any; + /* eslint-enable @typescript-eslint/no-explicit-any */ } catch (e) { // eslint-disable-next-line no-console console.error( diff --git a/server/routes/integration_logos/serveIntegrationLogo.ts b/server/routes/integration_logos/serveIntegrationLogo.ts index b0e485ed..df498022 100644 --- a/server/routes/integration_logos/serveIntegrationLogo.ts +++ b/server/routes/integration_logos/serveIntegrationLogo.ts @@ -35,8 +35,8 @@ export default function serveIntegrationLogo( // so the SPA can load it via when deployed on a different origin // than the API. res.setHeader('Cross-Origin-Resource-Policy', 'cross-origin'); - res.sendFile(filePath, (err) => { - if (err != null && !res.headersSent) { + res.sendFile(filePath, (err?: Error) => { + if (err && !res.headersSent) { next(err); } diff --git a/server/routes/integration_logos/serveIntegrationLogoWithBackground.ts b/server/routes/integration_logos/serveIntegrationLogoWithBackground.ts index 0ccbeaeb..9b8381ae 100644 --- a/server/routes/integration_logos/serveIntegrationLogoWithBackground.ts +++ b/server/routes/integration_logos/serveIntegrationLogoWithBackground.ts @@ -35,8 +35,8 @@ export default function serveIntegrationLogoWithBackground( // so the SPA can load it via when deployed on a different origin // than the API. res.setHeader('Cross-Origin-Resource-Policy', 'cross-origin'); - res.sendFile(filePath, (err) => { - if (err != null && !res.headersSent) { + res.sendFile(filePath, (err?: Error) => { + if (err && !res.headersSent) { next(err); } diff --git a/server/routes/policies/PoliciesRoutes.ts b/server/routes/policies/PoliciesRoutes.ts index 1886e27c..52e031aa 100644 --- a/server/routes/policies/PoliciesRoutes.ts +++ b/server/routes/policies/PoliciesRoutes.ts @@ -1,19 +1,15 @@ -import { route, type Route } from '../../utils/route-helpers.js'; +import { route } from '../../utils/route-helpers.js'; import { createApiKeyMiddleware } from '../../utils/apiKeyMiddleware.js'; -import { type Controller } from '../index.js'; +import { type Controller, type ControllerRouteList } from '../index.js'; import getPolicies from './getPolicies.js'; export type GetPoliciesOutput = { policies: { id: string; name: string; parentId: string | null }[]; }; -// eslint-disable-next-line @typescript-eslint/consistent-type-assertions export default { pathPrefix: '/policies', routes: [ - route.get('/', (deps) => [createApiKeyMiddleware(deps), getPolicies(deps)]) as Route< - any, - GetPoliciesOutput - >, - ], -} as Controller; + route.get('/', (deps) => [createApiKeyMiddleware(deps), getPolicies(deps)]), + ] as ControllerRouteList, +} satisfies Controller; diff --git a/server/routes/reporting/submitReport.ts b/server/routes/reporting/submitReport.ts index e147938a..af6cf333 100644 --- a/server/routes/reporting/submitReport.ts +++ b/server/routes/reporting/submitReport.ts @@ -128,7 +128,6 @@ export default function submitReport({ const matchedBankNames: string[] = []; if ( - hashes && Object.keys(hashes).length > 0 && allBankNames.length > 0 ) { diff --git a/server/routes/user_scores/UserScoresRoutes.ts b/server/routes/user_scores/UserScoresRoutes.ts index 1cfd6a6e..fc8148e6 100644 --- a/server/routes/user_scores/UserScoresRoutes.ts +++ b/server/routes/user_scores/UserScoresRoutes.ts @@ -1,17 +1,13 @@ -import { route, type Route } from '../../utils/route-helpers.js'; +import { route } from '../../utils/route-helpers.js'; import { createApiKeyMiddleware } from '../../utils/apiKeyMiddleware.js'; -import { type Controller } from '../index.js'; +import { type Controller, type ControllerRouteList } from '../index.js'; import getUserScores from './getUserScores.js'; export type GetUserScoresOutput = number; -// eslint-disable-next-line @typescript-eslint/consistent-type-assertions export default { pathPrefix: '/user_scores', routes: [ - route.get('/', (deps) => [createApiKeyMiddleware(deps), getUserScores(deps)]) as Route< - any, - GetUserScoresOutput - >, - ], -} as Controller; + route.get('/', (deps) => [createApiKeyMiddleware(deps), getUserScores(deps)]), + ] as ControllerRouteList, +} satisfies Controller; diff --git a/server/rule_engine/RuleEngine.ts b/server/rule_engine/RuleEngine.ts index 3e016573..2dad8413 100644 --- a/server/rule_engine/RuleEngine.ts +++ b/server/rule_engine/RuleEngine.ts @@ -101,19 +101,14 @@ class RuleEngine { executionsCorrelationId: RuleExecutionCorrelationId, sync: boolean = false, ) { - // enabledRules can be null when the contentType can't be found. - // getEnabledRulesForContentTypeEventuallyConsistent has `null` in its - // return type primarily in case the contentTypeId points to a content type - // that doesn't exist. However, even though we know that the contentTypeId - // is for a content type that does exist (because we have the full - // ContentType model object), we still must handle `null` b/c it could be - // that contentType was _just_ created and can't be seen yet by - // getEnabledRulesForContentTypeEventuallyConsistent, which, as the name - // implies, is eventually consistent. + // enabledRules will be an empty array when the contentType can't be found + // or has no rules attached. getEnabledRulesForItemTypeEventuallyConsistent + // is, as the name implies, eventually consistent, so a content type that + // was just created may not yet be visible. const enabledRules = - (await this.getEnabledRulesForItemTypeEventuallyConsistent( + await this.getEnabledRulesForItemTypeEventuallyConsistent( itemSubmission.itemType.id, - )) ?? []; + ); const [liveRules, backgroundRules] = partition( enabledRules, diff --git a/server/services/aggregationsService/AggregationsService.ts b/server/services/aggregationsService/AggregationsService.ts index 59c35949..f558bb7b 100644 --- a/server/services/aggregationsService/AggregationsService.ts +++ b/server/services/aggregationsService/AggregationsService.ts @@ -66,7 +66,14 @@ export class AggregationsService { const keys = getStoreKeysForAggregation(aggregation, runtimeArgs); const kvs = await this.keyValueStore.getAll(keys); + // The Aggregation union has only one variant ('COUNT') today, so the + // case label and the discriminant are trivially equal and trip + // no-unnecessary-condition. The switch is intentional: when a new + // variant is added, the existing case stops being exhaustive and the + // "default: assertUnreachable(...)" branch surfaces the gap at compile + // time. Disabling the condition rule (only) preserves that safety net. switch (aggregation.aggregation.type) { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- see comment above case 'COUNT': return Array.from(kvs.values()).reduce((acc, count) => acc + count, 0); default: @@ -212,7 +219,14 @@ function getStoreKeyPrefixForAggregation( } function serializeAggregation(aggregation: Aggregation): string { + // The Aggregation union has only one variant ('COUNT') today, so the + // case label and the discriminant are trivially equal and trip + // no-unnecessary-condition. The switch is intentional: when a new + // variant is added, the existing case stops being exhaustive and the + // `default: assertUnreachable(...)` branch surfaces the gap at compile + // time. Disabling the condition rule (only) preserves that safety net. switch (aggregation.type) { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- see comment above case 'COUNT': return 'COUNT'; default: diff --git a/server/services/analyticsLoggers/ContentApiLogger.ts b/server/services/analyticsLoggers/ContentApiLogger.ts index e05508b4..94bd046d 100644 --- a/server/services/analyticsLoggers/ContentApiLogger.ts +++ b/server/services/analyticsLoggers/ContentApiLogger.ts @@ -52,7 +52,7 @@ class ContentApiLogger { const { itemType } = itemSubmission; const now = new Date(); await this.analytics.bulkWrite( - 'CONTENT_API_REQUESTS' as any, + 'CONTENT_API_REQUESTS', [ { ds: getUtcDateOnlyString(now), diff --git a/server/services/analyticsLoggers/ReportingRuleExecutionLogger.ts b/server/services/analyticsLoggers/ReportingRuleExecutionLogger.ts index 652053d5..2c4c9c68 100644 --- a/server/services/analyticsLoggers/ReportingRuleExecutionLogger.ts +++ b/server/services/analyticsLoggers/ReportingRuleExecutionLogger.ts @@ -37,7 +37,7 @@ class ReportingRuleExecutionLogger { ) { const now = new Date(); await this.analytics.bulkWrite( - 'REPORTING_SERVICE.REPORTING_RULE_EXECUTIONS' as any, + 'REPORTING_SERVICE.REPORTING_RULE_EXECUTIONS', executions.map((data) => ({ ds: getUtcDateOnlyString(now), ts: now.valueOf(), diff --git a/server/services/analyticsLoggers/RoutingRuleExecutionLogger.ts b/server/services/analyticsLoggers/RoutingRuleExecutionLogger.ts index 3f52e68b..00d36954 100644 --- a/server/services/analyticsLoggers/RoutingRuleExecutionLogger.ts +++ b/server/services/analyticsLoggers/RoutingRuleExecutionLogger.ts @@ -43,7 +43,7 @@ class RoutingRuleExecutionLogger { ) { const now = new Date(); await this.analytics.bulkWrite( - 'MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS' as any, + 'MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS', executions.map((data) => ({ ds: getUtcDateOnlyString(now), ts: now.valueOf(), diff --git a/server/services/analyticsLoggers/RuleExecutionLogger.ts b/server/services/analyticsLoggers/RuleExecutionLogger.ts index d6fa0298..ca871d1b 100644 --- a/server/services/analyticsLoggers/RuleExecutionLogger.ts +++ b/server/services/analyticsLoggers/RuleExecutionLogger.ts @@ -3,6 +3,7 @@ import { type ReadonlyDeep } from 'type-fest'; import { type Dependencies } from '../../iocContainer/index.js'; import { inject } from '../../iocContainer/utils.js'; +import { type AnalyticsSchema } from '../../storage/dataWarehouse/IDataWarehouseAnalytics.js'; import { type RuleEnvironment } from '../../rule_engine/RuleEngine.js'; import { type ConditionSetWithResult } from '../../services/moderationConfigService/index.js'; import { fromCorrelationId } from '../../utils/correlationIds.js'; @@ -80,7 +81,7 @@ class RuleExecutionLogger { correlation_id: fromCorrelationId(data.correlationId), result: jsonStringifyUnstable(pickConditionPropsToLog(data.result)), passed: data.passed, - })) as any, + })) as AnalyticsSchema['RULE_EXECUTIONS'][], { batchTimeout: sync ? 0 : undefined }, ); } diff --git a/server/services/analyticsQueries/ItemHistoryQueries.ts b/server/services/analyticsQueries/ItemHistoryQueries.ts index 6b639ef9..8e4f7056 100644 --- a/server/services/analyticsQueries/ItemHistoryQueries.ts +++ b/server/services/analyticsQueries/ItemHistoryQueries.ts @@ -19,6 +19,11 @@ type ItemHistoryQueryFilter = { class ItemHistoryQueries { constructor(private readonly dialect: Dependencies['DataWarehouseDialect']) {} + // The data warehouse schema isn't statically modeled in TS, so we use + // "any" here to opt out of Kysely's column-name checking. Kysely's stricter + // alternatives ("Record") reject the string-based column + // selections this query relies on. + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- see comment above private get kysely(): Kysely { return this.dialect.getKyselyInstance(); } @@ -76,7 +81,7 @@ class ItemHistoryQueries { try { const results = await query.execute(); - return results.map((it: any) => ({ + return results.map((it) => ({ itemTypeName: it.itemTypeName, itemTypeId: it.itemTypeId, userId: it.userId, diff --git a/server/services/apiKeyService/apiKeyService.ts b/server/services/apiKeyService/apiKeyService.ts index c701752d..fe513466 100644 --- a/server/services/apiKeyService/apiKeyService.ts +++ b/server/services/apiKeyService/apiKeyService.ts @@ -1,7 +1,10 @@ import crypto from 'node:crypto'; -import type { Kysely } from 'kysely'; +import type { Kysely, Selectable } from 'kysely'; import { inject } from '../../iocContainer/index.js'; import { type CombinedPg } from '../combinedDbTypes.js'; +import { type ApiKeyServicePg } from './dbTypes.js'; + +type ApiKeyRow = Selectable; export interface ApiKeyMetadata { name: string; @@ -196,7 +199,7 @@ class ApiKeyService { /** * Maps database record to ApiKeyRecord */ - private mapDbRecordToApiKeyRecord(record: any): ApiKeyRecord { + private mapDbRecordToApiKeyRecord(record: ApiKeyRow): ApiKeyRecord { return { id: record.id, orgId: record.org_id, diff --git a/server/services/derivedFieldsService/helpers.test.ts b/server/services/derivedFieldsService/helpers.test.ts index f5260679..745d9205 100644 --- a/server/services/derivedFieldsService/helpers.test.ts +++ b/server/services/derivedFieldsService/helpers.test.ts @@ -261,6 +261,10 @@ describe('Item type schemas', () => { }); test('should reject invalid specs', () => { + // The casts to "any" below intentionally bypass TS so that we can + // construct invalid inputs (the very thing parseDerivedFieldSpec is + // expected to reject at runtime). + /* eslint-disable @typescript-eslint/no-explicit-any */ expect(() => { parseDerivedFieldSpec( serializeDerivedFieldSpec({ @@ -309,6 +313,7 @@ describe('Item type schemas', () => { }), ); }).toThrowErrorMatchingInlineSnapshot(`"Invalid derived field spec"`); + /* eslint-enable @typescript-eslint/no-explicit-any */ }); }); }); diff --git a/server/services/derivedFieldsService/helpers.ts b/server/services/derivedFieldsService/helpers.ts index c54b2b43..c28678c3 100644 --- a/server/services/derivedFieldsService/helpers.ts +++ b/server/services/derivedFieldsService/helpers.ts @@ -339,7 +339,15 @@ export async function getDerivedFieldValue( .reduce(async (derivedResPromise, recipeOperation) => { // get value(s) as they've been generated so far. const valueOrValues = await derivedResPromise; + // The DerivedFieldOperationType enum has only one variant + // (RUN_SIGNAL) today, so the case label and the discriminant are + // trivially equal and trip no-unnecessary-condition. The switch is + // intentional: when a new variant is added, the existing case stops + // being exhaustive and the "default: assertUnreachable(...)" branch + // surfaces the gap at compile time. Disabling the condition rule + // (only) preserves that safety net. switch (recipeOperation.type) { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- see comment above case DerivedFieldOperationType.RUN_SIGNAL: { const transformValue = transformWithSignal.bind( null, diff --git a/server/services/integrationRegistry/index.ts b/server/services/integrationRegistry/index.ts index 066e81d2..c602d335 100644 --- a/server/services/integrationRegistry/index.ts +++ b/server/services/integrationRegistry/index.ts @@ -91,9 +91,7 @@ let cachedRegistry: IntegrationRegistry | null = null; * Returns the integration registry (built once on first call). */ export function getIntegrationRegistry(): IntegrationRegistry { - if (cachedRegistry == null) { - cachedRegistry = buildRegistry(); - } + cachedRegistry ??= buildRegistry(); return cachedRegistry; } diff --git a/server/services/manualReviewToolService/modules/CommentOperations.test.ts b/server/services/manualReviewToolService/modules/CommentOperations.test.ts index c87edc8d..ad94eb5d 100644 --- a/server/services/manualReviewToolService/modules/CommentOperations.test.ts +++ b/server/services/manualReviewToolService/modules/CommentOperations.test.ts @@ -6,6 +6,7 @@ import createUser from '../../../test/fixtureHelpers/createUser.js'; import { makeTestWithFixture } from '../../../test/utils.js'; import { UserPermission } from '../../userManagementService/index.js'; import CommentOperations from './CommentOperations.js'; +import { type JobId } from '../manualReviewToolService.js'; describe('CommentOperations', () => { const testWithFixtures = makeTestWithFixture(async () => { @@ -55,7 +56,7 @@ describe('CommentOperations', () => { .insertInto('manual_review_tool.job_creations') .values([ { - id: jobId1 as any, + id: jobId1 as JobId, org_id: orgId, item_id: itemId, item_type_id: itemTypeId, @@ -64,7 +65,7 @@ describe('CommentOperations', () => { enqueue_source_info: {}, }, { - id: jobId2 as any, + id: jobId2 as JobId, org_id: orgId, item_id: itemId, item_type_id: itemTypeId, @@ -121,8 +122,7 @@ describe('CommentOperations', () => { async ({ commentOps, orgId }) => { const nonExistentJobId = uuidv1(); - // Access private method for testing - const result = await (commentOps as any).getRelatedJobIds({ + const result = await commentOps['getRelatedJobIds']({ orgId, jobId: nonExistentJobId, }); @@ -134,7 +134,7 @@ describe('CommentOperations', () => { testWithFixtures( 'should return all related job IDs when job found in job_creations', async ({ commentOps, orgId, jobId1, jobId2 }) => { - const result = await (commentOps as any).getRelatedJobIds({ + const result = await commentOps['getRelatedJobIds']({ orgId, jobId: jobId1, }); diff --git a/server/services/manualReviewToolService/modules/CommentOperations.ts b/server/services/manualReviewToolService/modules/CommentOperations.ts index 88edd2be..6d43b3e3 100644 --- a/server/services/manualReviewToolService/modules/CommentOperations.ts +++ b/server/services/manualReviewToolService/modules/CommentOperations.ts @@ -4,6 +4,7 @@ import { v1 as uuidv1 } from 'uuid'; import { makeNotFoundError } from '../../../utils/errors.js'; import { isForeignKeyViolationError } from '../../../utils/kysely.js'; import { type ManualReviewToolServicePg } from '../dbTypes.js'; +import { type JobId } from '../manualReviewToolService.js'; export type ManualReviewJobComment = { id: string; @@ -30,7 +31,7 @@ export default class CommentOperations { .selectFrom('manual_review_tool.job_creations') .select(['item_id', 'item_type_id']) .where('org_id', '=', orgId) - .where('id', '=', jobId as any) + .where('id', '=', jobId as JobId) .executeTakeFirst(); if (!currentJob) { @@ -58,7 +59,7 @@ export default class CommentOperations { .selectFrom('manual_review_tool.job_comments') .select(manualReviewCommentDbSelection) .where('org_id', '=', orgId) - .where('job_id', 'in', jobIds as any[]) + .where('job_id', 'in', jobIds as JobId[]) .orderBy('created_at', 'asc') .execute(); @@ -73,7 +74,7 @@ export default class CommentOperations { .selectFrom('manual_review_tool.job_comments') .select((eb) => eb.fn.count('id').as('count')) .where('org_id', '=', orgId) - .where('job_id', 'in', jobIds as any[]) + .where('job_id', 'in', jobIds as JobId[]) .executeTakeFirst(); return result?.count ? Number(result.count) : 0; diff --git a/server/services/ncmecService/ncmecReporting.ts b/server/services/ncmecService/ncmecReporting.ts index 3b888676..d3b1a50e 100644 --- a/server/services/ncmecService/ncmecReporting.ts +++ b/server/services/ncmecService/ncmecReporting.ts @@ -1919,40 +1919,34 @@ export default class NcmecReporting { if (!fileAnnotations || fileAnnotations.length === 0) { return undefined; } - return { - ...(fileAnnotations.includes( - NCMECFileAnnotation.ANIME_DRAWING_VIRTUAL_HENTAI, - ) - ? { animeDrawingVirtualHentai: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.POTENTIAL_MEME) - ? { potentialMeme: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.VIRAL) - ? { viral: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.POSSIBLE_SELF_PRODUCTION) - ? { possibleSelfProduction: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.PHYSICAL_HARM) - ? { physicalHarm: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.VIOLENCE_GORE) - ? { violenceGore: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.BESTIALITY) - ? { bestiality: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.LIVE_STREAMING) - ? { liveStreaming: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.INFANT) - ? { infant: undefined } - : {}), - ...(fileAnnotations.includes(NCMECFileAnnotation.GENERATIVE_AI) - ? { generativeAi: undefined } - : {}), + // Map each NCMEC annotation enum value to the corresponding XML field + // name. Iterating through the table avoids one logical branch per + // annotation, which previously pushed cyclomatic complexity over the + // configured ESLint limit and made the function hard to extend when new + // annotation types are added. + const annotationFieldByType: Record< + NCMECFileAnnotationType, + keyof FileAnnotations + > = { + [NCMECFileAnnotation.ANIME_DRAWING_VIRTUAL_HENTAI]: + 'animeDrawingVirtualHentai', + [NCMECFileAnnotation.POTENTIAL_MEME]: 'potentialMeme', + [NCMECFileAnnotation.VIRAL]: 'viral', + [NCMECFileAnnotation.POSSIBLE_SELF_PRODUCTION]: 'possibleSelfProduction', + [NCMECFileAnnotation.PHYSICAL_HARM]: 'physicalHarm', + [NCMECFileAnnotation.VIOLENCE_GORE]: 'violenceGore', + [NCMECFileAnnotation.BESTIALITY]: 'bestiality', + [NCMECFileAnnotation.LIVE_STREAMING]: 'liveStreaming', + [NCMECFileAnnotation.INFANT]: 'infant', + [NCMECFileAnnotation.GENERATIVE_AI]: 'generativeAi', }; + + const result: FileAnnotations = {}; + for (const annotation of fileAnnotations) { + const field = annotationFieldByType[annotation]; + result[field] = undefined; + } + return result; } async #uploadFileDetails( diff --git a/server/services/orgAwareSignalExecutionService/signalExecutionService.test.ts b/server/services/orgAwareSignalExecutionService/signalExecutionService.test.ts index 539a876c..9e5e1836 100644 --- a/server/services/orgAwareSignalExecutionService/signalExecutionService.test.ts +++ b/server/services/orgAwareSignalExecutionService/signalExecutionService.test.ts @@ -30,10 +30,10 @@ describe('Signal Execution Service', () => { // eslint-disable-next-line functional/immutable-data mockLocationsLoader.close = jest.fn(); - // eslint-disable-next-line functional/immutable-data + /* eslint-disable functional/immutable-data, @typescript-eslint/no-explicit-any */ (mockTextBankStringsLoader as any).close = jest.fn(); - // eslint-disable-next-line functional/immutable-data, @typescript-eslint/no-explicit-any (mockGetImageBank as any).close = jest.fn(); + /* eslint-enable functional/immutable-data, @typescript-eslint/no-explicit-any */ describe('getTransientRunSignalWithCache', () => { beforeAll(async () => { @@ -133,10 +133,13 @@ describe('Signal Execution Service', () => { // signals don't hit the network. (The point of the mock is just so we can // spy on how many times runSignal was called.) const signalsServiceSpy = (await getBottle()).container.SignalsService; - // eslint-disable-next-line functional/immutable-data + /* eslint-disable functional/immutable-data, @typescript-eslint/no-explicit-any -- + jest.fn doesn't preserve the original method's overloads, so we need + a cast to assign back onto the typed "runSignal" property. */ signalsServiceSpy.runSignal = jest.fn( signalsServiceSpy.runSignal.bind(signalsServiceSpy), ) as any; + /* eslint-enable functional/immutable-data, @typescript-eslint/no-explicit-any */ const runSignal = makeGetTransientRunSignalWithCache( mockLocationsLoader, diff --git a/server/services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.ts b/server/services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.ts index 9afbdda3..dd3340ec 100644 --- a/server/services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.ts +++ b/server/services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.ts @@ -73,7 +73,11 @@ export default inject( const ruleNowInAlarm = newAlarmStatusByRule[rule.id].status === RuleAlarmStatus.ALARM; - const orgRow = orgsForChangedRules[rule.org_id]; + // Use a "Partial" view to make the index access return the honest + // "OrgAlertRow | undefined" so the runtime guard stays meaningful. + const orgRow = (orgsForChangedRules as Partial)[ + rule.org_id + ]; if (!orgRow) { return []; } diff --git a/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.test.ts b/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.test.ts index 25b89d50..d2217942 100644 --- a/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.test.ts +++ b/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.test.ts @@ -13,7 +13,7 @@ describe('getRuleAnomalyDetectionStatistics', () => { let queryMock: MockedFn< ( query: string, - tracer: any, + tracer: unknown, binds?: readonly unknown[], ) => Promise >; @@ -33,6 +33,7 @@ describe('getRuleAnomalyDetectionStatistics', () => { // Scope of this is just the test suite, so mutation should be ok. + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- jest.fn() defaults to a generic call signature; cast lets us use the typed "queryMock" declared above. queryMock = jest.fn() as any; queryMock.mockResolvedValue(queryResult); @@ -48,6 +49,7 @@ describe('getRuleAnomalyDetectionStatistics', () => { getRulePassStatistics = makeGetRuleAnomalyDetectionStatistics( dataWarehouseMock, + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- the second injected arg (tracer) is unused by this code path; passing an empty object stub. {} as any, ); }); diff --git a/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.ts b/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.ts index 198d4347..c9dbe075 100644 --- a/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.ts +++ b/server/services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.ts @@ -44,7 +44,7 @@ const makeGetRuleAnomalyDetectionaStatistics = // UTC time, which is what we need (current_timestamp() is server-local time). const [conditions, conditionBindValues] = unzip2([ ...(!includePeriodsInProgress - ? [['ts_end_exclusive <= SYSDATE()', [] as any] as const] + ? [['ts_end_exclusive <= SYSDATE()', [] as string[]] as const] : []), ...(startTime ? [['ts_start_inclusive >= ?', startTime] as const] : []), ...(ruleIds @@ -77,20 +77,23 @@ const makeGetRuleAnomalyDetectionaStatistics = bindValues, ); - return results.map((result: any) => ({ - ruleId: result.RULE_ID, - // name is a reminder that JS may trim the precision on the Date here, - // but that should be ok for our purposes. - approxRuleVersion: new Date(result.RULE_VERSION), - // nb: the warehouse returned value for a timestamp is a JS Date, but with - // some extra methods attached to it. These methods include toString, so - // we cast back to a proper Date to avoid the string representation - // changing (e.g., when serializing to JSON). - windowStart: new Date(result.TS_START_INCLUSIVE), - passCount: result.NUM_PASSES, - passingUsersCount: result.NUM_DISTINCT_USERS, - runsCount: result.NUM_RUNS, - })); + return results.map((result) => { + const row = result as Record; + return { + ruleId: row.RULE_ID as string, + // name is a reminder that JS may trim the precision on the Date here, + // but that should be ok for our purposes. + approxRuleVersion: new Date(row.RULE_VERSION as string | number | Date), + // nb: the warehouse returned value for a timestamp is a JS Date, but with + // some extra methods attached to it. These methods include toString, so + // we cast back to a proper Date to avoid the string representation + // changing (e.g., when serializing to JSON). + windowStart: new Date(row.TS_START_INCLUSIVE as string | number | Date), + passCount: row.NUM_PASSES as number, + passingUsersCount: row.NUM_DISTINCT_USERS as number, + runsCount: row.NUM_RUNS as number, + }; + }); }; export default inject( diff --git a/server/storage/dataWarehouse/ClickhouseAdapter.ts b/server/storage/dataWarehouse/ClickhouseAdapter.ts index 1f1615a2..70b8a744 100644 --- a/server/storage/dataWarehouse/ClickhouseAdapter.ts +++ b/server/storage/dataWarehouse/ClickhouseAdapter.ts @@ -104,6 +104,7 @@ function createDialect(client: ClickHouseClient): Dialect { createAdapter(): DialectAdapter { return new PostgresAdapter(); }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Kysely's Dialect interface hardcodes Kysely for createIntrospector createIntrospector(db: Kysely) { return new PostgresIntrospector(db); }, @@ -112,8 +113,7 @@ function createDialect(client: ClickHouseClient): Dialect { export class ClickhouseKyselyAdapter implements IDataWarehouseDialect { private readonly client: ClickHouseClient; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - private readonly kysely: Kysely; + private readonly kysely: Kysely>; constructor( connectionSettings: ClickhouseConnectionSettings, @@ -136,13 +136,14 @@ export class ClickhouseKyselyAdapter implements IDataWarehouseDialect { }, }); - this.kysely = new Kysely({ + this.kysely = new Kysely>({ dialect: createDialect(this.client), }); } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getKyselyInstance(): Kysely { + // The interface declares Kysely but we implement with a concrete schema type; + // callers that need a specific schema cast the result themselves. + getKyselyInstance(): Kysely> { return this.kysely; } diff --git a/server/storage/dataWarehouse/IDataWarehouseAnalytics.ts b/server/storage/dataWarehouse/IDataWarehouseAnalytics.ts index 9744907d..520a0647 100644 --- a/server/storage/dataWarehouse/IDataWarehouseAnalytics.ts +++ b/server/storage/dataWarehouse/IDataWarehouseAnalytics.ts @@ -91,38 +91,54 @@ export type AnalyticsSchema = { error_message?: string; }; + // Written by ContentApiLogger. Keep this shape in sync with the + // `bulkWrite('CONTENT_API_REQUESTS', ...)` call there so the analytics + // adapter can type-check the row contents at compile time. CONTENT_API_REQUESTS: { ds: string; ts: number; org_id: string; item_id: string; + item_data: string; item_type_id: string; - item_submission_id?: string; + item_type_kind: string; + item_type_name: string; + item_type_version: string; + item_type_schema: string; + item_type_schema_variant: string; + item_type_schema_field_roles?: Record; item_creator_id?: string; - endpoint: string; - method: string; - correlation_id: string; - duration_ms: number; - failed: boolean; - error_message?: string; + item_creator_type_id?: string; + request_id: string; + submission_id: string; + event: 'REQUEST_SUCCEEDED' | 'REQUEST_FAILED'; + failure_reason?: string; }; // Operational tables (using warehouse for operational data) 'REPORTING_SERVICE.REPORTS': { [key: string]: unknown; // Dynamic schema }; - + 'REPORTING_SERVICE.APPEALS': { [key: string]: unknown; // Dynamic schema }; - + 'USER_STATISTICS_SERVICE.USER_SCORES': { [key: string]: unknown; // Dynamic schema }; - + 'USER_STATISTICS_SERVICE.SUBMISSION_STATS': { [key: string]: unknown; // Dynamic schema }; + + 'REPORTING_SERVICE.REPORTING_RULE_EXECUTIONS': { + [key: string]: unknown; // Logged by ReportingRuleExecutionLogger + }; + + 'MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS': { + [key: string]: unknown; // Logged by RoutingRuleExecutionLogger + }; }; /** @@ -222,7 +238,7 @@ export const ANALYTICS_SCHEMA_DOCS = { }, ACTION_EXECUTIONS: { description: 'Logs every action execution (moderation actions taken)', - partitionKey: 'ds', + partitionKey: 'ds', sortKey: 'ts', indexes: ['org_id', 'action_id', 'item_id'], }, @@ -272,4 +288,3 @@ CREATE TABLE rule_executions ( PARTITION BY ds ORDER BY (ds, ts, org_id); `; - diff --git a/server/storage/dataWarehouse/PostgresAnalyticsAdapter.ts b/server/storage/dataWarehouse/PostgresAnalyticsAdapter.ts index 74dce5ac..674073ff 100644 --- a/server/storage/dataWarehouse/PostgresAnalyticsAdapter.ts +++ b/server/storage/dataWarehouse/PostgresAnalyticsAdapter.ts @@ -11,7 +11,7 @@ * - See ../README.md for implementation guide */ -import { sql, type Kysely } from 'kysely'; +import { sql, type InsertObject, type Kysely } from 'kysely'; import type SafeTracer from '../../utils/SafeTracer.js'; import { type IDataWarehouseAnalytics, @@ -26,9 +26,14 @@ import { * Uses batch inserts and logical replication for CDC */ export class PostgresAnalyticsAdapter implements IDataWarehouseAnalytics { - private pendingWrites: Map = new Map(); + // Each entry pairs a row buffer with a typed flush closure captured while + // TableName is in scope in bulkWrite, so flushTable never needs a cast. + private pendingWrites = new Map< + string, + { rows: unknown[]; flush: (rows: unknown[]) => Promise } + >(); - constructor(private readonly kysely: Kysely) {} + constructor(private readonly kysely: Kysely) {} async bulkWrite( tableName: TableName, @@ -38,14 +43,24 @@ export class PostgresAnalyticsAdapter implements IDataWarehouseAnalytics { const tableKey = tableName as string; if (!this.pendingWrites.has(tableKey)) { - this.pendingWrites.set(tableKey, []); + this.pendingWrites.set(tableKey, { + rows: [], + // Closure captures the concrete TableName so Kysely can type-check + // the insert at the call site where the generic is still in scope. + flush: async (r) => { + await this.kysely + .insertInto(tableName) + .values(r as ReadonlyArray>) + .execute(); + }, + }); } - this.pendingWrites.get(tableKey)!.push(...rows); + this.pendingWrites.get(tableKey)!.rows.push(...rows); const batchSize = config?.batchSize ?? 500; const pending = this.pendingWrites.get(tableKey)!; - if (config?.batchTimeout === 0 || pending.length >= batchSize) { + if (config?.batchTimeout === 0 || pending.rows.length >= batchSize) { await this.flushTable(tableKey); } } @@ -87,36 +102,36 @@ export class PostgresAnalyticsAdapter implements IDataWarehouseAnalytics { } private async flushTable(tableName: string): Promise { - const rows = this.pendingWrites.get(tableName); - if (!rows || rows.length === 0) return; + const pending = this.pendingWrites.get(tableName); + if (!pending || pending.rows.length === 0) return; - await this.kysely.insertInto(tableName as any).values(rows).execute(); - this.pendingWrites.set(tableName, []); + await pending.flush(pending.rows); + pending.rows = []; } // Stub implementations - integrators must implement these - logActionExecutions = async (..._args: any[]): Promise => { + logActionExecutions = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logRuleExecutions = async (..._args: any[]): Promise => { + logRuleExecutions = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logItemModelScore = async (..._args: any[]): Promise => { + logItemModelScore = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logReportingRuleExecutions = async (..._args: any[]): Promise => { + logReportingRuleExecutions = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logContentApiRequest = async (..._args: any[]): Promise => { + logContentApiRequest = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logContentDetailsApiRequest = async (..._args: any[]): Promise => { + logContentDetailsApiRequest = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logRoutingRuleExecutions = async (..._args: any[]): Promise => { + logRoutingRuleExecutions = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; - logOrgCreation = async (..._args: any[]): Promise => { + logOrgCreation = async (..._args: unknown[]): Promise => { throw new Error('Not implemented'); }; } diff --git a/server/test/extendExpect.ts b/server/test/extendExpect.ts index 65a4afb0..9717581b 100644 --- a/server/test/extendExpect.ts +++ b/server/test/extendExpect.ts @@ -1,5 +1,5 @@ // eslint-disable-next-line import/no-extraneous-dependencies -import jestSnapshot from 'jest-snapshot'; +import jestSnapshot, { type Context as SnapshotContext } from 'jest-snapshot'; import { JSONPath } from 'jsonpath-plus'; import lodash from 'lodash'; @@ -41,7 +41,11 @@ declare global { } expect.extend({ - toMatchDynamicSnapshot(received, propertyMatchers: object, hint?: string) { + toMatchDynamicSnapshot( + received, + propertyMatchers: object, + hint?: string, + ): jest.CustomMatcherResult | Promise { // Treat property matcher keys as jsonpath queries // if they start with a $ and contain a dot. const isJsonPath = (it: string) => it[0] === '$' && it.includes('.'); @@ -63,8 +67,8 @@ expect.extend({ } }); - return (toMatchSnapshot as any).call( - this, + return toMatchSnapshot.call( + this as SnapshotContext, received, generatedPropertyMatchers, hint, diff --git a/server/utils/errors.ts b/server/utils/errors.ts index 08bdb0d2..9185fcd0 100644 --- a/server/utils/errors.ts +++ b/server/utils/errors.ts @@ -261,6 +261,9 @@ export type CoopErrorName = | 'BadRequestError' | 'UnauthenticatedError' | 'UnauthorizedError' + // apiKey errors + | 'RotateApiKeyError' + | 'RotateWebhookSigningKeyError' // gql mutation errors | UserErrorType | IntegrationErrorType diff --git a/server/utils/sql.ts b/server/utils/sql.ts index 3b66f1bb..01440873 100644 --- a/server/utils/sql.ts +++ b/server/utils/sql.ts @@ -65,9 +65,5 @@ export function takeLast< for (const it of sortCriteria) { outer = outer.orderBy(it.column, it.order); } - return outer as SelectQueryBuilder< - DB & { [K in typeof SUBQUERY_ALIAS]: O }, - typeof SUBQUERY_ALIAS, - O - >; + return outer; } diff --git a/server/workers_jobs/RefreshMRTDecisionsMaterializedViewJob.ts b/server/workers_jobs/RefreshMRTDecisionsMaterializedViewJob.ts index c03b76ea..d3e3c863 100644 --- a/server/workers_jobs/RefreshMRTDecisionsMaterializedViewJob.ts +++ b/server/workers_jobs/RefreshMRTDecisionsMaterializedViewJob.ts @@ -25,11 +25,14 @@ export default inject( throw new Error('No last_insert timestamp found for the table'); } - // When last_insert is null (fresh deploy), do a full backfill - const isInitialBackfill = lastTimestamp.last_insert == null; - const oneMinutePrevious = isInitialBackfill - ? new Date(0) - : new Date(lastTimestamp.last_insert.valueOf() - MINUTE_MS); + // On a fresh deploy the row is seeded with Postgres `-infinity`, + // which `pg-types` parses to the JS number `-Infinity`. In that case + // `new Date(-Infinity - MINUTE_MS)` would be an Invalid Date, so fall + // back to epoch and do a full backfill. + const lastInsertMs = lastTimestamp.last_insert.valueOf(); + const oneMinutePrevious = Number.isFinite(lastInsertMs) + ? new Date(lastInsertMs - MINUTE_MS) + : new Date(0); const insertedRows = await trx .insertInto('manual_review_tool.dim_mrt_decisions_materialized') diff --git a/server/workers_jobs/RetryFailedNcmecDecisionsJob.ts b/server/workers_jobs/RetryFailedNcmecDecisionsJob.ts index a554b725..18d9df63 100644 --- a/server/workers_jobs/RetryFailedNcmecDecisionsJob.ts +++ b/server/workers_jobs/RetryFailedNcmecDecisionsJob.ts @@ -53,10 +53,7 @@ export default inject( const orgId = row.org_id; const itemId = row.job_payload.payload.item.itemId; const itemTypeId = row.job_payload.payload.item.itemTypeIdentifier.id; - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - if (usersByOrg[orgId] === undefined) { - usersByOrg[orgId] = await userManagementService.getUsersForOrg(orgId); - } + usersByOrg[orgId] ??= await userManagementService.getUsersForOrg(orgId); const user = usersByOrg[orgId].find((it) => it.id === itemId); if (itemType === undefined || itemType.kind !== 'USER') { await ncmecService.insertOrUpdateNcmecReportError({