From d7fd22af0ab28d592e1f917f76684b64ec8e0809 Mon Sep 17 00:00:00 2001 From: Ross Jones Date: Thu, 11 Sep 2025 16:17:27 +0100 Subject: [PATCH] Explore using a Range class instead of an object Currently we pass around an object to act as our range, but this puts the onus on the caller to validate and also ensure that iterating over the range is accurate - this new Range class aims to encapsulate that functionality in one place --- lib/importer/src/dudk/backend.js | 90 ++++++---------- lib/importer/src/dudk/backend.test.js | 131 ++++++++++++----------- lib/importer/src/dudk/sheets.js | 57 +++++----- lib/importer/src/dudk/sheets.test.js | 11 +- lib/importer/src/dudk/types/date.test.js | 52 ++++----- lib/importer/src/dudk/util/range.js | 104 ++++++++++++++++++ lib/importer/src/dudk/util/range.test.js | 93 ++++++++++++++++ lib/importer/src/dudk/validation.test.js | 41 +++---- lib/importer/src/index.js | 31 +++--- lib/importer/src/session.js | 14 ++- lib/importer/src/session.test.js | 13 ++- 11 files changed, 405 insertions(+), 232 deletions(-) create mode 100644 lib/importer/src/dudk/util/range.js create mode 100644 lib/importer/src/dudk/util/range.test.js diff --git a/lib/importer/src/dudk/backend.js b/lib/importer/src/dudk/backend.js index fa6b85de..83cf80ef 100644 --- a/lib/importer/src/dudk/backend.js +++ b/lib/importer/src/dudk/backend.js @@ -5,6 +5,7 @@ const attributeTypes = require('./types/attribute-types'); const guesser = require('./types/guesser'); const store = require("./session_store") const errorTypes = require('./util/errors'); +const Range = require('./util/range'); // An implementation of the interface described in https://struct.register-dynamics.co.uk/trac/wiki/DataImporter/API @@ -78,13 +79,12 @@ exports.SessionGetSheets = (sid) => { exports.SessionSetHeaderRange = (sid, range) => { assert(range != null, "Null range passed to SessionSetHeaderRange"); assert(range.sheet != null, "Range with null sheet passed to SessionSetHeaderRange"); - sessionStore.apply(sid, (s) => { s.headerRanges[range.sheet] = range; }) }; exports.SessionGetHeaderRange = (sid, sheetName) => { let session = sessionStore.get(sid); - return session.headerRanges[sheetName] + return session.headerRanges[sheetName]; }; exports.SessionSetFooterRange = (sid, range) => { @@ -95,7 +95,7 @@ exports.SessionSetFooterRange = (sid, range) => { exports.SessionGetFooterRange = (sid, sheetName) => { let session = sessionStore.get(sid); - return session.footerRanges[sheetName] + return session.footerRanges[sheetName]; }; function getDimensions(sid) { @@ -237,46 +237,28 @@ exports.SessionSuggestDataRange = (sid, headerRange, footerRange) => { // the header. // Header and footer, so just go from the row below the header to the row above the footer - return { - sheet: sheetName, - start: { - row: headerRange.start.row + 1, - column: headerRange.start.column - }, - end: { - row: footerRange.start.row, - column: headerRange.end.column - } - }; + return new Range( + sheetName, + { row: headerRange.start.row + 1, column: headerRange.start.column }, + { row: footerRange.start.row, column: headerRange.end.column } + ); } else { // Header, but no footer, so just go from the row below the header to the end - return { - sheet: sheetName, - start: { - row: headerRange.start.row + 1, - column: headerRange.start.column - }, - end: { - row: sheetDimensions.rows - 1, - column: headerRange.end.column - } - }; + return new Range( + sheetName, + { row: headerRange.start.row + 1, column: headerRange.start.column }, + { row: sheetDimensions.rows - 1, column: headerRange.end.column } + ); } } else { // No header range specified, so take all but the first row of the first sheet const sheetName = sessionStore.get(sid).sheetNames[0]; // Find the first sheet const sheetDimensions = dimensions.sheetDimensions.get(sheetName); - return { - sheet: sheetName, - start: { - row: 1, - column: 0 - }, - end: { - row: sheetDimensions.rows - 1, - column: sheetDimensions.columns - 1 - } - }; + return new Range( + sheetName, + { row: 1, column: 0 }, + { row: sheetDimensions.rows - 1, column: sheetDimensions.columns - 1 } + ); } } @@ -375,7 +357,8 @@ function cellsToSamples(row) { return row.map(cellToSample); } -// Returns a sample of rows in a range. range is of the form {sheet: 'Foo', start:{row: X, column: Y}, end:{row: X, column: Y}}. +// Returns a sample of rows in a range. range is and instance of the Range class and +// is of the form {sheet: 'Foo', start:{row: X, column: Y}, end:{row: X, column: Y}}. // Returns three arrays - one with startCount rows from the top of the range, // one with a random selectino of middleCount rows from the middle, and another @@ -385,7 +368,8 @@ function cellsToSamples(row) { exports.SessionGetInputSampleRows = (sid, range, startCount, middleCount, endCount) => { assert(range != null, "Null range passed to SessionGetInputSampleRows"); assert(sessionStore.has(sid), `No such session ${sid} when getting input sample rows`); - assert(sessionStore.get(sid).wb.Sheets[range.sheet], `No such sheet ${range.sheet} in session ${sid} when getting input sample rows`); + assert(sessionStore.get(sid).wb.Sheets[range.sheet], `No such sheet ${range.sheet} in session ${sid} when getting input values`); + if (range.end.row < range.start.row || range.end.column < range.start.column) { // Range is empty return [[], [], []]; @@ -393,7 +377,6 @@ exports.SessionGetInputSampleRows = (sid, range, startCount, middleCount, endCou // If the caller asked for more rows than we have, or if the start/middle/end would overlap, clamp the counts const totalRowsInRange = (range.end.row - range.start.row + 1); [startCount, middleCount, endCount] = clampCounts(startCount, middleCount, endCount, totalRowsInRange); - const sheet = sessionStore.get(sid).wb.Sheets[range.sheet]; const data = sheet["!data"]; const merges = sheet["!merges"]; @@ -418,7 +401,6 @@ exports.SessionGetInputSampleRows = (sid, range, startCount, middleCount, endCou let endRows = extractColsFromRows(getMergedRows(sheet, range.end.row - endCount + 1, range.end.row), range.start.column, range.end.column + 1); - // Given an index generate a function that we can use to map across an array to turn it into // an object with both index and the array data. const mapWithIndex = (arr, startIndex) => { @@ -430,12 +412,6 @@ exports.SessionGetInputSampleRows = (sid, range, startCount, middleCount, endCou } } - // FIXME: Work out how the xlsx library represents styles and - // rowspan/colspan and make sure that what we return does something useful - // with that information. We DO want styling information to be available in - // the preview, so that users can see their spreadsheet in a more familiar - // form, and because styling information might be a significant part of the - // data. return [ startRows.map(mapWithIndex(startRows, range.start.row)), middleRows.map(mapWithIndex(middleRows, middleStart)), @@ -532,8 +508,8 @@ exports.SessionGuessTypes = (sid, range) => { // objects with string fields "displayName" and "description", and an optional Map // field "formats" that maps from format names (as returned by SessionGetTypes) // to objects with strings field "displayName" and "description". -exports.SessionGetSupportedTypes = (sid) => { - return types.supportedTypes; +exports.SessionGetSupportedTypes = (_sid) => { + return attributeTypes.supportedTypes; }; // Given guesses as returned by SessionGuessTypes and a list of domain-model @@ -573,8 +549,7 @@ exports.SessionSuggestFields = (sid, typeGuesses, domainModelFields) => { exports.SessionGetInputValues = (sid, range, maxValues) => { assert(sessionStore.get(sid), `No such session ${sid} when getting input values`); assert(sessionStore.get(sid).wb.Sheets[range.sheet], `No such sheet ${range.sheet} in session ${sid} when getting input values`); - assert(range.end.row >= range.start.row, `End row (${range.end.row}) must be >= start row (${range.start.row}) to get input values`); - assert(range.end.column >= range.start.column, `End column (${range.end.column}) must be >= start column (${range.start.column}) to get input values`); + assert(range.isValid(), `Range is invalid: end (${JSON.stringify(range.end)}) must be >= start (${JSON.stringify(range.start)}) to get input values`); const sheet = sessionStore.get(sid).wb.Sheets[range.sheet]; const data = sheet["!data"]; @@ -679,7 +654,7 @@ exports.SessionPerformMappingJob = (sid, range, mapping, includeErrorRow = false validateMapping(range, mapping); const records = new Array(); // Array of output records - const errors = new Array();; + const errors = new Array(); const warnings = new Array(); const sheet = sessionStore.get(sid).wb.Sheets[range.sheet]; @@ -688,15 +663,14 @@ exports.SessionPerformMappingJob = (sid, range, mapping, includeErrorRow = false const attrMap = Object.entries(mapping.attributeMappings); const attrTypes = mapping.attributeTypes || {}; - - // TODO: Replace this with a map/sum once range is a real type. + // Use Range.applyRows to determine expectedColumnCount let expectedColumnCount = 0; - for (let rowIdx = range.start.row; rowIdx <= range.end.row; rowIdx++) { + range.applyRows((rowIdx) => { let row = getMergedRow(data, merges, rowIdx); if (row && row.length > expectedColumnCount) expectedColumnCount = row.length; - } + }); - for (let rowIdx = range.start.row; rowIdx <= range.end.row; rowIdx++) { + range.applyRows((rowIdx) => { let rowErrorCount = 0; let row = getMergedRow(data, merges, rowIdx); let recordCells = {}; @@ -706,7 +680,7 @@ exports.SessionPerformMappingJob = (sid, range, mapping, includeErrorRow = false // Ensure we exclude footer rows from the validation as they may not // have the same number of columns as the header row. if (rowIdx == range.end.row && footerRange) { - continue; + return; } if (row.length < expectedColumnCount) { @@ -777,7 +751,7 @@ exports.SessionPerformMappingJob = (sid, range, mapping, includeErrorRow = false } else { warnings.push({ row: rowIdx, type: errorTypes.ValidationError.EmptyRow({}) }) } - } + }); let jid = makeAccessToken("j"); let job = new JobResult(records, errors, warnings); diff --git a/lib/importer/src/dudk/backend.test.js b/lib/importer/src/dudk/backend.test.js index 6d471fc3..f3fcfff2 100644 --- a/lib/importer/src/dudk/backend.test.js +++ b/lib/importer/src/dudk/backend.test.js @@ -1,5 +1,6 @@ const backend = require('./backend'); const attributeTypes = require('./types/attribute-types'); +const Range = require('./util/range'); const testFiles = new Map([ ["test", [ @@ -53,11 +54,11 @@ describe("Backend tests", () => { ]), }); - const dataRange = { - sheet: sheetname, - start: { row: 3, column: 0 }, - end: { row: 5, column: 2 } - }; + const dataRange = new Range( + sheetname, + { row: 3, column: 0 }, + { row: 5, column: 2 } + ); const samples = backend.SessionGetInputSampleRows(sid, dataRange, 1, 1, 1); @@ -133,11 +134,11 @@ describe("Backend tests", () => { const sid = backend.CreateSession(); backend.SessionSetFile(sid, '../../fixtures/merged-cells.xlsx'); - const dataRange = { - sheet: 'Cool Data', - start: { row: 3, column: 2 }, - end: { row: 5, column: 5 } - }; + const dataRange = new Range( + 'Cool Data', + { row: 3, column: 2 }, + { row: 5, column: 5 } + ); const samples = backend.SessionGetInputSampleRows(sid, dataRange, 1, 1, 1); @@ -159,11 +160,11 @@ describe("Backend tests", () => { [{ index: 6, row: [{ value: 'Sid' }, undefined, { value: '10' }, { value: 'Medium' }] }] ]); - const allRange = { - sheet: 'Cool Data', - start: { row: 0, column: 0 }, - end: { row: 7, column: 5 } - }; + const allRange = new Range( + 'Cool Data', + { row: 0, column: 0 }, + { row: 7, column: 5 } + ); const wholeSheet = backend.SessionGetInputSampleRows(sid, allRange, 8, 0, 0); @@ -301,11 +302,11 @@ describe("Backend tests", () => { const sid = backend.CreateSession(); backend.SessionSetFile(sid, filename); - const dataRange = { - sheet: sheetname, - start: { row: 3, column: 0 }, - end: { row: 5, column: 3 } - }; + const dataRange = new Range( + sheetname, + { row: 3, column: 0 }, + { row: 5, column: 3 } + ); const samples = backend.SessionGetInputSampleRows(sid, dataRange, 1, 1, 1); @@ -329,38 +330,38 @@ describe("Backend tests", () => { const sid = backend.CreateSession(); backend.SessionSetFile(sid, filename); - const headerRange = { - sheet: sheetname, - start: { row: 2, column: 0 }, - end: { row: 2, column: 2 } - }; + const headerRange = new Range( + sheetname, + { row: 2, column: 0 }, + { row: 2, column: 2 } + ); - const footerRange = { - sheet: sheetname, - start: { row: 6, column: 0 }, - end: { row: 5, column: 1 } - }; + const footerRange = new Range( + sheetname, + { row: 5, column: 0 }, + { row: 6, column: 1 } + ); const defaultDataRange = backend.SessionSuggestDataRange(sid, null, null); - expect(defaultDataRange).toMatchObject({ - sheet: sheetname, - start: { row: 1, column: 0 }, - end: { row: 6, column: 6 } - }); + expect(defaultDataRange).toMatchObject(new Range( + sheetname, + { row: 1, column: 0 }, + { row: 6, column: 6 } + )); const headerBasedDataRange = backend.SessionSuggestDataRange(sid, headerRange, null); - expect(headerBasedDataRange).toMatchObject({ - sheet: sheetname, - start: { row: 3, column: 0 }, - end: { row: 6, column: 2 } - }); + expect(headerBasedDataRange).toMatchObject(new Range( + sheetname, + { row: 3, column: 0 }, + { row: 6, column: 2 } + )); const headerAndFooterBasedDataRange = backend.SessionSuggestDataRange(sid, headerRange, footerRange); - expect(headerAndFooterBasedDataRange).toMatchObject({ - sheet: sheetname, - start: { row: 3, column: 0 }, - end: { row: 6, column: 2 } - }); + expect(headerAndFooterBasedDataRange).toMatchObject(new Range( + sheetname, + { row: 3, column: 0 }, + { row: 5, column: 2 } + )); } }); @@ -370,11 +371,11 @@ describe("Backend tests", () => { const sid = backend.CreateSession(); backend.SessionSetFile(sid, filename); - const dataRange = { - sheet: sheetname, - start: { row: 3, column: 0 }, - end: { row: 5, column: 2 } - }; + const dataRange = new Range( + sheetname, + { row: 3, column: 0 }, + { row: 5, column: 2 } + ); // There are three rows available in the range. Let's see how the sampling // functions allocate them if we ask for too many sample rows. @@ -451,11 +452,11 @@ describe("Backend tests", () => { const sid = backend.CreateSession(); backend.SessionSetFile(sid, filename); - const dataRange = { - sheet: sheetname, - start: { row: 3, column: 0 }, - end: { row: 5, column: 2 } - }; + const dataRange = new Range( + sheetname, + { row: 3, column: 0 }, + { row: 5, column: 2 } + ); // We can be fairly confident startCount and endCount work right, but // middleCount has an *algorithm* that tries to pick a random sample of rows @@ -536,11 +537,11 @@ test('validation', () => { const sid = backend.CreateSession(); backend.SessionSetFile(sid, filename); - const dataRange = { - sheet: sheetname, - start: { row: 3, column: 0 }, - end: { row: 8, column: 3 } - }; + const dataRange = new Range( + sheetname, + { row: 3, column: 0 }, + { row: 8, column: 3 } + ); const mapping = { attributeMappings: { @@ -600,11 +601,11 @@ test('validation', () => { test('guessing types', () => { const sid = backend.CreateSession(); backend.SessionSetFile(sid, "../../fixtures/type-guessing.csv"); - const dataRange = { - sheet: 'Sheet1', - start: { row: 1, column: 0 }, - end: { row: 6, column: 9 } - }; + const dataRange = new Range( + 'Sheet1', + { row: 1, column: 0 }, + { row: 6, column: 9 } + ); const guesses = backend.SessionGuessTypes(sid, dataRange); expect(guesses).toMatchObject([ diff --git a/lib/importer/src/dudk/sheets.js b/lib/importer/src/dudk/sheets.js index c2ab0652..2acc2c76 100644 --- a/lib/importer/src/dudk/sheets.js +++ b/lib/importer/src/dudk/sheets.js @@ -1,6 +1,7 @@ const b26 = require('./util/base26') const backend = require("./backend"); const attributeTypes = require('./types/attribute-types'); +const Range = require('./util/range'); const DEFAULT_PREVIEW_LIMIT = 20 @@ -30,7 +31,7 @@ exports.GetHeader = (sid, sheet) => { } else { // We have a valid header range selected by a user, get the real // values from the sheet - const headerSample = backend.SessionGetInputSampleRows(sid, headerRange, 1, 0, 0); + const headerSample = backend.SessionGetInputSampleRows(sid, headerRange, 1, 0, 0); const samples = headerSample[0][0].row; for (let i = 0; i < samples.length; i++) { @@ -127,11 +128,12 @@ exports.GetPreview = (sid, sheet, count = 10) => { .SessionGetInputDimensions(sid) .sheetDimensions.get(sheet); - const preview = backend.SessionGetInputSampleRows(sid, { - sheet: sheet, - start: { row: 0, column: 0 }, - end: { row: dimensions.rows, column: dimensions.columns > 0 ? dimensions.columns - 1 : 0 } - }, Math.min(count, dimensions.rows), 0, 0)[0]; + const previewRange = new Range( + sheet, + { row: 0, column: 0 }, + { row: dimensions.rows, column: dimensions.columns > 0 ? dimensions.columns - 1 : 0 } + ); + const preview = backend.SessionGetInputSampleRows(sid, previewRange, Math.min(count, dimensions.rows), 0, 0)[0]; // TODO: Is there a better way to tell if the sheet is empty than iterating // all the cells? - we should have something in SessionGetInputDimensions @@ -177,11 +179,11 @@ exports.GetRows = (sid, sheet, start = 0, count = 10) => { // Try to provide rows from start to start+count-1, but limit ourselves to // the rows in the sheet. - const rowRange = { - sheet: sheet, - start: { row: start, column: 0 }, - end: { row: Math.min(start + count, sheetDimensions.rows), column: sheetDimensions.columns - 1 } - } + const rowRange = new Range( + sheet, + { row: start, column: 0 }, + { row: Math.min(start + count, sheetDimensions.rows), column: sheetDimensions.columns - 1 } + ); const wantedRows = rowRange.end.row - start; const preview = backend.SessionGetInputSampleRows(sid, rowRange, wantedRows, 0, 0)[0]; @@ -197,11 +199,11 @@ exports.GetRowRangeFromEnd = (sid, sheet, count = 10) => { const hRange = backend.SessionGetHeaderRange(sid, sheet) const end = sheetDimensions.rows - 1; - return { - sheet: sheet, - start: { row: Math.max(end - count, hRange.start.row + 1), column: hRange.start.column }, - end: { row: end, column: hRange.end.column } - } + return new Range( + sheet, + { row: Math.max(end - count, hRange.start.row + 1), column: hRange.start.column }, + { row: end, column: hRange.end.column } + ); } // Given a session id and a sheet returns up to count rows. @@ -220,22 +222,17 @@ exports.GetColumnValues = (sid, sheet, columnIndex, cellWidth = 30, count = 10) const fRange = backend.SessionGetFooterRange(sid, sheet) let dataRange = backend.SessionSuggestDataRange(sid, hRange, fRange); - // Limit to just the column we want - dataRange.start.column += columnIndex; - dataRange.end.column = dataRange.start.column; - - // The range passed to SessionGetInputValues is inclusive, and therefore - // we end up with the first row of the selected footer. To avoid this - // we will decrement the end.row to ensure we don't pull too many values. - // We also need to make sure in small tables thast this doesn't make end.row - // less than start.row - if (dataRange.end.row > dataRange.start.row) { - dataRange.end.row = dataRange.end.row - 1 - } - + // Create a new Range for just the column we want + const colStart = dataRange.start.row; + const colEnd = dataRange.end.row > dataRange.start.row ? dataRange.end.row - 1 : dataRange.end.row; + const singleColRange = new Range( + dataRange.sheet, + { row: colStart, column: dataRange.start.column + columnIndex }, + { row: colEnd, column: dataRange.start.column + columnIndex } + ); let values = backend.SessionGetInputValues( sid, - dataRange, + singleColRange, count )[0]; diff --git a/lib/importer/src/dudk/sheets.test.js b/lib/importer/src/dudk/sheets.test.js index bdbfc641..f7399dc2 100644 --- a/lib/importer/src/dudk/sheets.test.js +++ b/lib/importer/src/dudk/sheets.test.js @@ -10,11 +10,12 @@ test('trailing columns', () => { expect(sid).not.toBeNull(); backend.SessionSetFile(sid, "../../fixtures/trailing-column.csv") - backend.SessionSetHeaderRange(sid, { - sheet: "Sheet1", - start: { row: 0, column: 0 }, - end: { row: 0, column: 2 }, - }) + const Range = require('./util/range'); + backend.SessionSetHeaderRange(sid, new Range( + "Sheet1", + { row: 0, column: 0 }, + { row: 0, column: 2 } + )) let headers = sheets_lib.GetHeader(sid, "Sheet1") expect(headers.length).toBe(3); diff --git a/lib/importer/src/dudk/types/date.test.js b/lib/importer/src/dudk/types/date.test.js index e0745ec2..5802f8d1 100644 --- a/lib/importer/src/dudk/types/date.test.js +++ b/lib/importer/src/dudk/types/date.test.js @@ -1,5 +1,5 @@ const date = require('./date'); // For unit tests -const errors = require('../util/errors'); +const Range = require('../util/range'); test('string date parsing', () => { const ymd = date.makeCombinedDateType(['year', 'month', 'day']); @@ -100,11 +100,11 @@ const at = require('./attribute-types'); test('dates in ods', () => { const sid = backend.CreateSession(); backend.SessionSetFile(sid, "../../fixtures/dates.ods"); - const dataRange = { - sheet: 'Sheet1', - start: { row: 0, column: 0 }, - end: { row: 3, column: 0 } - }; + const dataRange = new Range( + 'Sheet1', + { row: 0, column: 0 }, + { row: 3, column: 0 } + ); const mapping = { attributeMappings: { Date: 0 @@ -147,11 +147,11 @@ test('dates in ods', () => { test('dates in csv', () => { const sid = backend.CreateSession(); backend.SessionSetFile(sid, "../../fixtures/dates.csv"); - const dataRange = { - sheet: 'Sheet1', - start: { row: 2, column: 1 }, - end: { row: 8, column: 1 } - }; + const dataRange = new Range( + 'Sheet1', + { row: 2, column: 1 }, + { row: 8, column: 1 } + ); const mapping = { attributeMappings: { Date: 0 @@ -200,11 +200,11 @@ test('dates in csv', () => { test('dates in xlsx from numbers', () => { const sid = backend.CreateSession(); backend.SessionSetFile(sid, "../../fixtures/dates.numbers.xlsx"); - const dataRange = { - sheet: 'Sheet 1', - start: { row: 2, column: 1 }, - end: { row: 8, column: 1 } - }; + const dataRange = new Range( + 'Sheet 1', + { row: 2, column: 1 }, + { row: 8, column: 1 } + ); const mapping = { attributeMappings: { Date: 0 @@ -247,11 +247,11 @@ test('dates in xlsx from numbers', () => { test('dates in xlsx from office 2016', () => { const sid = backend.CreateSession(); backend.SessionSetFile(sid, "../../fixtures/dates.office2016.xlsx"); - const dataRange = { - sheet: 'Sheet1', - start: { row: 2, column: 1 }, - end: { row: 3, column: 1 } - }; + const dataRange = new Range( + 'Sheet1', + { row: 2, column: 1 }, + { row: 3, column: 1 } + ); const mapping = { attributeMappings: { Date: 0 @@ -289,11 +289,11 @@ test('dates in xlsx from office 2016', () => { test('dates in xls from office 2016', () => { const sid = backend.CreateSession(); backend.SessionSetFile(sid, "../../fixtures/dates.office2016.xls"); - const dataRange = { - sheet: 'Sheet1', - start: { row: 2, column: 1 }, - end: { row: 3, column: 1 } - }; + const dataRange = new Range( + 'Sheet1', + { row: 2, column: 1 }, + { row: 3, column: 1 } + ); const mapping = { attributeMappings: { Date: 0 diff --git a/lib/importer/src/dudk/util/range.js b/lib/importer/src/dudk/util/range.js new file mode 100644 index 00000000..e7a3a3d1 --- /dev/null +++ b/lib/importer/src/dudk/util/range.js @@ -0,0 +1,104 @@ +class Range { + constructor(sheet, start, end) { + // start and end: { row: Number, column: Number } + if (!sheet || !start || !end) { + throw new Error('Range requires sheet, start, and end'); + } + this.sheet = sheet; + this.start = { + row: Math.min(start.row, end.row), + column: Math.min(start.column, end.column) + }; + this.end = { + row: Math.max(start.row, end.row), + column: Math.max(start.column, end.column) + }; + } + + // Returns true if the range is valid (end >= start for both row and column) + isValid() { + return this.end.row >= this.start.row && this.end.column >= this.start.column; + } + + // Apply a function to each row index in the range + // inclusive=true: includes end row (default, spreadsheet style) + // inclusive=false: excludes end row (half-open interval) + applyRows(fn, inclusive = true) { + const end = inclusive ? this.end.row : this.end.row - 1; + for (let r = this.start.row; r <= end; r++) { + fn(r); + } + } + + + // Inclusive row indices + *rows(inclusive = true) { + const end = inclusive ? this.end.row : this.end.row - 1; + for (let r = this.start.row; r <= end; r++) { + yield r; + } + } + + // Inclusive column indices + *columns(inclusive = true) { + const end = inclusive ? this.end.column : this.end.column - 1; + for (let c = this.start.column; c <= end; c++) { + yield c; + } + } + + // Iterate over all cells (row, col) in the range + *cells(inclusive = true) { + const endRow = inclusive ? this.end.row : this.end.row - 1; + const endCol = inclusive ? this.end.column : this.end.column - 1; + for (let r = this.start.row; r <= endRow; r++) { + for (let c = this.start.column; c <= endCol; c++) { + yield { row: r, column: c }; + } + } + } + + // Number of rows in the range + numRows(inclusive = true) { + return (this.end.row - this.start.row) + (inclusive ? 1 : 0); + } + + // Number of columns in the range + numCols(inclusive = true) { + return (this.end.column - this.start.column) + (inclusive ? 1 : 0); + } + + // Area (number of cells) in the range + area(inclusive = true) { + return this.numRows(inclusive) * this.numCols(inclusive); + } + + // Convert column index to Base26 (A, B, ..., Z, AA, AB, ...) + static colToBase26(column) { + // Use the base26 utility, which expects 1-based input + const base26 = require('./base26'); + return base26.toBase26(column + 1); + } + + // Convert row index to 1-based (spreadsheet style) + static rowTo1Based(row) { + return (row + 1).toString(); + } + + // Get Base26 notation for start cell (e.g., "A1") + get startBase26() { + return `${Range.colToBase26(this.start.column)}${Range.rowTo1Based(this.start.row)}`; + } + + // Get Base26 notation for end cell (e.g., "C10") + get endBase26() { + return `${Range.colToBase26(this.end.column)}${Range.rowTo1Based(this.end.row)}`; + } + + // Get A1-style range string (e.g., "Sheet1!A1:C10") + toA1String() { + return `${this.sheet}!${this.startBase26}:${this.endBase26}`; + } +} + +module.exports = Range; diff --git a/lib/importer/src/dudk/util/range.test.js b/lib/importer/src/dudk/util/range.test.js new file mode 100644 index 00000000..bda8bcd1 --- /dev/null +++ b/lib/importer/src/dudk/util/range.test.js @@ -0,0 +1,93 @@ +const Range = require('./range'); + +describe('Range', () => { + test('applyRows applies function to each row (inclusive)', () => { + const r = new Range('Sheet1', { row: 2, column: 0 }, { row: 4, column: 1 }); + const rows = []; + r.applyRows((rowIdx) => rows.push(rowIdx)); + expect(rows).toEqual([2, 3, 4]); + }); + + test('applyRows applies function to each row (exclusive)', () => { + const r = new Range('Sheet1', { row: 2, column: 0 }, { row: 4, column: 1 }); + const rows = []; + r.applyRows((rowIdx) => rows.push(rowIdx), false); + expect(rows).toEqual([2, 3]); + }); + + const sheet = 'Sheet1'; + const start = { row: 1, column: 2 }; // C2 + const end = { row: 3, column: 4 }; // E4 + + test('constructs and normalizes start/end', () => { + const r = new Range(sheet, end, start); // reversed + expect(r.sheet).toBe(sheet); + expect(r.start).toEqual({ row: 1, column: 2 }); + expect(r.end).toEqual({ row: 3, column: 4 }); + }); + + test('rows() yields correct indices (inclusive/exclusive)', () => { + const r = new Range(sheet, start, end); + expect([...r.rows()]).toEqual([1, 2, 3]); + expect([...r.rows(false)]).toEqual([1, 2]); + }); + + test('columns() yields correct indices (inclusive/exclusive)', () => { + const r = new Range(sheet, start, end); + expect([...r.columns()]).toEqual([2, 3, 4]); + expect([...r.columns(false)]).toEqual([2, 3]); + }); + + test('cells() yields all cell coordinates (inclusive/exclusive)', () => { + const r = new Range(sheet, start, end); + const allCells = [...r.cells()]; + expect(allCells.length).toBe(9); // 3 rows x 3 cols + expect(allCells[0]).toEqual({ row: 1, column: 2 }); + expect(allCells[8]).toEqual({ row: 3, column: 4 }); + const exclCells = [...r.cells(false)]; + expect(exclCells.length).toBe(4); // 2 rows x 2 cols + expect(exclCells[0]).toEqual({ row: 1, column: 2 }); + expect(exclCells[3]).toEqual({ row: 2, column: 3 }); + }); + + test('numRows, numCols, area (inclusive/exclusive)', () => { + const r = new Range(sheet, start, end); + expect(r.numRows()).toBe(3); + expect(r.numRows(false)).toBe(2); + expect(r.numCols()).toBe(3); + expect(r.numCols(false)).toBe(2); + expect(r.area()).toBe(9); + expect(r.area(false)).toBe(4); + }); + + test('colToBase26 converts indices to spreadsheet letters', () => { + expect(Range.colToBase26(0)).toBe('A'); + expect(Range.colToBase26(25)).toBe('Z'); + expect(Range.colToBase26(26)).toBe('AA'); + expect(Range.colToBase26(27)).toBe('AB'); + expect(Range.colToBase26(51)).toBe('AZ'); + expect(Range.colToBase26(52)).toBe('BA'); + }); + + test('rowTo1Based converts indices to 1-based strings', () => { + expect(Range.rowTo1Based(0)).toBe('1'); + expect(Range.rowTo1Based(9)).toBe('10'); + }); + + test('startBase26 and endBase26 getters', () => { + const r = new Range(sheet, start, end); + expect(r.startBase26).toBe('C2'); + expect(r.endBase26).toBe('E4'); + }); + + test('toA1String returns correct A1 notation', () => { + const r = new Range(sheet, start, end); + expect(r.toA1String()).toBe('Sheet1!C2:E4'); + }); + + test('throws if missing arguments', () => { + expect(() => new Range()).toThrow(); + expect(() => new Range(sheet)).toThrow(); + expect(() => new Range(sheet, start)).toThrow(); + }); +}); diff --git a/lib/importer/src/dudk/validation.test.js b/lib/importer/src/dudk/validation.test.js index e4c4118f..3fa3e24e 100644 --- a/lib/importer/src/dudk/validation.test.js +++ b/lib/importer/src/dudk/validation.test.js @@ -6,6 +6,7 @@ const session = require('../session'); const backend = require('./backend'); const attributeTypes = require('./types/attribute-types'); const mock_files = require('mock-fs'); +const Range = require('./util/range'); function localFileMock(file, data) { const obj = {} @@ -47,11 +48,11 @@ describe("Validation tests", () => { localFileMock('./empty/app/config.json', JSON.stringify(config)) process.env.KIT_PROJECT_DIR = './empty/app/config.json' - const dataRange = { - sheet: "Sheet1", - start: { row: 0, column: 0 }, - end: { row: 2, column: 10 } - }; + const dataRange = new Range( + "Sheet1", + { row: 0, column: 0 }, + { row: 2, column: 10 } + ); const mapping = createTestMapping( { @@ -110,11 +111,11 @@ describe("Validation tests", () => { localFileMock('./empty/app/config.json', JSON.stringify(config)) process.env.KIT_PROJECT_DIR = './empty/app/config.json' - const dataRange = { - sheet: "Sheet1", - start: { row: 0, column: 0 }, - end: { row: 2, column: 10 } - }; + const dataRange = new Range( + "Sheet1", + { row: 0, column: 0 }, + { row: 2, column: 10 } + ); const mapping = createTestMapping( { @@ -174,11 +175,11 @@ describe("Validation tests", () => { localFileMock('./empty/app/config.json', JSON.stringify(config)) process.env.KIT_PROJECT_DIR = './empty/app/config.json' - const dataRange = { - sheet: "Sheet1", - start: { row: 0, column: 0 }, - end: { row: 2, column: 10 } - }; + const dataRange = new Range( + "Sheet1", + { row: 0, column: 0 }, + { row: 2, column: 10 } + ); const mapping = createTestMapping( { @@ -236,11 +237,11 @@ describe("Validation tests", () => { localFileMock('./empty/app/config.json', JSON.stringify(config)) process.env.KIT_PROJECT_DIR = './empty/app/config.json' - const dataRange = { - sheet: "Sheet1", - start: { row: 0, column: 0 }, - end: { row: 2, column: 10 } - }; + const dataRange = new Range( + "Sheet1", + { row: 0, column: 0 }, + { row: 2, column: 10 } + ); const mapping = createTestMapping( { diff --git a/lib/importer/src/index.js b/lib/importer/src/index.js index 26ee7c2d..839d975d 100644 --- a/lib/importer/src/index.js +++ b/lib/importer/src/index.js @@ -7,6 +7,7 @@ const sheets_lib = require("./dudk/sheets.js"); const tpl_functions = require("./functions.js") const tpl_filters = require("./filters.js") const usage = require("./usage.js") +const Range = require("./dudk/util/range.js"); const IMPORTER_SESSION_KEY = "importer.session"; const IMPORTER_ERROR_KEY = "importer.error"; @@ -216,11 +217,11 @@ exports.Initialise = (config, router, prototypeKit) => { // should default to the first row in the sheet. let maxCol = sheets_lib.GetTotalColumns(session.backendSid, session.sheet); - session.headerRange = { - sheet: session.sheet, - start: { row: -1, column: 0 }, - end: { row: -1, column: maxCol - 1 }, - }; + session.headerRange = new Range( + session.sheet, + { row: -1, column: 0 }, + { row: -1, column: maxCol - 1 }, + ); // Ensure the session is persisted. Currently in session, eventually another way request.session.data[IMPORTER_SESSION_KEY] = session; @@ -250,11 +251,11 @@ exports.Initialise = (config, router, prototypeKit) => { // If the user did not select a header range, then we should create one if (session.headerRange == null) { let [minCol, maxCol] = sheets_lib.GuessHeaderRange(session.backendSid, session.sheet); - session.headerRange = { - sheet: session.sheet, - start: { row: -1, column: minCol }, - end: { row: -1, column: maxCol }, - }; + session.headerRange = new Range( + session.sheet, + { row: -1, column: minCol }, + { row: -1, column: maxCol }, + ); } // Ensure the session is persisted. Currently in session, eventually another way @@ -606,9 +607,9 @@ const getSelectionFromRequest = (request, session, optional = false) => { [tlCol, brCol] = [brCol, tlCol]; } - return { - sheet: session.sheet, - start: { row: tlRow, column: tlCol }, - end: { row: brRow, column: brCol }, - }; + return new Range( + session.sheet, + { row: tlRow, column: tlCol }, + { row: brRow, column: brCol }, + ); }; diff --git a/lib/importer/src/session.js b/lib/importer/src/session.js index 1f2a8254..d2c8bc5d 100644 --- a/lib/importer/src/session.js +++ b/lib/importer/src/session.js @@ -29,11 +29,10 @@ class Session { set headerRange(range) { if (!range) return; - const r = range; - if (!r.sheet) { - r.sheet = this.sheet + if (!range.sheet) { + range.sheet = this.sheet } - backend.SessionSetHeaderRange(this.backendSid, r) + backend.SessionSetHeaderRange(this.backendSid, range) } get footerRange() { @@ -41,12 +40,11 @@ class Session { } set footerRange(range) { - const r = range; - if (!r.sheet) { - r.sheet = this.sheet + if (!range.sheet) { + range.sheet = this.sheet } - backend.SessionSetFooterRange(this.backendSid, r) + backend.SessionSetFooterRange(this.backendSid, range) } get mapping() { diff --git a/lib/importer/src/session.test.js b/lib/importer/src/session.test.js index 25ac4ba2..657e219b 100644 --- a/lib/importer/src/session.test.js +++ b/lib/importer/src/session.test.js @@ -1,4 +1,5 @@ const session_lib = require('./session'); +const Range = require('./dudk/util/range'); const basic_config = { fields: [{ name: 'field', type: 'text', required: false }], @@ -42,7 +43,7 @@ describe("Header Row Display tests", () => { s.sheet = "Sheet1" - s.headerRange = { sheet: "Sheet1", start: { row: 0, column: 0 }, end: { row: 0, column: 2 } } + s.headerRange = new Range("Sheet1", { row: 0, column: 0 }, { row: 0, column: 2 } ); const headers = session_lib.HeaderRowDisplay(s, "None") expect(headers).toBeNull(); }); @@ -81,7 +82,7 @@ describe("Header Row Display tests", () => { ).session; s.sheet = "Sheet1" - s.headerRange = { sheet: "Sheet1", start: { row: 0, column: 0 }, end: { row: 0, column: 2 } } + s.headerRange = new Range("Sheet1", { row: 0, column: 0 }, { row: 0, column: 2 }); const headers = session_lib.HeaderRowDisplay(s, "index") expect(headers).toStrictEqual(["A", "B", "C"]) @@ -140,9 +141,11 @@ describe("Header Row Display tests", () => { ).session; s.sheet = "Sheet1" - s.headerRange = { - sheet: "Sheet1", start: { row: 0, column: 1 }, end: { row: 1, column: 3 } - } + s.headerRange = new Range( + 'Sheet1', + { row: 0, column: 1 }, + { row: 1, column: 3 } + ); const headers = session_lib.HeaderRowDisplay(s, "source") expect(headers).toStrictEqual(["Title", "First name", "Surname"]);