-
Notifications
You must be signed in to change notification settings - Fork 341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue 1217 - csv upload validation #1403
base: development
Are you sure you want to change the base?
Changes from 13 commits
187dde5
02712f0
55db711
9722c00
67b8078
62288b6
034e7b4
4f12d03
f4b60f3
5f30318
143a96d
6c612b8
30d213e
f6b4c1c
06751f8
45ae817
7c317f5
4d85514
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,14 @@ async function uploadMeters(req, res, filepath, conn) { | |
try { | ||
for (let i = 0; i < meters.length; i++) { | ||
let meter = meters[i]; | ||
//validation for boolean values | ||
validateBooleanFields(meter, i); | ||
|
||
// Validate min and max values | ||
validateMinMaxValues(meter, i); | ||
|
||
|
||
|
||
// First verify GPS is okay | ||
// This assumes that the sixth column is the GPS as order is assumed for now in a GPS file. | ||
const gpsInput = meter[6]; | ||
|
@@ -56,6 +64,49 @@ async function uploadMeters(req, res, filepath, conn) { | |
meter[6] = switchGPS(gpsInput); | ||
} | ||
|
||
// Verify area unit | ||
const areaInput = meter[9]; | ||
if (areaInput) { | ||
if (!isValidArea(areaInput)){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a small point but here and a few other lines don't have spaces: |
||
let msg = `For meter ${meter[0]} the area entry of ${areaInput} is invalid.`; | ||
throw new CSVPipelineError(msg, undefined, 500); | ||
} | ||
} | ||
|
||
const timeSortValue = meter[17]; | ||
if (timeSortValue) { | ||
if (!isValidTimeSort(timeSortValue)){ | ||
let msg = `For meter ${meter[0]} the time sort ${timeSortValue} is invalid.`; | ||
throw new CSVPipelineError(msg, undefined, 500); | ||
} | ||
} | ||
|
||
const timezone = meter[5]; | ||
if (timezone){ | ||
if (!isValidTimeZone(timezone)){ | ||
let msg = `For meter ${meter[0]}, ${timeSortValue} is not a valid time zone.`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think |
||
throw new CSVPipelineError(msg, undefined, 500); | ||
} | ||
} | ||
|
||
// Verify area unit provided | ||
const areaUnitString = meter[25]; | ||
if (areaUnitString) { | ||
if (!isValidAreaUnit(areaUnitString)){ | ||
let msg = `For meter ${meter[0]} the area unit of ${areaUnitString} is invalid.`; | ||
throw new CSVPipelineError(msg, undefined, 500); | ||
} | ||
} | ||
|
||
// Verify meter type | ||
const meterTypeString = meter[4]; | ||
if (meterTypeString) { | ||
if (!isValidMeterType(meterTypeString)){ | ||
let msg = `For meter ${meter[0]} the meter type of ${meterTypeString} is invalid.`; | ||
throw new CSVPipelineError(msg, undefined, 500); | ||
} | ||
} | ||
|
||
// Process unit. | ||
const unitName = meter[23]; | ||
const unitId = await getUnitId(unitName, Unit.unitType.METER, conn); | ||
|
@@ -109,7 +160,7 @@ async function uploadMeters(req, res, filepath, conn) { | |
throw new CSVPipelineError( | ||
`Meter name of \"${meter[0]}\" got database error of: ${error.message}`, undefined, 500); | ||
} | ||
); | ||
); | ||
} | ||
} | ||
} catch (error) { | ||
|
@@ -152,6 +203,78 @@ function switchGPS(gpsString) { | |
return (array[1] + ',' + array[0]); | ||
} | ||
|
||
/** | ||
* Checks if the area provided is a number and if it is larger than zero. | ||
* @param areaInput the provided area for the meter | ||
* @returns true or false | ||
*/ | ||
function isValidArea(areaInput) { | ||
// must be a number and must be non-negative | ||
if (Number.isInteger(areaInput) && areaInput > 0){ | ||
return true; | ||
} else { | ||
return false; | ||
} | ||
} | ||
|
||
/** | ||
* Checks if the area unit provided is an option | ||
* @param areaUnit the provided area for the meter | ||
* @returns true or false | ||
*/ | ||
function isValidAreaUnit(areaUnit) { | ||
const validTypes = ['feet', 'meters', 'none']; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think using the standard enum would be better. See src/server/models/Unit.js for |
||
// must be one of the three values | ||
if (validTypes.includes(areaUnit)){ | ||
return true; | ||
} else { | ||
return false; | ||
} | ||
} | ||
|
||
/** | ||
* Checks if the time sort value provided is accurate (should be increasing or decreasing) | ||
* @param timeSortValue the provided time sort | ||
* @returns true or false | ||
*/ | ||
function isValidTimeSort(timeSortValue) { | ||
// must be one of the three values | ||
if (timeSortValue == 'increasing' || timeSortValue == 'decreasing'){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to above about enum, see src/server/services/csvPipeline/validateCsvUploadParams.js for |
||
return true; | ||
} else { | ||
return false; | ||
} | ||
} | ||
|
||
/** | ||
* Checks if the meter type provided is one of the 5 options allowed when creating a meter. | ||
* @param meterTypeString the string for the meter type | ||
* @returns true or false | ||
*/ | ||
function isValidMeterType(meterTypeString) { | ||
const validTypes = ['egauge', 'mamac', 'metasys', 'obvius', 'other']; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same enum and see src/server/models/Meter.js for `Meter.type``. |
||
if (validTypes.includes(meterTypeString)){ | ||
return true; | ||
} else { | ||
return false; | ||
} | ||
} | ||
|
||
/** | ||
* Checks the provided time zone and if it is a real time zone. | ||
* @param zone the provided time zone from the csv | ||
* @returns true or false | ||
*/ | ||
function isValidTimeZone(zone) { | ||
// check against the built in timezones, must use a try catch since it does not return a boolean | ||
try { | ||
new Intl.DateTimeFormat(undefined, {timeZone : zone}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The src/client/app/components/TimeZoneSelect.tsx front-end component uses |
||
return true; | ||
} catch (e) { | ||
return false; | ||
} | ||
} | ||
|
||
/** | ||
* Return the id associated with the given unit's name. | ||
* If the unit's name is invalid or its type is different from expected type, return null. | ||
|
@@ -170,4 +293,64 @@ async function getUnitId(unitName, expectedUnitType, conn) { | |
return unit.id; | ||
} | ||
|
||
module.exports = uploadMeters; | ||
|
||
Comment on lines
295
to
+296
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe remove second blank line. |
||
/** | ||
* Validates all boolean-like fields for a given meter row. | ||
* @param {Array} meter - A single row from the CSV file. | ||
* @param {number} rowIndex - The current row index for error reporting. | ||
*/ | ||
function validateBooleanFields(meter, rowIndex) { | ||
// all inputs that involve a true or false all bieng validated together. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bieng ->> being |
||
const booleanFields = { | ||
2: 'enabled', | ||
3: 'displayable', | ||
10: 'cumulative', | ||
11: 'reset', | ||
18: 'end only', | ||
32: 'disableChecks' | ||
}; | ||
|
||
for (const [index, name] of Object.entries(booleanFields)) { | ||
let value = meter[index]; | ||
|
||
// allows upper/lower case. | ||
if (typeof value === 'string') { | ||
value = value.toLowerCase(); | ||
} | ||
|
||
// Validates read values to either false or true | ||
if (value !== 'true' && value !== 'false' && value !== true && value !== false) { | ||
huss marked this conversation as resolved.
Show resolved
Hide resolved
|
||
throw new CSVPipelineError( | ||
`Invalid input for '${name}' in row ${rowIndex + 1}: "${meter[index]}". Expected 'true' or 'false'.`, | ||
undefined, | ||
500 | ||
); | ||
} | ||
} | ||
} | ||
|
||
|
||
|
||
|
||
function validateMinMaxValues(meter, rowIndex) { | ||
const minValue = Number(meter[27]); | ||
const maxValue = Number(meter[28]); | ||
|
||
if ( | ||
isNaN(minValue) || | ||
isNaN(maxValue) || | ||
minValue < -9007199254740991 || | ||
maxValue > 9007199254740991 || | ||
minValue >= maxValue | ||
) { | ||
throw new CSVPipelineError( | ||
`Invalid min/max values in row ${rowIndex + 1}: min="${meter[27]}", max="${meter[28]}". ` + | ||
`Min or/and max must be a number larger than -9007199254740991, and less then 9007199254740991, and min must be less than max.`, | ||
undefined, | ||
500 | ||
); | ||
} | ||
} | ||
|
||
|
||
module.exports = uploadMeters; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check can fail in certain circumstances. I tested and 0 (zero) and -1 returns false so the area check does not happen. I also notice that 0.1 or any value with a decimal part does not work but it should. Finally, this allowed letters for numbers to pass the test. This may impact other tests you added. I did not try a fix so let me know if you have issues in resolving.