feat(operations): replace full_record and __unset__ with a put operation

Scope reduction, per kriszyp's review on #2347: one operation instead of a flag
plus a per-record directive.

    { "operation": "put", "database": "dev", "table": "dog",
      "records": [{ "id": 1, "dog_name": "Penny" }] }

The stored record becomes exactly that, so an attribute the request omits is
removed. Creates a missing record and replaces an existing one — the same
`Table.put` REST's `PUT /Table/id` performs, so the same audit type, replication
shape, and retained `__createdtime__`. `update`/`upsert` keep merging, which is the
v4-compatible behaviour existing clients depend on and the reason this is a
separate operation rather than semantics changed in place.

This covers what studio#1643 actually needs: its row editor holds the whole record
already. Removed with the flag and the directive:

- `full_record` as a client-facing field on update/upsert
- `__unset__`, its reserved key, its validation, the attribute-collector
  divergence in both `insertUpdateValidate.js` and `insert.ts`, and the
  `getRecordAttributes` name expansion
- `takeUnsetAttributes` and its latch

Three problems retire with them, all raised in review and none now reachable:
`__unset__` degrading a merge to a whole-record put under contention (kept as a
feature request, see below), the reserved key reinterpreting a pre-existing
`__unset__` attribute with no migration, and the directive's interaction with
dynamic attribute expansion.

The internal marker is an ARGUMENT, not a field: `putRecords` calls
`upsertRecords(obj, true)`. Reading it off the request — which the first draft of
this commit did — let a client send `full_record: true` with an `update` and get a
replace, which makes the operation name stop describing the write and bypasses the
attribute-scoped denial, since that is keyed on the operation. A test pins it;
restoring the request read fails that test and nothing else.

Kept from the previous rounds, unchanged in substance:

- The attribute-scoped denial, now keyed on `op === write.put.name`. A replace
  removes what the request omits, and `checkAttributePerms` only sees what it
  supplies.
- The target-database resolution fix and its fail-closed backstop, including the
  numeric `table: 0` guard.
- The bulk-load opt-out keyed on the operation rather than a caller-supplied
  `action`. The `__unset__` half of that fix goes away with the directive; the
  pre-existing half — a direct request skipping every attribute check by attaching
  `action` — stays fixed and stays tested.

Wired as a first-class operation: `OPERATIONS_ENUM.PUT`, the dispatch map, the
replication catchup switch, `requiredPermissions` with `[insert, update]` (what
`upsert` and REST `PUT` both require), the grantable `data` operation group, and
the `new_attributes`/`txn_time` result cleanup. Returns `put_hashes`; a put never
skips.

Tests: `integrationTests/database/put-operation.test.ts`, renamed from
full-record-write — 18 probes, all passing. Every guard mutation-checked: making
put ignore the marker fails 5, reading the marker off the request fails 1, dropping
the attribute-scoped denial fails 1. 143 tests across the security, terminology,
auto-fields and patch-merge suites pass, plus 575 in northwind, which exercises the
CSV and bulk-load paths.

`__unset__` is not abandoned — #2350 is being reworked from "this
is broken" into "add it properly", with concurrency-safe removal at the patch
layer rather than a read-then-replace in the bridge.

Refs: HarperFast/studio#1643

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: dawsontoth

dataLayer/harperBridge/ResourceBridge.ts

@@ -8,7 +8,6 @@ import {
 	VALUE_SEARCH_COMPARATORS,
 	VALUE_SEARCH_COMPARATORS_REVERSE_LOOKUP,
 	READ_AUDIT_LOG_SEARCH_TYPES_ENUM,
-	UNSET_ATTRIBUTES as OPERATIONS_UNSET_KEY,
 } from '../../utility/hdbTerms.ts';
 import * as signalling from '../../utility/signalling.ts';
 import { SchemaEventMsg } from '../../server/threads/itc.js';
@@ -24,7 +23,7 @@ import type {
 	Operator,
 } from '../../resources/ResourceInterface.ts';
 import { collapseData } from '../../resources/tracked.ts';
