Skip to content

Commit

Permalink
Sanketika-Obsrv/issue-tracker#14: feat: validate sql query (#100)
Browse files Browse the repository at this point in the history
  • Loading branch information
@shreyasb22
shreyasb22 authored Mar 4, 2024
1 parent 9b14ecd commit 5493b12
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 11 deletions.
4 changes: 3 additions & 1 deletion api-service/src/test/Fixtures.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class TestDruidQuery {
'{"context":{"dataSource":"telemetry-events"},"querySql":{"query":"SELECT __time FROM \\"invalid-datasource\\" LIMIT 10"}}';
public static SKIP_VALIDATION_NATIVE = '{"context":{"dataSource":"system-stats"},"query":{"queryType":"timeBoundary","dataSource":"system-stats","granularity":"all","intervals":["2022-10-17/2022-10-19"],"resultFormat":"compactedList","columns":["__time","scans"],"metrics":{"type":"numeric","metric":"count"},"aggregations":[{"type":"count","name":"count"}]}}';
public static SKIP_VALIDATION_SQL = '{"context":{"dataSource":"failed-events-summary"},"querySql":{"query":"SELECT * FROM \\"failed-events-summary\\" WHERE __time >= TIMESTAMP \'2020-12-31\' AND __time < TIMESTAMP \'2021-01-21\' LIMIT 10"}}';
public static INVALID_SQL_QUERY = '{\"context\":{\"dataSource\":\"system-events\",\"granularity\":\"day\"},\"querySql\":{\"query\":\"SELECT * \"}}';
public static MISSING_TABLE_NAME = '{\"context\":{\"dataSource\":\"system-events\",\"granularity\":\"day\"},\"querySql\":{\"query\":\"SELECT * FROM \"}}';
}

class TestDataIngestion {
Expand Down Expand Up @@ -142,4 +144,4 @@ class TestExhaust {
}
}

