Skip to content
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

feat(schema): send a clear message when user wants to access to an unknown field #1186

Merged
merged 9 commits into from
Oct 4, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions packages/agent/src/routes/access/chart.ts
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ import {
DateOperation,
Filter,
FilterFactory,
RelationSchema,
SchemaUtils,
ValidationError,
} from '@forestadmin/datasource-toolkit';
import {
@@ -187,8 +187,11 @@ export default class ChartRoute extends CollectionRoute {
context: Context,
): Promise<Array<{ key: string; value: number }>> {
const body = <LeaderboardChart>context.request.body;
const field = this.collection.schema.fields[body.relationshipFieldName] as RelationSchema;

const field = SchemaUtils.getRelation(
this.collection.schema,
body.relationshipFieldName,
this.collection.name,
);
let collection: string;
let filter: Filter;
let aggregation: Aggregation;
13 changes: 8 additions & 5 deletions packages/agent/src/routes/modification/create.ts
Original file line number Diff line number Diff line change
@@ -7,7 +7,6 @@ import {
ManyToOneSchema,
RecordData,
RecordValidator,
RelationSchema,
SchemaUtils,
} from '@forestadmin/datasource-toolkit';
import Router from '@koa/router';
@@ -43,7 +42,7 @@ export default class CreateRoute extends CollectionRoute {
const relations: Record<string, RecordData> = {};

const promises = Object.entries(record).map(async ([field, value]) => {
const schema = this.collection.schema.fields[field];
const schema = SchemaUtils.getField(this.collection.schema, field, this.collection.name);

if (schema?.type === 'OneToOne' || schema?.type === 'ManyToOne') {
relations[field] = this.getRelationRecord(field, value as CompositeId);
@@ -87,7 +86,7 @@ export default class CreateRoute extends CollectionRoute {
const caller = QueryStringParser.parseCaller(context);

const promises = Object.entries(relations).map(async ([field, linked]) => {
const relation = this.collection.schema.fields[field];
const relation = SchemaUtils.getRelation(this.collection.schema, field, this.collection.name);
if (linked === null || relation.type !== 'OneToOne') return;

// Permissions
@@ -121,7 +120,7 @@ export default class CreateRoute extends CollectionRoute {
private getRelationRecord(field: string, id: CompositeId): RecordData {
if (id === null) return null;

const schema = this.collection.schema.fields[field] as RelationSchema;
const schema = SchemaUtils.getRelation(this.collection.schema, field, this.collection.name);
const foreignCollection = this.dataSource.getCollection(schema.foreignCollection);
const pkName = SchemaUtils.getPrimaryKeys(foreignCollection.schema);

@@ -136,7 +135,11 @@ export default class CreateRoute extends CollectionRoute {
if (id === null) return null;

const caller = QueryStringParser.parseCaller(context);
const schema = this.collection.schema.fields[field] as ManyToOneSchema;
const schema = SchemaUtils.getRelation(
this.collection.schema,
field,
this.collection.name,
) as ManyToOneSchema;
const foreignCollection = this.dataSource.getCollection(schema.foreignCollection);

return CollectionUtils.getValue(foreignCollection, caller, id, schema.foreignKeyTarget);
7 changes: 6 additions & 1 deletion packages/agent/src/routes/modification/update-relation.ts
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ import {
Filter,
ManyToOneSchema,
OneToOneSchema,
SchemaUtils,
} from '@forestadmin/datasource-toolkit';
import Router from '@koa/router';
import { Context } from 'koa';
@@ -29,7 +30,11 @@ export default class UpdateRelation extends RelationRoute {
public async handleUpdateRelationRoute(context: Context): Promise<void> {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const body = context.request.body as any;
const relation = this.collection.schema.fields[this.relationName];
const relation = SchemaUtils.getRelation(
this.collection.schema,
this.relationName,
this.collection.name,
);
const caller = QueryStringParser.parseCaller(context);
const parentId = IdUtils.unpackId(this.collection.schema, context.params.parentId);

8 changes: 6 additions & 2 deletions packages/agent/src/routes/relation-route.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Collection, DataSource, RelationSchema } from '@forestadmin/datasource-toolkit';
import { Collection, DataSource, SchemaUtils } from '@forestadmin/datasource-toolkit';

import CollectionRoute from './collection-route';
import { ForestAdminHttpDriverServices } from '../services';
@@ -8,7 +8,11 @@ export default abstract class RelationRoute extends CollectionRoute {
protected readonly relationName: string;

protected get foreignCollection(): Collection {
const schema = this.collection.schema.fields[this.relationName] as RelationSchema;
const schema = SchemaUtils.getRelation(
this.collection.schema,
this.relationName,
this.collection.name,
);

return this.collection.dataSource.getCollection(schema.foreignCollection);
}
3 changes: 1 addition & 2 deletions packages/agent/src/utils/forest-schema/generator-actions.ts
Original file line number Diff line number Diff line change
@@ -3,7 +3,6 @@ import {
ActionFormElement,
ActionLayoutElement,
Collection,
ColumnSchema,
DataSource,
LayoutElementInput,
PrimitiveTypes,
@@ -118,7 +117,7 @@ export default class SchemaGeneratorActions {
if (ActionFields.isCollectionField(field)) {
const collection = dataSource.getCollection(field.collectionName);
const [pk] = SchemaUtils.getPrimaryKeys(collection.schema);
const pkSchema = collection.schema.fields[pk] as ColumnSchema;
const pkSchema = SchemaUtils.getColumn(collection.schema, pk, collection.name);

output.type = pkSchema.columnType;
output.reference = `${collection.name}.${pk}`;
49 changes: 36 additions & 13 deletions packages/agent/src/utils/forest-schema/generator-fields.ts
Original file line number Diff line number Diff line change
@@ -9,7 +9,6 @@ import {
OneToManySchema,
OneToOneSchema,
PrimitiveTypes,
RelationSchema,
SchemaUtils,
ValidationError,
} from '@forestadmin/datasource-toolkit';
@@ -28,7 +27,7 @@ export default class SchemaGeneratorFields {
};

static buildSchema(collection: Collection, name: string): ForestServerField {
const { type } = collection.schema.fields[name];
const { type } = SchemaUtils.getField(collection.schema, name, collection.name);

let schema: ForestServerField;

@@ -58,7 +57,7 @@ export default class SchemaGeneratorFields {
}

private static buildColumnSchema(collection: Collection, name: string): ForestServerField {
const column = collection.schema.fields[name] as ColumnSchema;
const column = SchemaUtils.getColumn(collection.schema, name, collection.name);
ColumnSchemaValidator.validate(column, name);

const isForeignKey = SchemaUtils.isForeignKey(collection.schema, name);
@@ -112,17 +111,33 @@ export default class SchemaGeneratorFields {

if (relation.type === 'OneToMany') {
targetName = relation.originKeyTarget;
targetField = collection.schema.fields[targetName] as ColumnSchema;
targetField = SchemaUtils.getColumn(collection.schema, targetName, collection.name);

const originKey = foreignCollection.schema.fields[relation.originKey] as ColumnSchema;
const originKey = SchemaUtils.getColumn(
foreignCollection.schema,
relation.originKey,
foreignCollection.name,
);
isReadOnly = originKey.isReadOnly;
} else {
targetName = relation.foreignKeyTarget;
targetField = foreignCollection.schema.fields[targetName] as ColumnSchema;
targetField = SchemaUtils.getColumn(
foreignCollection.schema,
targetName,
foreignCollection.name,
);

const throughSchema = collection.dataSource.getCollection(relation.throughCollection).schema;
const foreignKey = throughSchema.fields[relation.foreignKey] as ColumnSchema;
const originKey = throughSchema.fields[relation.originKey] as ColumnSchema;
const throughCollection = collection.dataSource.getCollection(relation.throughCollection);
const foreignKey = SchemaUtils.getColumn(
throughCollection.schema,
relation.foreignKey,
throughCollection.name,
);
const originKey = SchemaUtils.getColumn(
throughCollection.schema,
relation.originKey,
throughCollection.name,
);
isReadOnly = originKey.isReadOnly || foreignKey.isReadOnly;
}

@@ -154,8 +169,16 @@ export default class SchemaGeneratorFields {
foreignCollection: Collection,
baseSchema: ForestServerField,
): ForestServerField {
const targetField = collection.schema.fields[relation.originKeyTarget] as ColumnSchema;
const keyField = foreignCollection.schema.fields[relation.originKey] as ColumnSchema;
const targetField = SchemaUtils.getColumn(
collection.schema,
relation.originKeyTarget,
collection.name,
);
const keyField = SchemaUtils.getColumn(
foreignCollection.schema,
relation.originKey,
foreignCollection.name,
);

return {
...baseSchema,
@@ -177,7 +200,7 @@ export default class SchemaGeneratorFields {
foreignCollection: Collection,
baseSchema: ForestServerField,
): ForestServerField {
const keyField = collection.schema.fields[relation.foreignKey] as ColumnSchema;
const keyField = SchemaUtils.getColumn(collection.schema, relation.foreignKey, collection.name);

return {
...baseSchema,
@@ -197,7 +220,7 @@ export default class SchemaGeneratorFields {
}

private static buildRelationSchema(collection: Collection, name: string): ForestServerField {
const relation = collection.schema.fields[name] as RelationSchema;
const relation = SchemaUtils.getRelation(collection.schema, name, collection.name);
const foreignCollection = collection.dataSource.getCollection(relation.foreignCollection);

const relationSchema = {
3 changes: 1 addition & 2 deletions packages/agent/src/utils/id.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {
CollectionSchema,
ColumnSchema,
CompositeId,
FieldValidator,
RecordData,
@@ -49,7 +48,7 @@ export default class IdUtils {
}

return pkNames.map((pkName, index) => {
const schemaField = schema.fields[pkName] as ColumnSchema;
const schemaField = SchemaUtils.getColumn(schema, pkName);
const value = pkValues[index];

const castedValue = schemaField.columnType === 'Number' ? Number(value) : value;
9 changes: 4 additions & 5 deletions packages/agent/src/utils/query-string.ts
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ import {
Projection,
ProjectionFactory,
ProjectionValidator,
SchemaUtils,
Sort,
SortFactory,
SortValidator,
@@ -54,11 +55,9 @@ export default class QueryStringParser {
const { schema } = collection;
const rootFields = fields.toString().split(',');
const explicitRequest = rootFields.map(field => {
if (!schema.fields[field]) {
throw new ValidationError(`field not found '${collection.name}.${field}'`);
}
const columnOrRelation = SchemaUtils.getField(schema, field, collection.name);

return schema.fields[field].type === 'Column'
return columnOrRelation.type === 'Column'
? field
: `${field}:${context.request.query[`fields[${field}]`]}`;
});
@@ -67,7 +66,7 @@ export default class QueryStringParser {

return new Projection(...explicitRequest);
} catch (e) {
throw new ValidationError(`Invalid projection: ${e.message}}`);
throw new ValidationError(`Invalid projection: ${e.message}`);
}
}

43 changes: 36 additions & 7 deletions packages/agent/test/routes/access/chart.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { ConditionTreeFactory, ValidationError } from '@forestadmin/datasource-toolkit';
import {
ConditionTreeFactory,
MissingRelationError,
ValidationError,
} from '@forestadmin/datasource-toolkit';
import { createMockContext } from '@shopify/jest-koa-mocks';

import Chart from '../../../src/routes/access/chart';
@@ -71,6 +75,8 @@ describe('ChartRoute', () => {
originKey: 'authorId',
originKeyTarget: 'id',
}),
manyToOneRelation: factories.manyToOneSchema.build(),
oneToOneRelation: factories.oneToOneSchema.build(),
},
}),
}),
@@ -892,7 +898,7 @@ describe('ChartRoute', () => {
});
});

describe('when relation field is invalid', () => {
describe('when relation field is missing', () => {
test('it should throw an error', async () => {
jest.spyOn(dataSource.getCollection('persons'), 'aggregate').mockResolvedValueOnce([
{ value: 1234, group: { id: 2 } },
@@ -913,13 +919,36 @@ describe('ChartRoute', () => {
...defaultContext,
});

await expect(chart.handleChart(context)).rejects.toThrow(
new ValidationError(
'Failed to generate leaderboard chart: parameters do not match pre-requisites',
),
);
await expect(chart.handleChart(context)).rejects.toThrow(MissingRelationError);
});
});

describe('when the given relation is not allowed', () => {
test.each(['manyToOneRelation', 'oneToOneRelation'])(
'should throw an error when the relation is %s',
async relationType => {
const chart = new Chart(services, options, dataSource, 'persons');

const chartRequest = {
type: 'Leaderboard',
aggregator: 'Sum',
aggregateFieldName: 'id',
sourceCollectionName: 'persons',
labelFieldName: 'id',
relationshipFieldName: relationType,
limit: 2,
};
const context = createMockContext({
requestBody: chartRequest,
...defaultContext,
});

await expect(chart.handleChart(context)).rejects.toThrow(
'Failed to generate leaderboard chart: parameters do not match pre-requisites',
);
},
);
});
});
});
});
2 changes: 1 addition & 1 deletion packages/agent/test/utils/query-string.test.ts
Original file line number Diff line number Diff line change
@@ -157,7 +157,7 @@ describe('QueryStringParser', () => {
const fn = () => QueryStringParser.parseProjection(collectionSimple, context);

expect(fn).toThrow(
"Invalid projection: field not found 'books.field-that-do-not-exist'",
"Invalid projection: The 'books.field-that-do-not-exist' field was not found. Available fields are: [id,name]. Please check if the field name is correct.",
);
});
});
4 changes: 2 additions & 2 deletions packages/datasource-customizer/src/collection-customizer.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {
CollectionSchema,
CollectionUtils,
ColumnSchema,
Logger,
Operator,
SchemaUtils,
allowedOperatorsForColumnType,
} from '@forestadmin/datasource-toolkit';

@@ -458,7 +458,7 @@ export default class CollectionCustomizer<
emulateFieldFiltering(name: TColumnName<S, N>): this {
return this.pushCustomization(async () => {
const collection = this.stack.lateOpEmulate.getCollection(this.name);
const field = collection.schema.fields[name] as ColumnSchema;
const field = SchemaUtils.getColumn(collection.schema, name, collection.name);

if (typeof field.columnType === 'string') {
const operators = allowedOperatorsForColumnType[field.columnType];
Loading
Loading