Skip to content

Commit

Permalink
fix(core): ignore SQL converter on object embeddables with custom types
Browse files Browse the repository at this point in the history
Closes #5074
  • Loading branch information
B4nan committed Jan 6, 2024
1 parent 40c5d5d commit 83b989e
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 4 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/drivers/DatabaseDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ export abstract class DatabaseDriver<C extends Connection> implements IDatabaseD
data[k] = prop.customType.convertToDatabaseValue(data[k], this.platform, { fromQuery: true, key: k, mode: 'query-data' });
}

if (prop.hasConvertToDatabaseValueSQL && !this.platform.isRaw(data[k])) {
if (prop.hasConvertToDatabaseValueSQL && !prop.object && !this.platform.isRaw(data[k])) {
const quoted = this.platform.quoteValue(data[k]);
const sql = prop.customType!.convertToDatabaseValueSQL!(quoted, this.platform);
data[k] = raw(sql.replace(/\?/g, '\\?'));
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/metadata/MetadataDiscovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,7 @@ export class MetadataDiscovery {
meta.properties[name].fieldNameRaw = fieldName.sql; // for querying in SQL drivers
meta.properties[name].persist = false; // only virtual as we store the whole object
meta.properties[name].userDefined = false; // mark this as a generated/internal property, so we can distinguish from user-defined non-persist properties
meta.properties[name].object = true;
}

this.initEmbeddables(meta, meta.properties[name], visited);
Expand Down
6 changes: 3 additions & 3 deletions packages/knex/src/AbstractSqlDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
const field = prop.fieldNames[0];

if (!duplicates.includes(field) || !usedDups.includes(field)) {
if (prop.customType && 'convertToDatabaseValueSQL' in prop.customType && !this.platform.isRaw(row[prop.name])) {
if (prop.customType && !prop.object && 'convertToDatabaseValueSQL' in prop.customType && !this.platform.isRaw(row[prop.name])) {
keys.push(prop.customType.convertToDatabaseValueSQL!('?', this.platform));
} else {
keys.push('?');
Expand Down Expand Up @@ -693,7 +693,7 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
const pks = Utils.getOrderedPrimaryKeys(cond as Dictionary, meta);
sql += ` when (${pkCond}) then `;

if (prop.customType && 'convertToDatabaseValueSQL' in prop.customType && !this.platform.isRaw(data[idx][key])) {
if (prop.customType && !prop.object && 'convertToDatabaseValueSQL' in prop.customType && !this.platform.isRaw(data[idx][key])) {
sql += prop.customType.convertToDatabaseValueSQL!('?', this.platform);
} else {
sql += '?';
Expand Down Expand Up @@ -1452,7 +1452,7 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
});

meta.props
.filter(prop => prop.hasConvertToDatabaseValueSQL || prop.hasConvertToJSValueSQL)
.filter(prop => !prop.object && (prop.hasConvertToDatabaseValueSQL || prop.hasConvertToJSValueSQL))
.forEach(prop => ret.push(prop.name));
}

Expand Down
120 changes: 120 additions & 0 deletions tests/features/embeddables/GH5074.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import { Embeddable, Embedded, Entity, MikroORM, PrimaryKey, Property, Type } from '@mikro-orm/sqlite';

class GeoPoint {

constructor(
readonly latitude: number,
readonly longitude: number,
) {}

}

class GeoPointType extends Type<GeoPoint | undefined, string | undefined> {

override convertToDatabaseValue(
value: GeoPoint | undefined,
): string | undefined {
if (!value) {
return value;
}

return `point(${value.latitude} ${value.longitude})`;
}

override convertToJSValue(value: string | undefined): GeoPoint | undefined {
const m = value?.match(/point\((-?\d+(\.\d+)?) (-?\d+(\.\d+)?)\)/i);

if (!m) {
return undefined;
}

return new GeoPoint(+m[1], +m[3]);
}

override convertToJSValueSQL(key: string) {
return `ST_AsText(${key})`;
}

override convertToDatabaseValueSQL(key: string) {
return `ST_PointFromText(${key}, 4326)`;
}

override getColumnType(): string {
return 'point SRID 4326';
}

}

@Embeddable()
class SnapshotMore {

@Property({
type: GeoPointType,
})
position!: GeoPoint;

@Property({
nullable: true,
})
description?: string;

}

@Embeddable()
class Snapshot {

@Embedded(() => SnapshotMore, {
object: true,
})
ref!: SnapshotMore;

}

@Entity()
class Outer {

@PrimaryKey()
id!: number;

@Embedded(() => Snapshot, {
object: true,
})
bigSnapshot!: Snapshot;

}

let orm: MikroORM;

beforeAll(async () => {
orm = await MikroORM.init({
dbName: ':memory:',
entities: [Outer, Snapshot],
});
await orm.schema.createSchema();
});

afterAll(async () => {
await orm.close(true);
});

test(`GH issue 5074`, async () => {
const snapshotMore = new SnapshotMore();
snapshotMore.position = new GeoPoint(21, 32);

const snapshot = new Snapshot();
snapshot.ref = snapshotMore;

const outer = new Outer();
outer.id = 123_456;
outer.bigSnapshot = snapshot;

await orm.em.persistAndFlush([outer]);

const result = await orm.em.fork().findOneOrFail(Outer, {
id: outer.id,
});
expect(result.bigSnapshot.ref.position).toEqual({
latitude: 21,
longitude: 32,
});
});

0 comments on commit 83b989e

Please sign in to comment.