chore(sonar): remove the rejected Tinqer spike, clear the lint debt, and report measures in CI (#261)
The standing cleanup, three features behind. Four changes. Report the measures in CI. This is the one that matters, because the rest was only findable by reading the tree. SonarQube here is 9.9 Community: no Bearer auth, so the official MCP cannot connect, and the host is a CI secret, so hotspots, duplication, debt and coverage existed only on a dashboard — which made "reduce the debt" an instruction nobody could act on without a browser open beside them. scripts/summarize-sonar.js queries the measures API with the secrets the workflow already holds and prints the result into the job log. The scanner masks the URL and token; measures are not secret. It polls the compute task before reading. The workflow does not set sonar.qualitygate.wait, so the scan step returns once the report is uploaded and the server computes measures afterwards — reading immediately would return the previous analysis, indistinguishable from this one and quietly wrong. When it cannot confirm, it says so in the output rather than presenting stale numbers as current. It is deliberately not guarded with continue-on-error: it exits 0 on every path, and guarding it would oblige it to appear in the final gate, whose job is to fail the build. Remove the Tinqer spike. #216 evaluated Drizzle against Tinqer and rejected Tinqer, and its closing comment said the throwaway src/db-tinqer/ probe must not reach main. The whole spike commit was merged, so it did. The probe is 71 lines imported by nothing, and @tinqerjs/tinqer, @tinqerjs/pg-promise-adapter and pg-promise were dependencies for a library nobody chose. The condition_note column that warning also named did not reach main. Clear the lint debt, both projects now at zero warnings from six and two. One of these was a real defect rather than tidiness: the third catch block in shippingAddresses.ts rolled back and returned 500 while discarding the error, so a failed default-address change left nothing behind to say why — the two catch blocks above it in the same file already logged, and this one had simply been missed. The Express namespace augmentation is a false positive and is disabled with the reason written beside it, because an interface that must merge into one Express declares inside a namespace has no ES module spelling. Dedupe the extension map. backfillImageReencode.ts kept its own .jpg/.png/.webp table whose comment named uploadTypes.ts as the source of truth, directly above duplicating it. That file rewrites stored images, so the two disagreeing would silently skip files it should re-encode. src/db-drizzle/ deliberately stays. #217 is open to promote exactly those files properly, with tablesFilter and the sql.param() array rule; deleting them here would be doing #217 badly in the wrong issue. Only their unused-symbol warnings are fixed, and if drizzle-kit pull regenerates schema.ts the table warning returns — worth #217 knowing. Hotspots and coverage are untouched because both numbers are still invisible. They are the next pass, once the step above has printed them once. Closes #261 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -43,19 +43,11 @@ import { promises as fs } from 'fs';
|
||||
import path from 'path';
|
||||
import { pool } from './db';
|
||||
import { needsProcessing, reencodeInPlace } from './imageProcessing';
|
||||
import { typeForExtension } from './uploadTypes';
|
||||
|
||||
const UPLOADS_DIR = process.env.UPLOADS_DIR || '/app/uploads';
|
||||
const APPLY = process.argv.includes('--apply');
|
||||
|
||||
// The stored extension is the file's real type — uploadTypes.ts derives it from
|
||||
// the validated content type on the way in, so it can be trusted on the way
|
||||
// back out. Anything else is a file this application would refuse to serve.
|
||||
const TYPE_FOR_EXTENSION: Readonly<Record<string, string>> = {
|
||||
'.jpg': 'image/jpeg',
|
||||
'.png': 'image/png',
|
||||
'.webp': 'image/webp'
|
||||
};
|
||||
|
||||
interface Totals {
|
||||
seen: number;
|
||||
missing: number;
|
||||
@@ -78,7 +70,12 @@ async function handleRow(imagePath: string, totals: Totals): Promise<void> {
|
||||
// basename only: image_path is '/uploads/<name>', and the directory it is
|
||||
// served from is a server constant rather than part of the stored value.
|
||||
const filePath = path.join(UPLOADS_DIR, path.basename(imagePath));
|
||||
const mimetype = TYPE_FOR_EXTENSION[path.extname(filePath).toLowerCase()];
|
||||
// typeForExtension rather than a copy of its table. The stored extension is
|
||||
// the file's real type — uploadTypes.ts derives it from the validated content
|
||||
// type on the way in — and this rewrites stored images, so a private copy
|
||||
// drifting from the real one would silently skip files it should re-encode.
|
||||
// It lowercases its own input, so the call site does not.
|
||||
const mimetype = typeForExtension(path.extname(filePath));
|
||||
|
||||
if (!mimetype) {
|
||||
console.warn(`[backfill] unrecognised extension, skipping: ${imagePath}`);
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
// match with a count equality, and array parameters. If this cannot be said
|
||||
// cleanly, nothing else in the conversion matters.
|
||||
|
||||
import { SQL, and, eq, gte, lte, sql, inArray, exists } from 'drizzle-orm';
|
||||
import { SQL, and, gte, lte, sql, inArray, exists } from 'drizzle-orm';
|
||||
import { items, itemTags, favorites, categories } from './schema';
|
||||
|
||||
export interface SpikeFilters {
|
||||
|
||||
@@ -77,7 +77,7 @@ export const tags = pgTable("tags", {
|
||||
name: text().notNull(),
|
||||
color: text().notNull(),
|
||||
createdAt: timestamp("created_at", { withTimezone: true, mode: 'string' }).defaultNow().notNull(),
|
||||
}, (table) => [
|
||||
}, () => [
|
||||
uniqueIndex("tags_name_uniq").using("btree", sql`lower(name)`),
|
||||
]);
|
||||
|
||||
|
||||
@@ -1,71 +0,0 @@
|
||||
// Spike (#216): what Tinqer can and cannot express, recorded as runnable cases.
|
||||
//
|
||||
// Every case is wrapped in a function rather than evaluated at module level.
|
||||
// defineSelect() parses the lambda eagerly and THROWS on a shape it cannot
|
||||
// handle, so a top-level call turns an unsupported query into an import-time
|
||||
// crash. That is itself a finding worth keeping: the failure is a runtime throw
|
||||
// at load, not a type error, so an unsupported query compiles cleanly and takes
|
||||
// the process down when the module is first required.
|
||||
|
||||
import { createSchema, defineSelect } from '@tinqerjs/tinqer';
|
||||
import { toSql } from '@tinqerjs/pg-promise-adapter';
|
||||
|
||||
interface Db {
|
||||
items: { id: number; name: string; price_cents: number; status: string; category_id: number | null };
|
||||
categories: { id: number; parent_id: number | null };
|
||||
}
|
||||
|
||||
const schema = createSchema<Db>();
|
||||
|
||||
export type Outcome = { label: string; ok: boolean; sql?: string; error?: string };
|
||||
|
||||
function attempt(label: string, build: () => unknown, params: Record<string, unknown>): Outcome {
|
||||
try {
|
||||
const r = toSql(build() as never, params as never);
|
||||
return { label, ok: true, sql: r.sql };
|
||||
} catch (e) {
|
||||
return { label, ok: false, error: (e as Error).message.split('\n')[0] };
|
||||
}
|
||||
}
|
||||
|
||||
export function runCases(): Outcome[] {
|
||||
return [
|
||||
// Supported.
|
||||
attempt(
|
||||
'compound condition',
|
||||
() =>
|
||||
defineSelect(schema, (q, p: { min: number; max: number }) =>
|
||||
q.from('items').where((i) => i.price_cents >= p.min && i.price_cents <= p.max).select((i) => ({ id: i.id }))
|
||||
),
|
||||
{ min: 1, max: 2 }
|
||||
),
|
||||
attempt(
|
||||
'array membership',
|
||||
() =>
|
||||
defineSelect(schema, (q, p: { statuses: string[] }) =>
|
||||
q.from('items').where((i) => p.statuses.includes(i.status)).select((i) => ({ id: i.id }))
|
||||
),
|
||||
{ statuses: ['available', 'sold'] }
|
||||
),
|
||||
|
||||
// NOT supported — and these are the two shapes buildItemFilterSql needs.
|
||||
attempt(
|
||||
'ternary (clause optional at run time)',
|
||||
() =>
|
||||
defineSelect(schema, (q, p: { apply: boolean; min: number }) =>
|
||||
q.from('items').where((i) => (p.apply ? i.price_cents >= p.min : true)).select((i) => ({ id: i.id }))
|
||||
),
|
||||
{ apply: true, min: 1 }
|
||||
),
|
||||
attempt(
|
||||
'block body with if (clause optional at run time)',
|
||||
() =>
|
||||
defineSelect(schema, (q, p: { apply: boolean; min: number }) => {
|
||||
let x = q.from('items');
|
||||
if (p.apply) x = x.where((i) => i.price_cents >= p.min);
|
||||
return x.select((i) => ({ id: i.id }));
|
||||
}),
|
||||
{ apply: true, min: 1 }
|
||||
)
|
||||
];
|
||||
}
|
||||
@@ -2,6 +2,11 @@ import { Request, Response, NextFunction } from 'express';
|
||||
import { pool } from '../db';
|
||||
|
||||
declare global {
|
||||
// A namespace is the only way to spell an Express type augmentation — the
|
||||
// interface has to merge into the one Express declares, and Express declares
|
||||
// it inside a namespace. There is no ES module form of this, so the rule is
|
||||
// disabled here rather than worked around.
|
||||
// eslint-disable-next-line @typescript-eslint/no-namespace
|
||||
namespace Express {
|
||||
interface Request {
|
||||
customerId?: number;
|
||||
|
||||
@@ -118,6 +118,10 @@ router.post('/:id/set-default', requireCustomer, asyncRoute(async (req: Request,
|
||||
res.json(rows[0]);
|
||||
} catch (err) {
|
||||
await client.query('ROLLBACK');
|
||||
// Logged like the two catch blocks above it in this file, which this one
|
||||
// was simply missing. Without it a failed default-address change rolls
|
||||
// back and returns 500 leaving nothing behind to say why.
|
||||
console.error(err);
|
||||
res.status(500).json({ error: 'internal error' });
|
||||
} finally {
|
||||
client.release();
|
||||
|
||||
Reference in New Issue
Block a user