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
Show file tree
Hide file tree
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
Expand Up @@ -8,7 +8,7 @@ import {
DateOperation,
Filter,
FilterFactory,
RelationSchema,
SchemaUtils,
ValidationError,
} from '@forestadmin/datasource-toolkit';
import {
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 8 additions & 5 deletions packages/agent/src/routes/modification/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
ManyToOneSchema,
RecordData,
RecordValidator,
RelationSchema,
SchemaUtils,
} from '@forestadmin/datasource-toolkit';
import Router from '@koa/router';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand Down
7 changes: 6 additions & 1 deletion packages/agent/src/routes/modification/update-relation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
Filter,
ManyToOneSchema,
OneToOneSchema,
SchemaUtils,
} from '@forestadmin/datasource-toolkit';
import Router from '@koa/router';
import { Context } from 'koa';
Expand All @@ -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);

Expand Down
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';
Expand All @@ -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);
}
Expand Down
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
Expand Up @@ -3,7 +3,6 @@ import {
ActionFormElement,
ActionLayoutElement,
Collection,
ColumnSchema,
DataSource,
LayoutElementInput,
PrimitiveTypes,
Expand Down Expand Up @@ -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}`;
Expand Down
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
Expand Up @@ -9,7 +9,6 @@ import {
OneToManySchema,
OneToOneSchema,
PrimitiveTypes,
RelationSchema,
SchemaUtils,
ValidationError,
} from '@forestadmin/datasource-toolkit';
Expand All @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Thenkei marked this conversation as resolved.
Show resolved Hide resolved

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;
}

Expand Down Expand Up @@ -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(
Thenkei marked this conversation as resolved.
Show resolved Hide resolved
foreignCollection.schema,
relation.originKey,
foreignCollection.name,
);

return {
...baseSchema,
Expand All @@ -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,
Expand All @@ -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 = {
Expand Down
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,
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 4 additions & 5 deletions packages/agent/src/utils/query-string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
Projection,
ProjectionFactory,
ProjectionValidator,
SchemaUtils,
Sort,
SortFactory,
SortValidator,
Expand Down Expand Up @@ -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}]`]}`;
});
Expand All @@ -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}`);
}
}

Expand Down
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';
Expand Down Expand Up @@ -71,6 +75,8 @@ describe('ChartRoute', () => {
originKey: 'authorId',
originKeyTarget: 'id',
}),
manyToOneRelation: factories.manyToOneSchema.build(),
oneToOneRelation: factories.oneToOneSchema.build(),
},
}),
}),
Expand Down Expand Up @@ -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 } },
Expand All @@ -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',
);
},
);
});
});
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/agent/test/utils/query-string.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
);
});
});
Expand Down
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';

Expand Down Expand Up @@ -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];
Expand Down
Loading
Loading