feat(operations): add a put operation, and fix the target-database authorization mismatch

dataLayer/harperBridge/ResourceBridge.ts

@@ -8,6 +8,7 @@ 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';
@@ -23,7 +24,7 @@ import type {
 	Operator,
 } from '../../resources/ResourceInterface.ts';
 import { collapseData } from '../../resources/tracked.ts';
-import { errorToString } from '../../utility/logging/harper_logger.ts';
+import logger, { 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';
@@ -35,6 +36,9 @@ 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[];
 
 export type SearchByConditionsRequest = Query &
 	Context & {
@@ -203,6 +207,23 @@ export class ResourceBridge extends BridgeMethods {
 		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).
+		//
+		// 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.
+		//
+		// 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;
 		const Table = getDatabases()[upsertObj.schema][upsertObj.table];
 		const context: Context = {
 			user: upsertObj.hdb_user,
@@ -259,12 +280,24 @@ export class ResourceBridge extends BridgeMethods {
 						}
 					}
 				}
+				const unset = takeUnsetAttributes(record, Table.primaryKey, upsertObj.requires_no_existing);
+				// `__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.
+				const toWrite = unset.length && existingRecord && !fullRecord ? { ...existingRecord, ...record } : record;
+				for (const attribute of unset) {
+					delete toWrite[attribute];
+				}
 				await (id == undefined
-					? Table.create(record, context)
-					: existingRecord
-						? Table.patch(record, context)
-						: Table.put(record, context));
-				keys.push(record[Table.primaryKey]);
+					? Table.create(toWrite, context)
+					: existingRecord && !fullRecord && !unset.length
+						? Table.patch(toWrite, context)
+						: Table.put(toWrite, context));
+				keys.push(toWrite[Table.primaryKey]);
 			}
 			return {
 				txn_time: (transaction as any).timestamp,
@@ -771,3 +804,57 @@ 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.
+ *
+ * Removed from the record because it is a directive, not data: leaving it in place would store it as
+ * an attribute. Shape is already validated (`validation/insertValidator.ts`); the primary key is
+ * refused here instead, since only this layer knows which attribute that is. Unsetting it would
+ * otherwise be silently ignored — `Table._writeUpdate` re-asserts the primary key on a full update —
+ * and a directive that quietly does nothing is worse than one that says no.
+ */
+function takeUnsetAttributes(
+	record: Record<string, unknown>,
+	primaryKey: string,
+	isCreateOnly: boolean | undefined
+): string[] {
+	if (!(OPERATIONS_UNSET_KEY in record)) return NO_UNSET_ATTRIBUTES;
+	const unset = record[OPERATIONS_UNSET_KEY];
+	// Removed before the shape is judged, and on every exit below: the key is a directive, so
+	// leaving it on a record that is about to be written stores it as data. `in` rather than an
+	// `undefined` check for the same reason — an explicitly-undefined key still has to come off.
+	delete record[OPERATIONS_UNSET_KEY];
+	// `insert` has no existing record to remove anything from, so the directive can only mean the
+	// caller misunderstood it. Refused rather than applied: applying it deleted attributes the same
+	// request had just supplied, which is a silent partial write, and rather than ignored, because a
+	// directive that quietly does nothing is worse than one that says no — the same reason the primary
+	// key below is refused instead of skipped. Authorization contributes nothing for `insert` on the
+	// strength of this (`utility/operation_authorization.ts` getRecordAttributes).
+	if (isCreateOnly) {
+		throw new ClientError(
+			`'${OPERATIONS_UNSET_KEY}' is not valid on an insert, which has no existing record to remove attributes from`
+		);
+	}
+	// Self-safeguarding rather than trusting the caller. `validation/insertValidator.ts` rejects a
+	// malformed value, and every current route here goes through it (`dataLayer/insert.ts`
+	// create/update/upsertRecords, including the replication catchup path) — but the bridge's own
+	// `insertUpdateValidate` does not check this key, so that safety lives in a different layer, and
+	// this is a module boundary an internal caller could reach directly. A non-iterable reaching the
+	// `for…of` in `upsertRecords` would throw and take the write down; a bare string would iterate
+	// per character and delete single-letter attributes.
+	//
+	// 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. Skipping the directive degrades to a
+	// plain merge, which removes nothing — the safe direction. Same choice `resources/tracked.ts`
+	// makes for an unrecognized CRDT operation, and the reason it makes it.
+	if (!Array.isArray(unset) || unset.some((name) => typeof name !== 'string' || name.length === 0)) {
+		logger.warn(`Ignoring a malformed '${OPERATIONS_UNSET_KEY}' on a record; expected an array of attribute names`);
+		return NO_UNSET_ATTRIBUTES;
+	}
+	const names = unset as string[];
+	if (primaryKey && names.includes(primaryKey)) {
+		throw new ClientError(`'${primaryKey}' is the primary key of this table and cannot be unset`);
+	}
+	return names;
+}

dataLayer/harperBridge/bridgeUtility/insertUpdateValidate.js

@@ -4,6 +4,7 @@ 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;
 
@@ -73,6 +74,15 @@ 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,6 +15,7 @@ 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);
 
@@ -89,6 +90,32 @@ 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;
 		}
 	});