-import logger, { errorToString } from '../../utility/logging/harper_logger.ts';
+import { errorToString } from '../../utility/logging/harper_logger.ts';
 import { RocksDatabase } from '@harperfast/rocksdb-js';
 import { BridgeMethods } from './BridgeMethods.ts';
 import lmdbGetBackup from './lmdbBridge/lmdbMethods/lmdbGetBackup.js';
@@ -36,14 +35,6 @@ const { HDB_ERROR_MSGS } = hdbErrors;
 const DEFAULT_DATABASE = 'data';
 const DELETE_CHUNK = 10000;
 const DELETE_PAUSE_MS = 10;
-/** Shared result for the overwhelmingly common case of a record carrying no `__unset__`, so the
- * default write path allocates nothing per record. Frozen because it is handed out repeatedly. */
-const NO_UNSET_ATTRIBUTES = Object.freeze([]) as unknown as string[];
-/** Tables that have already logged a malformed-directive warning. Latched per table, not per
- * process: `warnedNullSourcePut` (resources/Table.ts, declared inside `makeTable`) is one warn per
- * table per worker, and a single module-level flag would let a bad batch on one table silence the
- * warning for every other table until restart. Weak so it never keeps a dropped table alive. */
-const warnedMalformedUnset = new WeakSet<object>();
 
 export type SearchByConditionsRequest = Query &
 	Context & {
@@ -208,27 +199,35 @@ export class ResourceBridge extends BridgeMethods {
 		return this.upsertRecords(updateObj);
 	}
 
-	async upsertRecords(upsertObj) {
+	/**
+	 * Create-or-replace: the stored record becomes exactly the submitted one, so an attribute the
+	 * request omits is REMOVED rather than kept. `update`/`upsert` merge (`Table.patch`), which is the
+	 * v4-compatible behaviour every existing client depends on and is why this is a separate operation
+	 * rather than a flag on those. Equivalent to REST `PUT /Table/id` — same `Table.put`, so the same
+	 * audit type, replication shape, and retained `__createdtime__`.
+	 */
+	async putRecords(putObj) {
+		return this.upsertRecords(putObj, true);
+	}
+
+	async upsertRecords(upsertObj, fullRecord = false) {
 		const { attributes } = insertUpdateValidate(upsertObj);
 
 		let new_attributes;
-		// `full_record: true` makes this a full replace instead of a merge: an attribute absent from
-		// the submitted record is REMOVED rather than left at its stored value. Without it, a write
-		// over an existing record lands on `Table.patch`, so there is no way to drop an attribute
-		// through the operations API — omitting it keeps the stored value and `null` stores a null
-		// (HarperFast/studio#1643).
+		// `fullRecord` arrives as an ARGUMENT, never as a field on the request. Reading it off
+		// `upsertObj` would let a client send `full_record: true` with an `update` and get a replace —
+		// which makes the operation name stop describing the write, and bypasses the attribute-scoped
+		// `put` denial in `verifyPerms`, since that is keyed on the operation.
 		//
-		// The flag is orthogonal to each operation's create rule, which is what makes one flag cover
-		// both useful shapes: `update` still requires an existing record (a missing one is skipped),
-		// while `upsert` still creates one — so `upsert` + `full_record` is exactly REST's
-		// `PUT /Table/id`, and `update` + `full_record` is that same replace, refused if the record
-		// isn't there. It has no effect on `insert`, which never writes over an existing record.
+		// Without it a write over an existing record merges (`Table.patch`), so an attribute the caller
+		// omitted keeps its stored value and `null` stores a null. That merge is the v4-compatible
+		// behaviour `update`/`upsert` must keep; HarperFast/studio#1643 is the removal case it cannot
+		// serve, and `put` is the operation that can.
 		//
-		// Compared with the delete-then-insert this replaces, a full replace is one operation rather
-		// than two: the record is never absent between them, subscribers see a single write instead of
-		// a delete followed by an insert, and `__createdtime__` survives (`Table._writeUpdate` retains
-		// the stored created time on a full update and only stamps a new one for a new entry).
-		const fullRecord = upsertObj.full_record === true;
+		// A put is also one operation where a client emulating removal needs two: the record is never
+		// absent between them, subscribers see a single write rather than a delete followed by an
+		// insert, and `__createdtime__` survives (`Table._writeUpdate` retains the stored created time
+		// on a full update and only stamps a new one for a new entry).
 		const Table = getDatabases()[upsertObj.schema][upsertObj.table];
 		const context: Context = {
 			user: upsertObj.hdb_user,
@@ -290,33 +289,12 @@ export class ResourceBridge extends BridgeMethods {
 				// write one branch over: `insertUpdateValidate` requires a hash attribute only for
 				// `update`, so such a record takes the `id == undefined → Table.create` path and stored an
 				// auto-keyed record with the named attributes stripped, returning 200.
-				const unset = takeUnsetAttributes(record, Table, !existingRecord);
-				// `__unset__` removes named attributes while everything else still merges, which a patch
-				// cannot express — so it resolves the merge here, against the record this transaction
-				// already loaded, and writes the result as a full replace. Doing it server-side inside the
-				// write transaction is the point: a client emulating this with read-then-replace races
-				// every other writer, and `full_record` would make the caller resend attributes it never
-				// meant to touch. Under `full_record` the submitted record is already the whole intended
-				// state, so there is nothing to merge and the names are simply dropped from it.
-				//
-				// SINGLE-WRITER ONLY, and the surrounding wording should not be read as more than that.
-				// Resolving the merge here still writes a `put`, and a full put is last-writer-wins over
-				// the whole record: out-of-order reconciliation folds field-wise only for patches
-				// (`resources/Table.ts:2902-2914`) and the commit-retry path re-reads the stored record
-				// only when `!fullUpdate` (`resources/Table.ts:2530`). So a concurrent write to an
-				// attribute this request never named can be lost — the opposite of what "everything else
-				// merges" implies. Tracked in HarperFast/harper#2350; fixing it properly means resolving
-				// removals at the patch layer instead.
-				const toWrite = unset.length && existingRecord && !fullRecord ? { ...existingRecord, ...record } : record;
-				for (const attribute of unset) {
-					delete toWrite[attribute];
-				}
 				await (id == undefined
-					? Table.create(toWrite, context)
-					: existingRecord && !fullRecord && !unset.length
-						? Table.patch(toWrite, context)
-						: Table.put(toWrite, context));
-				keys.push(toWrite[Table.primaryKey]);
+					? Table.create(record, context)
+					: existingRecord && !fullRecord
+						? Table.patch(record, context)
+						: Table.put(record, context));
+				keys.push(record[Table.primaryKey]);
 			}
 			return {
 				txn_time: (transaction as any).timestamp,
@@ -823,60 +801,3 @@ async function* groupRecordsInHistory(table, start?, end?, limit?) {
 	}
 	if (enqueued) yield enqueued;
 }
-
-/**
- * Read and remove a record's `__unset__` list, returning the attribute names to drop.
- *
- * The key is removed because it is a directive, not data — left in place it would be stored as an
- * attribute. The refusals below live here rather than in the validator because only this layer knows
- * the table: which attribute is the primary key, and what the managed timestamps are called. Each is
- * refused rather than skipped, because `_writeUpdate` re-asserts all three on a full update, so
- * accepting them would return 200 and change nothing.
- */
-function takeUnsetAttributes(record: Record<string, unknown>, Table: any, hasNoExistingRecord: boolean): string[] {
-	// Own property, not `in`: an inherited `__unset__` (a record built on a prototype carrying one, or
-	// after prototype pollution) would enter this path with no directive of its own, and `delete`
-	// cannot remove an inherited value — so the caller would go on to delete the attributes it names.
-	// Still a presence check rather than a value check, so an explicitly-undefined own key comes off.
-	if (!Object.prototype.hasOwnProperty.call(record, OPERATIONS_UNSET_KEY)) return NO_UNSET_ATTRIBUTES;
-	const unset = record[OPERATIONS_UNSET_KEY];
-	// Removed before the shape is judged, and on every exit below.
-	delete record[OPERATIONS_UNSET_KEY];
-	if (hasNoExistingRecord) {
-		throw new ClientError(
-			`'${OPERATIONS_UNSET_KEY}' needs an existing record to remove attributes from; nothing is stored under this primary key`
-		);
-	}
-	// Ignored with a warning rather than thrown: this is the write and replication apply path, where
-	// throwing aborts the commit and can wedge a subscription, and skipping the directive degrades to
-	// a plain merge, which removes nothing. `resources/tracked.ts` makes the same choice for an
-	// unrecognized CRDT operation. Checked at all because the shape is validated one layer up, in a
-	// module this one does not own: a non-iterable would throw in the caller's `for…of` and take the
-	// write down, and a bare string would iterate per character, deleting single-letter attributes.
-	if (!Array.isArray(unset) || unset.some((name) => typeof name !== 'string' || name.length === 0)) {
-		// Latched per table, following `warnedNullSourcePut` in resources/Table.ts: one line per record
-		// would bury the log under a single bad batch.
-		if (!warnedMalformedUnset.has(Table)) {
-			warnedMalformedUnset.add(Table);
-			logger.warn(
-				`Ignoring a malformed '${OPERATIONS_UNSET_KEY}' on a record in ${Table.databaseName}.${Table.tableName}; expected an array of attribute names`
-			);
-		}
-		return NO_UNSET_ATTRIBUTES;
-	}
-	const names = unset as string[];
-	const primaryKey = Table.primaryKey;
-	if (primaryKey && names.includes(primaryKey)) {
-		throw new ClientError(`'${primaryKey}' is the primary key of this table and cannot be unset`);
-	}
-	// `createdTimeProperty`/`updatedTimeProperty` resolve `assignCreatedTime` OR the legacy
-	// `__createdtime__` spelling (resources/Table.ts), so this covers a schema-declared
-	// `createdAt: Float @createdTime` as well as the legacy names — which a static list in the
-	// validator, with no schema in scope, could not.
-	for (const managed of [Table.createdTimeProperty, Table.updatedTimeProperty]) {
-		if (managed?.name && names.includes(managed.name)) {
-			throw new ClientError(`'${managed.name}' is maintained by Harper and cannot be unset`);
-		}
-	}
-	return names;
-}

dataLayer/harperBridge/bridgeUtility/insertUpdateValidate.js

@@ -4,7 +4,6 @@ const hdbUtils = require('../../../utility/common_utils.ts');
 const log = require('../../../utility/logging/harper_logger.ts');
 const { getDatabases } = require('../../../resources/databases.ts');
 const { ClientError } = require('../../../utility/errors/hdbError.ts');
-const { UNSET_ATTRIBUTES } = require('../../../utility/hdbTerms.ts');
 
 module.exports = insertUpdateValidate;
 
@@ -74,15 +73,6 @@ function insertUpdateValidate(writeObject) {
 		dups.add(hdbUtils.autoCast(record[hash_attribute]));
 
 		for (let attr in record) {
-			// `__unset__` names attributes to remove; it is not one itself. Collecting it would make an
-			// open (non-schemaDefined) table register `__unset__` as a real attribute, since
-			// upsertRecords feeds this list straight to Table.addAttributes.
-			//
-			// Deliberately does NOT add the names it removes, unlike the async twin's list in
-			// dataLayer/insert.ts: that one is the bulk-load attribute-permission input, where a removal
-			// must be authorized, while this one creates table attributes — registering an attribute in
-			// order to delete it would be backwards. Keep both comments in step.
-			if (attr === UNSET_ATTRIBUTES) continue;
 			attributes[attr] = 1;
 		}
 	});

dataLayer/insert.ts

@@ -15,13 +15,13 @@ import * as globalSchema from '../utility/globalSchema.ts';
 import log from '../utility/logging/harper_logger.ts';
 import { handleHDBError } from '../utility/errors/hdbError.ts';
 import { HTTP_STATUS_CODES } from '../utility/errors/commonErrors.ts';
-import * as terms from '../utility/hdbTerms.ts';
 
 const pGlobalSchema = util.promisify(globalSchema.getTableSchema);
 
 const UPDATE_ACTION = 'updated';
 const INSERT_ACTION = 'inserted';
 const UPSERT_ACTION = 'upserted';
+const PUT_ACTION = 'put';
 
 //IMPORTANT - This validation function is the async version of the code in harperBridge/bridgeUtility/insertUpdateValidate.js
 // make sure any changes below are also made there. This is to resolve a circular dependency.
@@ -90,32 +90,6 @@ export async function validation(writeObject: any) {
 		dups.add(hdbUtils.autoCast(record[hash_attribute]));
 
 		for (let attr in record) {
-			// `__unset__` names attributes to remove and is not one itself, so the key never counts as
-			// an attribute — but the names it removes DO. This list is the bulk-load path's
-			// attribute-permission input: `bulkLoad.validateChunk` hands it to
-			// `verifyBulkLoadAttributePerms`, so a name missing from it is a removal nobody authorized.
-			// The direct operations path gets the same treatment from `getRecordAttributes`
-			// (utility/operation_authorization.ts); without it here, `import_from_s3` with
-			// `action: "update"` could unset an attribute the role has no permission to write while the
-			// equivalent `update` returns 403.
-			//
-			// Contributed for every action, not just update/upsert: requiring the permission is the safe
-			// direction, and `__unset__` is refused outright on an insert anyway (see
-			// ResourceBridge.takeUnsetAttributes).
-			//
-			// The sync twin in harperBridge/bridgeUtility/insertUpdateValidate.js feeds
-			// `Table.addAttributes` instead, so it must skip the key WITHOUT adding the removed names —
-			// registering an attribute in order to delete it would be backwards. The two lists are
-			// deliberately not identical; keep both comments in step.
-			if (attr === terms.UNSET_ATTRIBUTES) {
-				const unset = record[attr];
-				if (Array.isArray(unset)) {
-					for (const name of unset) {
-						if (typeof name === 'string' && name.length > 0) attributes[name] = 1;
-					}
-				}
-				continue;
-			}
 			attributes[attr] = 1;
 		}
 	});
@@ -245,6 +219,42 @@ async function upsertData(upsertObject: any) {
 	);
 }
 