export { TestDruidQuery, TestDataIngestion, TestDataset, TestDataSource, TestDatasetSourceConfig, TestExhaust, TestSubmitIngestion};
export { TestDruidQuery, TestDataIngestion, TestDataset, TestDataSource, TestDatasetSourceConfig, TestExhaust, TestSubmitIngestion};
50 changes: 50 additions & 0 deletions api-service/src/test/QueryTestService.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,56 @@ describe("QUERY API", () => {
done();
})
})
it("should throw error for invalid sql query", (done) => {
chai.spy.on(dbConnector, "readRecords", () => {
return [{ "datasource_ref": "sample_ref" }]
})
nock(config.druidHost + ":" + config.druidPort)
.get(config.druidDatasourcesEndPoint)
.reply(200, ["sample_ref"])
nock(config.druidHost + ":" + config.druidPort)
.post(config.druidSqlEndPoint)
.reply(200, [{ events: [] }]);
chai.
request(app)
.post(config.apiDruidSqlEndPoint)
.send(JSON.parse(TestDruidQuery.INVALID_SQL_QUERY))
.end((err, res) => {
res.should.have.status(httpStatus.BAD_REQUEST);
res.body.should.be.a("object");
res.body.responseCode.should.be.eq(httpStatus["400_NAME"]);
res.body.params.status.should.be.eq(constants.STATUS.FAILURE);
res.body.id.should.be.eq(routesConfig.query.sql_query.api_id);
chai.spy.restore(dbConnector, "readRecords");
nock.cleanAll();
done();
})
})
it("should throw error is table name is missing from the SQL Query", (done) => {
chai.spy.on(dbConnector, "readRecords", () => {
return [{ "datasource_ref": "sample_ref" }]
})
nock(config.druidHost + ":" + config.druidPort)
.get(config.druidDatasourcesEndPoint)
.reply(200, ["sample_ref"])
nock(config.druidHost + ":" + config.druidPort)
.post(config.druidSqlEndPoint)
.reply(200, [{ events: [] }]);
chai.
request(app)
.post(config.apiDruidSqlEndPoint)
.send(JSON.parse(TestDruidQuery.MISSING_TABLE_NAME))
.end((err, res) => {
res.should.have.status(httpStatus.BAD_REQUEST);
res.body.should.be.a("object");
res.body.responseCode.should.be.eq(httpStatus["400_NAME"]);
res.body.params.status.should.be.eq(constants.STATUS.FAILURE);
res.body.id.should.be.eq(routesConfig.query.sql_query.api_id);
chai.spy.restore(dbConnector, "readRecords");
nock.cleanAll();
done();
})
})
})
describe("error scenarios", () => {
it("should handle the error", (done) => {
Expand Down
37 changes: 27 additions & 10 deletions api-service/src/validators/QueryValidator.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import httpStatus from "http-status";
import _ from "lodash";
import moment, { Moment} from "moment";
import _ from "lodash";
import moment, { Moment } from "moment";
import { queryRules } from "../configs/QueryRules";
import { IConnector, IValidator } from "../models/DatasetModels";
import { ICommonRules, ILimits, IQuery, IQueryTypeRules, IRules } from "../models/QueryModels";
Expand Down Expand Up @@ -30,9 +30,12 @@ export class QueryValidator implements IValidator {
return validationStatus.isValid ? (shouldSkip ? validationStatus : this.setDatasourceRef(dataSource, data)) : validationStatus
case routesConfig.query.sql_query.api_id:
validationStatus = await this.validateSqlQuery(data)
dataSource = this.getDataSource(data)
shouldSkip = _.includes(config.exclude_datasource_validation, dataSource);
return validationStatus.isValid ? (shouldSkip ? validationStatus : this.setDatasourceRef(dataSource, data)) : validationStatus
if (validationStatus.isValid) {
dataSource = this.getDataSource(data)
shouldSkip = _.includes(config.exclude_datasource_validation, dataSource);
return validationStatus.isValid ? (shouldSkip ? validationStatus : this.setDatasourceRef(dataSource, data)) : validationStatus
}
return validationStatus
default:
return <ValidationStatus>{ isValid: false }
}
Expand All @@ -45,17 +48,31 @@ export class QueryValidator implements IValidator {
try {
return (!_.isEmpty(dataSourceLimits)) ? this.validateQueryRules(queryObj, dataSourceLimits.queryRules[queryObj.query.queryType as keyof IQueryTypeRules]) : { isValid: true }
} catch (error: any) {
return { isValid: false, message: error.message || "error ocuured while validating native query", code: error.code || httpStatus[ "400_NAME" ] };
return { isValid: false, message: error.message || "error ocuured while validating native query", code: error.code || httpStatus["400_NAME"] };
}
}

private validateSqlQuery(data: any): ValidationStatus {
this.setQueryLimits(data, this.limits.common);
let dataSourceLimits = this.getDataSourceLimits(this.getDataSource(data));
private validateSqlQuery(data: IQuery): ValidationStatus {
try {
let query = data.querySql.query;
if (_.isEmpty(query)) {
return { isValid: false, message: "Query must not be empty", code: httpStatus["400_NAME"] };
}
const fromClause = /\bFROM\b/i;
const isFromClausePresent = fromClause.test(query)
if (!isFromClausePresent) {
return { isValid: false, message: "Invalid SQL Query", code: httpStatus["400_NAME"] };
}
const dataset = query.substring(query.indexOf("FROM")).split(" ")[1].replace(/\\/g, "");
if (_.isEmpty(dataset)) {
return { isValid: false, message: "Dataset name must be present in the SQL Query", code: httpStatus["400_NAME"] };
}
this.setQueryLimits(data, this.limits.common);
let datasource = this.getDataSource(data);
let dataSourceLimits = this.getDataSourceLimits(datasource);
return (!_.isEmpty(dataSourceLimits)) ? this.validateQueryRules(data, dataSourceLimits.queryRules.scan) : { isValid: true };
} catch (error: any) {
return { isValid: false, message: error.message || "error ocuured while validating native query", code: error.code || httpStatus[ "400_NAME" ] };
return { isValid: false, message: error.message || "error ocuured while validating SQL query", code: error.code || httpStatus[ "500_NAME" ] };
}
}

Expand Down

0 comments on commit 5493b12

Please sign in to comment.