Skip to content

Fix fmodata mutation Prefer header merging for special columns #212

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.

Sign up for GitHub

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

Merged
eluce2 merged 3 commits into from
Apr 15, 2026
Merged

Fix fmodata mutation Prefer header merging for special columns #212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3d8cd82
Fix fmodata mutation Prefer header merging
eluce2 Apr 15, 2026
bd0dccf
Preserve and merge Prefer headers in OData writes
eluce2 Apr 15, 2026
7c971ff
Merge Prefer headers in fmodata requests
eluce2 Apr 15, 2026
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
5 changes: 5 additions & 0 deletions .changeset/fix-fmodata-mutation-special-columns.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@proofkit/fmodata": patch
---

Fix `insert()` and `update(..., { returnFullRecord: true })` to preserve merged `Prefer` headers for `fmodata.include-specialcolumns` and `fmodata.entity-ids`, and return special columns in typed full-record mutation responses.
26 changes: 25 additions & 1 deletion packages/fmodata/src/client/builders/mutation-helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,37 @@ export interface FilterQueryBuilder {
export function mergeMutationExecuteOptions(
options: (RequestInit & FFetchOptions & ExecuteOptions) | undefined,
databaseUseEntityIds: boolean,
): RequestInit & FFetchOptions & { useEntityIds?: boolean } {
databaseIncludeSpecialColumns: boolean,
): RequestInit & FFetchOptions & { useEntityIds?: boolean; includeSpecialColumns?: boolean } {
return {
...options,
useEntityIds: options?.useEntityIds ?? databaseUseEntityIds,
includeSpecialColumns: options?.includeSpecialColumns ?? databaseIncludeSpecialColumns,
};
}

export function mergePreferHeaderValues(...values: Array<string | undefined>): string | undefined {
const merged: string[] = [];
const seen = new Set<string>();

for (const value of values) {
if (!value) {
continue;
}

for (const part of value.split(",")) {
const normalized = part.trim();
if (!normalized || seen.has(normalized)) {
continue;
}
seen.add(normalized);
merged.push(normalized);
}
}

return merged.length > 0 ? merged.join(", ") : undefined;
}

export function resolveMutationTableId(
// biome-ignore lint/suspicious/noExplicitAny: Accepts any FMTable configuration
table: FMTable<any, any> | undefined,
Expand Down
6 changes: 5 additions & 1 deletion packages/fmodata/src/client/delete-builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,11 @@ export class ExecutableDeleteBuilder<Occ extends FMTable<any, any>>
}

execute(options?: ExecuteMethodOptions<ExecuteOptions>): Promise<Result<{ deletedCount: number }>> {
const mergedOptions = mergeMutationExecuteOptions(options, this.config.useEntityIds);
const mergedOptions = mergeMutationExecuteOptions(
options,
this.config.useEntityIds,
this.config.includeSpecialColumns,
);
// biome-ignore lint/suspicious/noExplicitAny: Execute options include dynamic fetch fields
const { method: _method, body: _body, ...requestOptions } = mergedOptions as any;
const useEntityIds = mergedOptions.useEntityIds ?? this.config.useEntityIds;
Expand Down
28 changes: 20 additions & 8 deletions packages/fmodata/src/client/entity-set.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,19 +230,25 @@ export class EntitySet<Occ extends FMTable<any, any>, DatabaseIncludeSpecialColu
}

// Overload: when returnFullRecord is false
insert(data: InsertDataFromFMTable<Occ>, options: { returnFullRecord: false }): InsertBuilder<Occ, "minimal">;
insert(
data: InsertDataFromFMTable<Occ>,
options: { returnFullRecord: false },
): InsertBuilder<Occ, "minimal", DatabaseIncludeSpecialColumns>;

// Overload: when returnFullRecord is true or omitted (default)
insert(data: InsertDataFromFMTable<Occ>, options?: { returnFullRecord?: true }): InsertBuilder<Occ, "representation">;
insert(
data: InsertDataFromFMTable<Occ>,
options?: { returnFullRecord?: true },
): InsertBuilder<Occ, "representation", DatabaseIncludeSpecialColumns>;

// Implementation
insert(
data: InsertDataFromFMTable<Occ>,
options?: { returnFullRecord?: boolean },
): InsertBuilder<Occ, "minimal" | "representation"> {
): InsertBuilder<Occ, "minimal" | "representation", DatabaseIncludeSpecialColumns> {
const returnPreference = options?.returnFullRecord === false ? "minimal" : "representation";

return new InsertBuilder<Occ, typeof returnPreference>({
return new InsertBuilder<Occ, typeof returnPreference, DatabaseIncludeSpecialColumns>({
occurrence: this.occurrence,
layer: this.layer,
// biome-ignore lint/suspicious/noExplicitAny: Input type is validated/transformed at runtime
Expand All @@ -253,19 +259,25 @@ export class EntitySet<Occ extends FMTable<any, any>, DatabaseIncludeSpecialColu
}

// Overload: when returnFullRecord is explicitly true
update(data: UpdateDataFromFMTable<Occ>, options: { returnFullRecord: true }): UpdateBuilder<Occ, "representation">;
update(
data: UpdateDataFromFMTable<Occ>,
options: { returnFullRecord: true },
): UpdateBuilder<Occ, "representation", DatabaseIncludeSpecialColumns>;

// Overload: when returnFullRecord is false or omitted (default)
update(data: UpdateDataFromFMTable<Occ>, options?: { returnFullRecord?: false }): UpdateBuilder<Occ, "minimal">;
update(
data: UpdateDataFromFMTable<Occ>,
options?: { returnFullRecord?: false },
): UpdateBuilder<Occ, "minimal", DatabaseIncludeSpecialColumns>;

// Implementation
update(
data: UpdateDataFromFMTable<Occ>,
options?: { returnFullRecord?: boolean },
): UpdateBuilder<Occ, "minimal" | "representation"> {
): UpdateBuilder<Occ, "minimal" | "representation", DatabaseIncludeSpecialColumns> {
const returnPreference = options?.returnFullRecord === true ? "representation" : "minimal";

return new UpdateBuilder<Occ, typeof returnPreference>({
return new UpdateBuilder<Occ, typeof returnPreference, DatabaseIncludeSpecialColumns>({
occurrence: this.occurrence,
layer: this.layer,
// biome-ignore lint/suspicious/noExplicitAny: Input type is validated/transformed at runtime
Expand Down
31 changes: 20 additions & 11 deletions packages/fmodata/src/client/filemaker-odata.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { createLogger, type InternalLogger, type Logger } from "../logger";
import { type FMODataLayer, HttpClient, ODataConfig, ODataLogger } from "../services";
import type { Auth, ExecutionContext, Result } from "../types";
import { getAcceptHeader } from "../types";
import { mergePreferHeaderValues } from "./builders/mutation-helpers";
import { Database } from "./database";
import { safeJsonParse } from "./sanitize-json";

Expand Down Expand Up @@ -219,19 +220,27 @@ export class FMServerConnection implements ExecutionContext {
preferValues.push("fmodata.include-specialcolumns");
}

const headers = {
Authorization:
"apiKey" in this.auth
? `Bearer ${this.auth.apiKey}`
: `Basic ${btoa(`${this.auth.username}:${this.auth.password}`)}`,
"Content-Type": "application/json",
Accept: getAcceptHeader(includeODataAnnotations),
...(preferValues.length > 0 ? { Prefer: preferValues.join(", ") } : {}),
...(options?.headers || {}),
};
const headers = new Headers(options?.headers);
headers.set(
"Authorization",
"apiKey" in this.auth
? `Bearer ${this.auth.apiKey}`
: `Basic ${btoa(`${this.auth.username}:${this.auth.password}`)}`,
);
headers.set("Content-Type", "application/json");
headers.set("Accept", getAcceptHeader(includeODataAnnotations));
const mergedPrefer = mergePreferHeaderValues(
preferValues.length > 0 ? preferValues.join(", ") : undefined,
headers.get("Prefer") ?? undefined,
);
if (mergedPrefer) {
headers.set("Prefer", mergedPrefer);
} else {
headers.delete("Prefer");
}

// Prepare loggableHeaders by omitting the Authorization key
const { Authorization, ...loggableHeaders } = headers;
const { authorization: _authorization, ...loggableHeaders } = Object.fromEntries(headers.entries());
logger.debug("Request headers:", loggableHeaders);

// TEMPORARY WORKAROUND: Hopefully this feature will be fixed in the ffetch library
Expand Down
83 changes: 64 additions & 19 deletions packages/fmodata/src/client/insert-builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,19 @@ import type { FMODataLayer, ODataConfig } from "../services";
import { transformFieldNamesToIds, transformResponseFields } from "../transform";
import type {
ConditionallyWithODataAnnotations,
ConditionallyWithSpecialColumns,
ExecutableBuilder,
ExecuteMethodOptions,
ExecuteOptions,
NormalizeIncludeSpecialColumns,
Result,
} from "../types";
import { getAcceptHeader } from "../types";
import { validateAndTransformInput, validateSingleResponse } from "../validation";
import {
getLocationHeader,
mergeMutationExecuteOptions,
mergePreferHeaderValues,
parseRowIdFromLocationHeader,
resolveMutationTableId,
} from "./builders/mutation-helpers";
Expand All @@ -36,9 +39,16 @@ export class InsertBuilder<
// biome-ignore lint/suspicious/noExplicitAny: Accepts any FMTable configuration
Occ extends FMTable<any, any> | undefined = undefined,
ReturnPreference extends "minimal" | "representation" = "representation",
DatabaseIncludeSpecialColumns extends boolean = false,
> implements
ExecutableBuilder<
ReturnPreference extends "minimal" ? { ROWID: number } : InferSchemaOutputFromFMTable<NonNullable<Occ>>
ReturnPreference extends "minimal"
? { ROWID: number }
: ConditionallyWithSpecialColumns<
InferSchemaOutputFromFMTable<NonNullable<Occ>>,
DatabaseIncludeSpecialColumns,
false
>
>
{
private readonly table?: Occ;
Expand Down Expand Up @@ -67,8 +77,8 @@ export class InsertBuilder<
*/
private mergeExecuteOptions(
options?: RequestInit & FFetchOptions & ExecuteOptions,
): RequestInit & FFetchOptions & { useEntityIds?: boolean } {
return mergeMutationExecuteOptions(options, this.config.useEntityIds);
): RequestInit & FFetchOptions & { useEntityIds?: boolean; includeSpecialColumns?: boolean } {
return mergeMutationExecuteOptions(options, this.config.useEntityIds, this.config.includeSpecialColumns);
}

/**
Expand Down Expand Up @@ -117,7 +127,11 @@ export class InsertBuilder<
ReturnPreference extends "minimal"
? { ROWID: number }
: ConditionallyWithODataAnnotations<
InferSchemaOutputFromFMTable<NonNullable<Occ>>,
ConditionallyWithSpecialColumns<
InferSchemaOutputFromFMTable<NonNullable<Occ>>,
NormalizeIncludeSpecialColumns<EO["includeSpecialColumns"], DatabaseIncludeSpecialColumns>,
false
>,
EO["includeODataAnnotations"] extends true ? true : false
>
>
Expand All @@ -128,8 +142,21 @@ export class InsertBuilder<
const { method: _method, headers: callerHeaders, body: _body, ...requestOptions } = mergedOptions as any;
const tableId = this.getTableId(mergedOptions.useEntityIds);
const url = `/${this.config.databaseName}/${tableId}`;
const shouldUseIds = mergedOptions.useEntityIds ?? false;
const preferHeader = this.returnPreference === "minimal" ? "return=minimal" : "return=representation";
const shouldUseIds = mergedOptions.useEntityIds ?? this.config.useEntityIds;
const includeSpecialColumns = mergedOptions.includeSpecialColumns ?? this.config.includeSpecialColumns;
const canonicalHeaders = new Headers(callerHeaders || {});
const preferHeader = mergePreferHeaderValues(
this.returnPreference === "minimal" ? "return=minimal" : "return=representation",
shouldUseIds ? "fmodata.entity-ids" : undefined,
includeSpecialColumns ? "fmodata.include-specialcolumns" : undefined,
canonicalHeaders.get("Prefer") ?? undefined,
);
canonicalHeaders.set("Content-Type", "application/json");
if (preferHeader) {
canonicalHeaders.set("Prefer", preferHeader);
} else {
canonicalHeaders.delete("Prefer");
}
Comment on lines +148 to +159
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not merge caller return=* preferences into the builder-controlled response shape.

Line 140 says caller options should not override the required request shape, but canonicalHeaders.get("Prefer") is merged verbatim here. A caller can send Prefer: return=minimal on a representation insert (or the inverse), and execute() will then parse the response using the wrong shape.

Apply the merge only to non-return= directives, then append the builder-selected return= value. The same sanitization is needed in ExecutableUpdateBuilder.execute().

🔧 Proposed fix 
     const shouldUseIds = mergedOptions.useEntityIds ?? this.config.useEntityIds;
     const includeSpecialColumns = mergedOptions.includeSpecialColumns ?? this.config.includeSpecialColumns;
     const canonicalHeaders = new Headers(callerHeaders || {});
+    const callerPrefer = canonicalHeaders
+      .get("Prefer")
+      ?.split(",")
+      .map((value) => value.trim())
+      .filter((value) => value && !value.startsWith("return="))
+      .join(", ");
     const preferHeader = mergePreferHeaderValues(
       this.returnPreference === "minimal" ? "return=minimal" : "return=representation",
       shouldUseIds ? "fmodata.entity-ids" : undefined,
       includeSpecialColumns ? "fmodata.include-specialcolumns" : undefined,
-      canonicalHeaders.get("Prefer") ?? undefined,
+      callerPrefer || undefined,
     );
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/fmodata/src/client/insert-builder.ts` around lines 148 - 159, The
current Prefer header construction in insert-builder.ts merges the
caller-provided "Prefer" including any return=* directive, which can override
the builder-controlled response shape; change the logic around
mergePreferHeaderValues so you first parse/sanitize the caller prefer directives
from canonicalHeaders.get("Prefer") to remove any return=* entry, merge only the
non-return directives with fmodata.entity-ids / fmodata.include-specialcolumns,
and then append the builder-controlled return value determined by
this.returnPreference (e.g., "return=minimal" or "return=representation") before
setting Prefer; apply the same sanitization/append pattern in
ExecutableUpdateBuilder.execute() to prevent caller return=* from overriding the
builder.


const pipeline = Effect.gen(this, function* () {
// Step 1: Validate input
Expand All @@ -154,11 +181,7 @@ export class InsertBuilder<
const responseData = yield* requestFromService<any>(url, {
...requestOptions,
method: "POST",
headers: {
...(callerHeaders || {}),
"Content-Type": "application/json",
Prefer: preferHeader,
},
headers: canonicalHeaders,
body: JSON.stringify(transformedData),
});

Expand Down Expand Up @@ -191,6 +214,7 @@ export class InsertBuilder<
undefined,
undefined,
"exact",
includeSpecialColumns,
),
);

Expand All @@ -216,7 +240,11 @@ export class InsertBuilder<
ReturnPreference extends "minimal"
? { ROWID: number }
: ConditionallyWithODataAnnotations<
InferSchemaOutputFromFMTable<NonNullable<Occ>>,
ConditionallyWithSpecialColumns<
InferSchemaOutputFromFMTable<NonNullable<Occ>>,
NormalizeIncludeSpecialColumns<EO["includeSpecialColumns"], DatabaseIncludeSpecialColumns>,
false
>,
EO["includeODataAnnotations"] extends true ? true : false
>
>
Expand All @@ -241,26 +269,41 @@ export class InsertBuilder<
toRequest(baseUrl: string, options?: ExecuteOptions): Request {
const config = this.getRequestConfig();
const fullUrl = `${baseUrl}${config.url}`;

// Set Prefer header based on return preference
const preferHeader = this.returnPreference === "minimal" ? "return=minimal" : "return=representation";
const preferHeader = mergePreferHeaderValues(
this.returnPreference === "minimal" ? "return=minimal" : "return=representation",
(options?.useEntityIds ?? this.config.useEntityIds) ? "fmodata.entity-ids" : undefined,
(options?.includeSpecialColumns ?? this.config.includeSpecialColumns)
? "fmodata.include-specialcolumns"
: undefined,
);

return new Request(fullUrl, {
method: config.method,
headers: {
"Content-Type": "application/json",
Accept: getAcceptHeader(options?.includeODataAnnotations),
Prefer: preferHeader,
...(preferHeader ? { Prefer: preferHeader } : {}),
},
body: config.body,
});
}

async processResponse(
async processResponse<EO extends ExecuteOptions | undefined = undefined>(
response: Response,
options?: ExecuteOptions,
options?: EO,
): Promise<
Result<ReturnPreference extends "minimal" ? { ROWID: number } : InferSchemaOutputFromFMTable<NonNullable<Occ>>>
Result<
ReturnPreference extends "minimal"
? { ROWID: number }
: ConditionallyWithSpecialColumns<
InferSchemaOutputFromFMTable<NonNullable<Occ>>,
NormalizeIncludeSpecialColumns<
EO extends ExecuteOptions ? EO["includeSpecialColumns"] : undefined,
DatabaseIncludeSpecialColumns
>,
false
>
>
> {
// Check for error responses (important for batch operations)
if (!response.ok) {
Expand Down Expand Up @@ -345,6 +388,7 @@ export class InsertBuilder<

// Transform response field IDs back to names if using entity IDs
const shouldUseIds = options?.useEntityIds ?? this.config.useEntityIds;
const includeSpecialColumns = options?.includeSpecialColumns ?? this.config.includeSpecialColumns;

let transformedResponse = rawResponse;
if (this.table && shouldUseIds) {
Expand Down Expand Up @@ -376,6 +420,7 @@ export class InsertBuilder<
undefined, // No selected fields for insert
undefined, // No expand configs
"exact", // Expect exactly one record
includeSpecialColumns,
);

if (!validation.valid) {
Expand Down
Loading
Loading