Skip to content

Commit

Permalink
feature(type): allow to control whether properties set explicitly und…
Browse files Browse the repository at this point in the history
…efined for null/undefined values.

fix(sql): `null` values in database for properties e.g. of `foo?: string` do no longer set explicitly `undefined`.

Fixes #252
  • Loading branch information
marcj committed Jun 19, 2022
1 parent 3c0f711 commit 41e5564
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 10 deletions.
8 changes: 7 additions & 1 deletion packages/sql/src/serializer/sql-serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ function deserializeSqlObjectLiteral(type: TypeClass | TypeObjectLiteral, state:
export class SqlSerializer extends Serializer {
name = 'sql';

override setExplicitUndefined(type: Type, state: TemplateState): boolean {
//make sure that `foo?: string` is not explicitly set to undefined when database returns `null`.
if (state.target === 'deserialize') return false;
return true;
}

protected registerSerializers() {
super.registerSerializers();

Expand All @@ -104,7 +110,7 @@ export class SqlSerializer extends Serializer {
const uuidType = uuidAnnotation.registerType({ kind: ReflectionKind.string }, true);

this.deserializeRegistry.register(ReflectionKind.string, (type, state) => {
//remove string enforcement, since UUID/MonogId are string but received as binary
//remove string enforcement, since UUID/MongoId are string but received as binary
state.addSetter(state.accessor);
});

Expand Down
4 changes: 4 additions & 0 deletions packages/sqlite/src/sqlite-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import { AbstractClassType, asyncOperation, ClassType, empty } from '@deepkit/core';
import {
DatabaseAdapter,
DatabaseError,
DatabaseLogger,
DatabasePersistenceChangeSet,
DatabaseSession,
Expand Down Expand Up @@ -515,6 +516,9 @@ export class SQLiteQueryResolver<T extends OrmEntity> extends SQLQueryResolver<T

const sqlBuilder = new SqlBuilder(this.platform, selectParams);
const selectSQL = sqlBuilder.select(this.classSchema, model, { select });
if (!set.length) {
throw new DatabaseError('SET is empty');
}

const sql = `
UPDATE
Expand Down
25 changes: 23 additions & 2 deletions packages/sqlite/tests/sqlite.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import { databaseFactory } from './factory';
import { User, UserCredentials } from '@deepkit/orm-integration';
import { SQLiteDatabaseAdapter, SQLiteDatabaseTransaction } from '../src/sqlite-adapter';
import { sleep } from '@deepkit/core';
import { AutoIncrement, cast, Entity, entity, PrimaryKey, Reference, ReflectionClass, typeOf } from '@deepkit/type';
import { DatabaseEntityRegistry } from '@deepkit/orm';
import { AutoIncrement, cast, Entity, entity, PrimaryKey, Reference, ReflectionClass, serialize, typeOf } from '@deepkit/type';
import { Database, DatabaseEntityRegistry } from '@deepkit/orm';

test('reflection circular reference', () => {
const user = ReflectionClass.from(User);
Expand Down Expand Up @@ -263,3 +263,24 @@ test('connection pool', async () => {
expect(c12).toBe(c2);
}
});

test('optional', async () => {
@entity.name('entity')
class MyEntity {
id: number & PrimaryKey & AutoIncrement = 0;
value?: string;
}

const database = new Database(new SQLiteDatabaseAdapter(), [MyEntity]);
await database.migrate();

const entity1 = new MyEntity();
expect('value' in entity1).toBe(false);
expect('value' in serialize<MyEntity>(entity1)).toBe(false);

await database.persist(entity1);

const entity2 = await database.query(MyEntity).findOne();
expect('value' in entity2).toBe(false);
expect('value' in serialize<MyEntity>(entity1)).toBe(false);
});
12 changes: 7 additions & 5 deletions packages/type/src/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import {
stringifyType,
Type,
TypeClass,
TypeFunction,
TypeIndexSignature,
TypeObjectLiteral,
TypeParameter,
Expand Down Expand Up @@ -725,9 +724,8 @@ export function createConverterJSForMember(
}
}

const optional = isOptional(property instanceof ReflectionProperty ? property.property : property);
const setExplicitUndefined = registry.serializer.setExplicitUndefined(type, state) && isOptional(property instanceof ReflectionProperty ? property.property : property);
const nullable = isNullable(type);
// const hasDefault = property instanceof ReflectionProperty ? property.hasDefault() : false;

// // since JSON does not support undefined, we emulate it via using null for serialization, and convert that back to undefined when deserialization happens.
// // note: When the value is not defined (property.name in object === false), then this code will never run.
Expand All @@ -746,7 +744,7 @@ export function createConverterJSForMember(
//note: this code is only reached when ${accessor} was actually defined checked by the 'in' operator.
return `
if (${state.accessor} === undefined) {
if (${optional}) {
if (${setExplicitUndefined}) {
${undefinedSetterCode}
}
} else if (${state.accessor} === null) {
Expand All @@ -756,7 +754,7 @@ export function createConverterJSForMember(
if (${nullable}) {
${nullSetterCode}
} else {
if (${optional}) {
if (${setExplicitUndefined}) {
${undefinedSetterCode}
}
}
Expand Down Expand Up @@ -1703,6 +1701,10 @@ export class Serializer {
this.registerValidators();
}

public setExplicitUndefined(type: Type, state: TemplateState): boolean {
return true;
}

protected registerValidators() {
}

Expand Down
5 changes: 3 additions & 2 deletions packages/type/tests/type-spec.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ test('partial keeps explicitely undefined fields', () => {

{
const item = serializeToJson<Partial<Model>>({ title: undefined });
expect(item).toEqual({ title: null });
}

{
Expand Down Expand Up @@ -755,9 +756,9 @@ test('dynamic properties', () => {
}
}

const back1 = deserializeFromJson<A>({'~type': 'abc'});
const back1 = deserializeFromJson<A>({ '~type': 'abc' });
expect(back1.getType()).toBe('abc');

const back2 = deserializeFromJson<A>({'type': 'abc'});
const back2 = deserializeFromJson<A>({ 'type': 'abc' });
expect(back2.getType()).toBe('abc');
});

0 comments on commit 41e5564

Please sign in to comment.