Skip to content

Commit e4e4cc8

Browse files
committed
fix: remove double dto mapping from save calls
1 parent 5af21e7 commit e4e4cc8

4 files changed

Lines changed: 75 additions & 16 deletions

File tree

src/coalesce-vue/src/api-client.ts

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import type {
1919
CollectionValue,
2020
ItemMethod,
2121
ListMethod,
22+
MethodParameter,
2223
TypeDiscriminatorToType,
2324
PropNames,
2425
} from "./metadata.js";
@@ -1326,20 +1327,28 @@ export class ModelApiClient<TModel extends Model<ModelType>> extends ApiClient<
13261327
): ItemResultPromise<TModel> {
13271328
const { fields, ...params } = parameters ?? new SaveParameters<TModel>();
13281329

1329-
const paramsMeta = Object.fromEntries([
1330-
[
1331-
"$type",
1332-
{
1333-
type: "string",
1334-
name: "$type",
1335-
displayName: "$type",
1330+
// The item is fully serialized to a DTO here (respecting `fields` for
1331+
// surgical saves). Declare the resulting fields as passthrough `unknown`
1332+
// params so that `getRequestBody` does not serialize them a *second* time.
1333+
// A double serialization is not idempotent for polymorphic child objects
1334+
// (their type discriminator is a `$type` string once serialized, not a
1335+
// `$metadata` reference), which would downgrade them to their base type.
1336+
const dto = mapToDtoFiltered(item, fields)!;
1337+
1338+
// Order the params by `$type` (if present) followed by metadata declaration
1339+
// order so that the serialized request body's field order is stable and
1340+
// matches what it was when each field was individually re-serialized.
1341+
const paramsMeta: { [name: string]: MethodParameter } = {};
1342+
for (const name of ["$type", ...Object.keys(this.$metadata.props)]) {
1343+
if (name in dto) {
1344+
paramsMeta[name] = {
1345+
type: "unknown",
1346+
name,
1347+
displayName: name,
13361348
role: "value",
1337-
},
1338-
],
1339-
...Object.entries(this.$metadata.props).filter(
1340-
(p) => !p[1].dontSerialize,
1341-
),
1342-
]);
1349+
};
1350+
}
1351+
}
13431352

13441353
return this.$invoke(
13451354
{
@@ -1350,7 +1359,7 @@ export class ModelApiClient<TModel extends Model<ModelType>> extends ApiClient<
13501359
params: paramsMeta,
13511360
return: this.$itemValueMeta,
13521361
},
1353-
mapToDtoFiltered(item, fields)!,
1362+
dto,
13541363
config,
13551364
params,
13561365
);

src/coalesce-vue/src/model.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -595,10 +595,10 @@ class MapToDtoVisitor extends Visitor<
595595

596596
if ("derivedTypes" in meta || "baseTypes" in meta) {
597597
meta =
598-
value.$metadata ??
598+
value.$metadata ||
599599
// Fall back to checking the object's $type, in case its getting double-mapped.
600600
("derivedTypes" in meta &&
601-
meta.derivedTypes?.find((t) => t.name == value.$type)) ??
601+
meta.derivedTypes?.find((t) => t.name == value.$type)) ||
602602
meta;
603603
output.$type = meta.name;
604604
}

src/coalesce-vue/test/model.toDto.spec.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,4 +246,32 @@ describe("mapToDto", () => {
246246
impl1OnlyField: "hello",
247247
});
248248
});
249+
250+
test("is idempotent for a leaf type mapped against its own (concrete) metadata", () => {
251+
// The concrete leaf metadata has `baseTypes` but no `derivedTypes`. The type
252+
// resolution fallback must handle this without corrupting `meta` when the
253+
// value has no `$metadata` reference (i.e. it's already a DTO).
254+
const implValue = {
255+
name: "model",
256+
displayName: "Model",
257+
type: "model",
258+
role: "value",
259+
typeDef: $metadata.AbstractImpl1,
260+
} as any;
261+
262+
const once = model.mapToDto(
263+
new AbstractImpl1({ id: 1, impl1OnlyField: "hello" }),
264+
implValue,
265+
) as any;
266+
expect(once).toMatchObject({
267+
$type: "AbstractImpl1",
268+
impl1OnlyField: "hello",
269+
});
270+
271+
const twice = model.mapToDto(once, implValue) as any;
272+
expect(twice).toMatchObject({
273+
$type: "AbstractImpl1",
274+
impl1OnlyField: "hello",
275+
});
276+
});
249277
});

src/coalesce-vue/test/viewmodel.inheritance.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,28 @@ describe("ViewModel", () => {
114114

115115
echoEndpoint.destroy();
116116
});
117+
118+
test("$save of a derived type sends its discriminator and derived props", async () => {
119+
// The save pipeline serializes the model to a DTO once (mapToDtoFiltered) and
120+
// must not serialize it a second time in getRequestBody, which would downgrade
121+
// a polymorphic model to its base type and drop derived-only props.
122+
const saveEndpoint = mockEndpoint(
123+
"/AbstractImpl1/save",
124+
vitest.fn((req) => ({ wasSuccessful: true, object: null })),
125+
);
126+
127+
const vm = new AbstractImpl1ViewModel({ id: 1, impl1OnlyField: "hello" });
128+
vm.$setPropDirty("impl1OnlyField");
129+
await vm.$save();
130+
131+
expect(JSON.parse(saveEndpoint.mock.calls[0][0].data)).toMatchObject({
132+
$type: "AbstractImpl1",
133+
id: 1,
134+
impl1OnlyField: "hello",
135+
});
136+
137+
saveEndpoint.destroy();
138+
});
117139
});
118140

119141
describe("abstract loader", () => {

0 commit comments

Comments
 (0)