+/**
+ * Create-or-replace the records in the putObject parameter: the stored record becomes exactly the
+ * submitted one, so an attribute the request omits is removed. `update`/`upsert` merge instead, and
+ * must keep doing so for v4 compatibility — hence a distinct operation rather than a flag on those.
+ * Equivalent to REST `PUT /Table/id`.
+ * @param putObject - Represents the data that will be written
+ */
+async function putData(putObject: any) {
+	if (putObject.operation !== 'put') {
+		throw handleHDBError(new Error(), 'invalid operation, must be put', HTTP_STATUS_CODES.INTERNAL_SERVER_ERROR);
+	}
+
+	let validator = insertValidator(putObject);
+	if (validator) {
+		throw handleHDBError(new Error(), validator.message, HTTP_STATUS_CODES.BAD_REQUEST);
+	}
+
+	hdbUtils.transformReq(putObject);
+
+	let invalidSchemaTableMsg = hdbUtils.checkSchemaTableExist(putObject.schema, putObject.table);
+	if (invalidSchemaTableMsg) {
+		throw handleHDBError(new Error(), invalidSchemaTableMsg, HTTP_STATUS_CODES.BAD_REQUEST);
+	}
+
+	let bridgePutResult = await harperBridge.putRecords(putObject);
+
+	return returnObject(
+		PUT_ACTION,
+		bridgePutResult.written_hashes,
+		putObject,
+		[],
+		bridgePutResult.new_attributes,
+		bridgePutResult.txn_time
+	);
+}
+
 /**
  * Constructs return object for insert, update, and upsert.
  * @param action
@@ -281,6 +291,12 @@ function returnObject(
 		return return_object;
 	}
 
+	if (action === PUT_ACTION) {
+		// `put` never skips: every submitted record is written, created or replaced.
+		return_object.put_hashes = written_hashes;
+		return return_object;
+	}
+
 	return_object.update_hashes = written_hashes;
 	return_object.skipped_hashes = skipped;
 	return return_object;
@@ -290,4 +306,4 @@ export function flush(object: any) {
 	hdbUtils.transformReq(object);
 	return harperBridge.flush(object.schema, object.table);
 }
-export { insertData as insert, updateData as update, upsertData as upsert };
+export { insertData as insert, updateData as update, upsertData as upsert, putData as put };