diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d87ae94b..959d1a7f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -38,6 +38,9 @@ jobs: - name: Lint Code Base run: npm run lint + - name: Run Unit Tests + run: npm test + - name: Compile Dataform Models run: npm run compile diff --git a/includes/reports.js b/includes/reports.js index 9ee48819..e9f21c37 100644 --- a/includes/reports.js +++ b/includes/reports.js @@ -50,22 +50,26 @@ class DataformTemplateBuilder { * @returns {function} A function that can be called with a context to resolve the template */ static create(templateFn) { - return (ctx, params) => { + if (typeof templateFn !== 'function') { + throw new TypeError('templateFn must be a function') + } + + return (ctx = {}, params = {}) => { // Custom replacer function to handle nested variables const resolveVariable = (path, scope) => { // Split the path into parts (handles nested objects like 'constants.devRankFilter') const parts = path.split('.') - // Traverse the provided scope (ctx or global) to find the value + // Traverse the provided scope (ctx or params) to find the value let value = scope for (const part of parts) { if (value === undefined || value === null) break value = value[part] } - // Convert value to appropriate string representation + // Convert value to appropriate string representation without wrapping identifiers in quotes if (value === undefined || value === null) return '' - if (typeof value === 'string') return `'${value}'` + if (typeof value === 'string') return value if (typeof value === 'number') return value.toString() if (typeof value === 'boolean') return value.toString() if (typeof value === 'function') return value.toString() @@ -74,7 +78,7 @@ class DataformTemplateBuilder { return JSON.stringify(value) } - // Generate the template with the provided context and global context + // Generate the template with the provided context and parameters return templateFn(ctx, params).replace(/\${(.*?)}/g, (match, p1) => { const [scope, path] = p1.includes(':') ? p1.split(':') @@ -3312,14 +3316,16 @@ const lensArrayExpression = `ARRAY[ ]` class HTTPArchiveReports { - constructor() { - this.config = config + constructor(customConfig = config) { + this.config = customConfig this.lenses = lenses this.lensArrayExpression = lensArrayExpression } listReports() { - const reportIds = this.config._reports + const reportIds = Array.isArray(this.config._reports) + ? this.config._reports + : [] const reports = reportIds.map((reportId) => { const report = this.getReport(reportId) @@ -3330,6 +3336,15 @@ class HTTPArchiveReports { } getReport(reportId) { + if ( + typeof reportId !== 'string' || + !reportId || + reportId.startsWith('_') || + !Object.prototype.hasOwnProperty.call(this.config, reportId) + ) { + throw new Error(`Unknown report ID: ${reportId}`) + } + const report = this.config[reportId] return { id: reportId, @@ -3363,6 +3378,14 @@ class HTTPArchiveReports { } getMetric(metricId) { + if ( + typeof metricId !== 'string' || + !metricId || + !Object.prototype.hasOwnProperty.call(this.config._metrics, metricId) + ) { + throw new Error(`Unknown metric ID: ${metricId}`) + } + const metric = this.config._metrics[metricId] return { @@ -3373,6 +3396,7 @@ class HTTPArchiveReports { } module.exports = { + DataformTemplateBuilder, HTTPArchiveReports, lenses, lensArrayExpression diff --git a/package.json b/package.json index d2d3472d..ed5e3a83 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,8 @@ "scripts": { "compile": "dataform compile", "format": "prettier --write . && eslint --fix . && markdownlint . --fix", - "lint": "prettier --check . && eslint . && markdownlint ." + "lint": "prettier --check . && eslint . && markdownlint .", + "test": "node --test tests/*.test.js" }, "dependencies": { "@dataform/core": "3.1.2", diff --git a/tests/reports.test.js b/tests/reports.test.js new file mode 100644 index 00000000..3c545ae5 --- /dev/null +++ b/tests/reports.test.js @@ -0,0 +1,230 @@ +const { describe, it } = require('node:test') +const nodeAssert = require('node:assert/strict') + +const { + DataformTemplateBuilder, + HTTPArchiveReports, + lenses, + lensArrayExpression +} = require('../includes/reports.js') + +const createMockCtx = () => ({ + ref: (...parts) => `\`httparchive.${parts.join('.')}\``, + constants: { + devRankFilter: 'AND rank <= 10000' + } +}) + +describe('DataformTemplateBuilder', () => { + it('throws TypeError when templateFn is not a function', () => { + for (const invalidInput of [null, undefined, 'SELECT 1', 123, {}]) { + nodeAssert.throws( + () => DataformTemplateBuilder.create(invalidInput), + TypeError + ) + } + }) + + it('interpolates template literals with ctx and params', () => { + const query = DataformTemplateBuilder.create( + (mockCtx, params) => + `SELECT * FROM ${mockCtx.ref('crawl', 'pages')} WHERE date = '${params.date}'` + ) + + const sql = query(createMockCtx(), { date: '2026-10-01' }) + nodeAssert.equal( + sql, + "SELECT * FROM `httparchive.crawl.pages` WHERE date = '2026-10-01'" + ) + }) + + it('resolves deferred string placeholders without wrapping identifiers in single quotes', () => { + const query = DataformTemplateBuilder.create( + () => + "SELECT * FROM ${params:table} WHERE date = '${params:date}' ${ctx:constants.devRankFilter}" + ) + + const sql = query(createMockCtx(), { + table: '`httparchive.crawl.pages`', + date: '2026-10-01' + }) + + nodeAssert.equal( + sql, + "SELECT * FROM `httparchive.crawl.pages` WHERE date = '2026-10-01' AND rank <= 10000" + ) + }) + + it('handles nested paths, numbers, booleans, objects, and missing values', () => { + const query = DataformTemplateBuilder.create( + () => + 'LIMIT ${nested.limit} FLAG=${flag} TAGS=${tags} MISSING=${missing.key}' + ) + + const sql = query(createMockCtx(), { + nested: { limit: 100 }, + flag: true, + tags: ['a', 'b'] + }) + + nodeAssert.equal(sql, 'LIMIT 100 FLAG=true TAGS=["a","b"] MISSING=') + }) +}) + +describe('HTTPArchiveReports', () => { + const reportsInstance = new HTTPArchiveReports() + + it('exports expected lenses and lensArrayExpression', () => { + nodeAssert.equal(typeof lenses, 'object') + nodeAssert.ok(lenses !== null && !Array.isArray(lenses)) + for (const expectedLens of [ + 'all', + 'drupal', + 'magento', + 'top1k', + 'top10k', + 'top100k', + 'top1m', + 'wordpress' + ]) { + nodeAssert.ok( + Object.prototype.hasOwnProperty.call(lenses, expectedLens), + `Expected lens "${expectedLens}" to be present` + ) + nodeAssert.equal(typeof lenses[expectedLens], 'string') + } + nodeAssert.equal(typeof lensArrayExpression, 'string') + nodeAssert.ok(lensArrayExpression.startsWith('ARRAY[')) + }) + + it('lists configured reports with valid metadata', () => { + nodeAssert.deepEqual(reportsInstance.listReports(), []) + + const customReports = new HTTPArchiveReports({ + _reports: ['page-weight'], + 'page-weight': { + name: 'Page Weight', + metrics: ['bytesTotal', 'bytesJs'] + }, + _metrics: reportsInstance.config._metrics + }) + + const reportList = customReports.listReports() + nodeAssert.equal(reportList.length, 1) + nodeAssert.equal(reportList[0].id, 'page-weight') + nodeAssert.equal(reportList[0].name, 'Page Weight') + nodeAssert.deepEqual(reportList[0].metrics, ['bytesTotal', 'bytesJs']) + }) + + it('validates reportId in getReport and listMetrics', () => { + for (const invalidId of [ + 'non-existent-report', + '_metrics', + '_reports', + '', + null, + undefined, + 123 + ]) { + nodeAssert.throws( + () => reportsInstance.getReport(invalidId), + /Unknown report ID/ + ) + } + + nodeAssert.throws( + () => reportsInstance.listMetrics('non-existent-report'), + /Unknown report ID/ + ) + + const customReports = new HTTPArchiveReports({ + _reports: ['page-weight'], + 'page-weight': { + name: 'Page Weight', + metrics: ['bytesTotal', 'bytesJs'] + }, + _metrics: reportsInstance.config._metrics + }) + + const validReport = customReports.getReport('page-weight') + nodeAssert.equal(validReport.id, 'page-weight') + nodeAssert.equal(validReport.name, 'Page Weight') + + const reportMetrics = customReports.listMetrics('page-weight') + nodeAssert.equal(reportMetrics.length, 2) + nodeAssert.deepEqual( + reportMetrics.map((m) => m.id), + ['bytesTotal', 'bytesJs'] + ) + }) + + it('validates metricId in getMetric', () => { + const metric = reportsInstance.getMetric('bytesTotal') + nodeAssert.equal(metric.id, 'bytesTotal') + nodeAssert.ok(Array.isArray(metric.SQL)) + + for (const invalidId of [ + 'nonExistentMetric', + '', + null, + undefined, + '__proto__' + ]) { + nodeAssert.throws( + () => reportsInstance.getMetric(invalidId), + /Unknown metric ID/ + ) + } + }) + + it('compiles SQL queries cleanly for every configured metric', () => { + const allMetrics = reportsInstance.listMetrics() + nodeAssert.ok(allMetrics.length > 0) + + const mockCtx = createMockCtx() + const params = { date: '2026-10-01' } + + for (const metric of allMetrics) { + nodeAssert.ok( + Array.isArray(metric.SQL) && metric.SQL.length > 0, + `Metric "${metric.id}" must define non-empty SQL array` + ) + + for (const entry of metric.SQL) { + nodeAssert.ok( + ['histogram', 'timeseries'].includes(entry.type), + `Metric "${metric.id}" has unexpected query type "${entry.type}"` + ) + nodeAssert.equal( + typeof entry.query, + 'function', + `Metric "${metric.id}" (${entry.type}) query must be a function` + ) + + const sql = entry.query(mockCtx, params) + nodeAssert.equal(typeof sql, 'string') + nodeAssert.ok(sql.includes('SELECT'), `${metric.id} missing SELECT`) + nodeAssert.ok( + sql.includes("'2026-10-01'"), + `${metric.id} (${entry.type}) missing rendered date literal` + ) + nodeAssert.ok( + sql.includes('`httparchive.crawl.'), + `${metric.id} (${entry.type}) missing resolved table reference` + ) + nodeAssert.ok( + !sql.includes("'undefined'"), + `${metric.id} (${entry.type}) contains literal 'undefined'` + ) + nodeAssert.ok( + !sql.includes("'null'"), + `${metric.id} (${entry.type}) contains literal 'null'` + ) + nodeAssert.ok( + !sql.includes("''2026-10-01''"), + `${metric.id} (${entry.type}) contains double-quoted date literal` + ) + } + } + }) +})