From 20ba8ed112b8df203a037329609258a4aaf5730f Mon Sep 17 00:00:00 2001 From: Hein Date: Wed, 30 Sep 2026 17:03:42 +0200 Subject: [PATCH] feat(update): allow clearing values with "" and null on update Update handlers skipped empty strings and nulls, so a client could not blank or null out a column. Every key present in the payload now overwrites the stored value, including "" and null. - add common.MergeUpdateValues and use it in resolvespec and restheadspec - add Handler.SetDisallowNulls to skip null values (""still overwrites) - websocketspec and mqttspec now write only the keys present in the payload via SetMap instead of updating the whole zeroed model, which clobbered absent fields --- pkg/common/update_merge.go | 23 +++++++++++++++ pkg/common/update_merge_test.go | 29 +++++++++++++++++++ pkg/mqttspec/handler.go | 49 ++++++++++++++++++++++---------- pkg/resolvespec/handler.go | 50 ++++++++++----------------------- pkg/restheadspec/handler.go | 26 ++++++++--------- pkg/websocketspec/handler.go | 49 ++++++++++++++++++++++---------- 6 files changed, 146 insertions(+), 80 deletions(-) create mode 100644 pkg/common/update_merge.go create mode 100644 pkg/common/update_merge_test.go diff --git a/pkg/common/update_merge.go b/pkg/common/update_merge.go new file mode 100644 index 0000000..3d7f5cf --- /dev/null +++ b/pkg/common/update_merge.go @@ -0,0 +1,23 @@ +package common + +// MergeUpdateValues merges the incoming request values into the existing +// record map (in place) and returns it. +// +// Every key present in incoming overwrites the existing value, including empty +// strings and explicit nulls, so clients can clear a column by sending "" or +// null. Keys absent from incoming are left untouched. +// +// When disallowNulls is true, nil values are skipped and the existing value is +// kept. Empty strings are still applied. +func MergeUpdateValues(existing, incoming map[string]interface{}, disallowNulls bool) map[string]interface{} { + if existing == nil { + existing = make(map[string]interface{}, len(incoming)) + } + for key, newValue := range incoming { + if newValue == nil && disallowNulls { + continue + } + existing[key] = newValue + } + return existing +} diff --git a/pkg/common/update_merge_test.go b/pkg/common/update_merge_test.go new file mode 100644 index 0000000..1eb0406 --- /dev/null +++ b/pkg/common/update_merge_test.go @@ -0,0 +1,29 @@ +package common + +import "testing" + +func TestMergeUpdateValues(t *testing.T) { + newExisting := func() map[string]interface{} { + return map[string]interface{}{"name": "old", "note": "keep", "other": "x"} + } + incoming := map[string]interface{}{"name": "", "note": nil} + + got := MergeUpdateValues(newExisting(), incoming, false) + if got["name"] != "" { + t.Errorf("name = %v, want empty string", got["name"]) + } + if v, ok := got["note"]; !ok || v != nil { + t.Errorf("note = %v (present=%v), want nil", v, ok) + } + if got["other"] != "x" { + t.Errorf("absent key changed: %v", got["other"]) + } + + got = MergeUpdateValues(newExisting(), incoming, true) + if got["name"] != "" { + t.Errorf("disallowNulls: name = %v, want empty string", got["name"]) + } + if got["note"] != "keep" { + t.Errorf("disallowNulls: note = %v, want keep", got["note"]) + } +} diff --git a/pkg/mqttspec/handler.go b/pkg/mqttspec/handler.go index c6acad8..841a0c9 100644 --- a/pkg/mqttspec/handler.go +++ b/pkg/mqttspec/handler.go @@ -45,6 +45,9 @@ type Handler struct { // Started flag started bool mu sync.RWMutex + + // disallowNulls skips null values in update payloads instead of applying them + disallowNulls bool } // NewHandler creates a new MQTT handler @@ -812,6 +815,14 @@ func (h *Handler) readMultiple(hookCtx *HookContext) (data interface{}, metadata } // create creates a new record +// SetDisallowNulls controls whether explicit null values in update payloads are +// ignored. By default a key present in the payload overwrites the stored value, +// including "" and null. When true, null values are skipped and the existing +// value is kept ("" still overwrites). +func (h *Handler) SetDisallowNulls(disallow bool) { + h.disallowNulls = disallow +} + func (h *Handler) create(hookCtx *HookContext) (interface{}, error) { // Marshal and unmarshal data into model dataBytes, err := json.Marshal(hookCtx.Data) @@ -840,25 +851,33 @@ func (h *Handler) create(hookCtx *HookContext) (interface{}, error) { // update updates an existing record func (h *Handler) update(hookCtx *HookContext) (interface{}, error) { - // Marshal and unmarshal data into model - dataBytes, err := json.Marshal(hookCtx.Data) - if err != nil { - return nil, fmt.Errorf("failed to marshal data: %w", err) + // Convert request data to a map + var updates map[string]interface{} + if m, ok := hookCtx.Data.(map[string]interface{}); ok { + updates = m + } else { + dataBytes, err := json.Marshal(hookCtx.Data) + if err != nil { + return nil, fmt.Errorf("failed to marshal data: %w", err) + } + if err := json.Unmarshal(dataBytes, &updates); err != nil { + return nil, fmt.Errorf("failed to unmarshal data into map: %w", err) + } } - if err := json.Unmarshal(dataBytes, hookCtx.ModelPtr); err != nil { - return nil, fmt.Errorf("failed to unmarshal data into model: %w", err) - } - - // Update record - query := h.db.NewUpdate().Model(hookCtx.ModelPtr).Table(hookCtx.TableName) - - // Add ID filter pkName := reflection.GetPrimaryKeyName(hookCtx.Model) - query = query.Where(fmt.Sprintf("%s = ?", pkName), hookCtx.ID) - if _, err := query.Exec(hookCtx.Context); err != nil { - return nil, fmt.Errorf("failed to update record: %w", err) + // Only the keys present in the request are written. "" and null overwrite + // the stored value unless disallowNulls is set, in which case null is skipped. + values := common.MergeUpdateValues(make(map[string]interface{}, len(updates)), updates, h.disallowNulls) + + if len(values) > 0 { + query := h.db.NewUpdate().Table(hookCtx.TableName).SetMap(values). + Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), hookCtx.ID) + + if _, err := query.Exec(hookCtx.Context); err != nil { + return nil, fmt.Errorf("failed to update record: %w", err) + } } // Fetch updated record diff --git a/pkg/resolvespec/handler.go b/pkg/resolvespec/handler.go index 5bc7f25..f582a2f 100644 --- a/pkg/resolvespec/handler.go +++ b/pkg/resolvespec/handler.go @@ -32,6 +32,7 @@ type Handler struct { fallbackHandler FallbackHandler openAPIGenerator func() (string, error) defaultSort map[string][]common.SortOption + disallowNulls bool } // NewHandler creates a new API handler with database and registry abstractions @@ -52,6 +53,14 @@ func (h *Handler) Hooks() *HookRegistry { return h.hooks } +// SetDisallowNulls controls whether explicit null values in update payloads are +// ignored. By default a key present in the payload overwrites the stored value, +// including "" and null. When true, null values are skipped and the existing +// value is kept ("" still overwrites). +func (h *Handler) SetDisallowNulls(disallow bool) { + h.disallowNulls = disallow +} + // SetFallbackHandler sets a fallback handler to be called when no model is found // If not set, the handler will simply return (pass through to next route) func (h *Handler) SetFallbackHandler(fallback FallbackHandler) { @@ -1236,21 +1245,8 @@ func (h *Handler) handleUpdate(ctx context.Context, w common.ResponseWriter, url return fmt.Errorf("error unmarshaling existing record: %w", err) } - // Merge only non-null and non-empty values from the incoming request into the existing record - for key, newValue := range updates { - // Skip if the value is nil - if newValue == nil { - continue - } - - // Skip if the value is an empty string - if strVal, ok := newValue.(string); ok && strVal == "" { - continue - } - - // Update the existing map with the new value - existingMap[key] = newValue - } + // Overwrite with every key present in the request (including "" and null unless disallowed) + common.MergeUpdateValues(existingMap, updates, h.disallowNulls) // Build update query with merged data query := tx.NewUpdate().Table(tableName).SetMap(existingMap) @@ -1421,16 +1417,8 @@ func (h *Handler) handleUpdate(ctx context.Context, w common.ResponseWriter, url item = modifiedData } - // Merge only non-null and non-empty values - for key, newValue := range item { - if newValue == nil { - continue - } - if strVal, ok := newValue.(string); ok && strVal == "" { - continue - } - existingMap[key] = newValue - } + // Overwrite with every key present in the request (including "" and null unless disallowed) + common.MergeUpdateValues(existingMap, item, h.disallowNulls) txQuery := tx.NewUpdate().Table(tableName).SetMap(existingMap).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), itemID) if _, err := txQuery.Exec(ctx); err != nil { @@ -1578,16 +1566,8 @@ func (h *Handler) handleUpdate(ctx context.Context, w common.ResponseWriter, url itemMap = modifiedData } - // Merge only non-null and non-empty values - for key, newValue := range itemMap { - if newValue == nil { - continue - } - if strVal, ok := newValue.(string); ok && strVal == "" { - continue - } - existingMap[key] = newValue - } + // Overwrite with every key present in the request (including "" and null unless disallowed) + common.MergeUpdateValues(existingMap, itemMap, h.disallowNulls) txQuery := tx.NewUpdate().Table(tableName).SetMap(existingMap).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), itemID) if _, err := txQuery.Exec(ctx); err != nil { diff --git a/pkg/restheadspec/handler.go b/pkg/restheadspec/handler.go index d04040d..d532108 100644 --- a/pkg/restheadspec/handler.go +++ b/pkg/restheadspec/handler.go @@ -33,6 +33,7 @@ type Handler struct { fallbackHandler FallbackHandler openAPIGenerator func() (string, error) defaultSort map[string][]common.SortOption + disallowNulls bool } // NewHandler creates a new API handler with database and registry abstractions @@ -59,6 +60,14 @@ func (h *Handler) Hooks() *HookRegistry { return h.hooks } +// SetDisallowNulls controls whether explicit null values in update payloads are +// ignored. By default a key present in the payload overwrites the stored value, +// including "" and null. When true, null values are skipped and the existing +// value is kept ("" still overwrites). +func (h *Handler) SetDisallowNulls(disallow bool) { + h.disallowNulls = disallow +} + // SetFallbackHandler sets a fallback handler to be called when no model is found // If not set, the handler will simply return (pass through to next route) func (h *Handler) SetFallbackHandler(fallback FallbackHandler) { @@ -1597,21 +1606,8 @@ func (h *Handler) handleUpdate(ctx context.Context, w common.ResponseWriter, id nestedRelations = relations } - // Merge only non-null and non-empty values from the incoming request into the existing record - for key, newValue := range dataMap { - // Skip if the value is nil - if newValue == nil { - continue - } - - // Skip if the value is an empty string - if strVal, ok := newValue.(string); ok && strVal == "" { - continue - } - - // Update the existing map with the new value - existingMap[key] = newValue - } + // Overwrite with every key present in the request (including "" and null unless disallowed) + common.MergeUpdateValues(existingMap, dataMap, h.disallowNulls) // Ensure ID is in the data map for the update existingMap[pkName] = targetID diff --git a/pkg/websocketspec/handler.go b/pkg/websocketspec/handler.go index 8c9d9ec..0dbe2f4 100644 --- a/pkg/websocketspec/handler.go +++ b/pkg/websocketspec/handler.go @@ -38,6 +38,9 @@ type Handler struct { subscriptionManager *SubscriptionManager upgrader websocket.Upgrader ctx context.Context + + // disallowNulls skips null values in update payloads instead of applying them + disallowNulls bool } // NewHandler creates a new WebSocket handler @@ -682,6 +685,14 @@ func (h *Handler) readMultiple(hookCtx *HookContext) (data interface{}, metadata return hookCtx.ModelPtr, metadata, nil } +// SetDisallowNulls controls whether explicit null values in update payloads are +// ignored. By default a key present in the payload overwrites the stored value, +// including "" and null. When true, null values are skipped and the existing +// value is kept ("" still overwrites). +func (h *Handler) SetDisallowNulls(disallow bool) { + h.disallowNulls = disallow +} + func (h *Handler) create(hookCtx *HookContext) (interface{}, error) { // Marshal and unmarshal data into model dataBytes, err := json.Marshal(hookCtx.Data) @@ -709,25 +720,33 @@ func (h *Handler) create(hookCtx *HookContext) (interface{}, error) { } func (h *Handler) update(hookCtx *HookContext) (interface{}, error) { - // Marshal and unmarshal data into model - dataBytes, err := json.Marshal(hookCtx.Data) - if err != nil { - return nil, fmt.Errorf("failed to marshal data: %w", err) + // Convert request data to a map + var updates map[string]interface{} + if m, ok := hookCtx.Data.(map[string]interface{}); ok { + updates = m + } else { + dataBytes, err := json.Marshal(hookCtx.Data) + if err != nil { + return nil, fmt.Errorf("failed to marshal data: %w", err) + } + if err := json.Unmarshal(dataBytes, &updates); err != nil { + return nil, fmt.Errorf("failed to unmarshal data into map: %w", err) + } } - if err := json.Unmarshal(dataBytes, hookCtx.ModelPtr); err != nil { - return nil, fmt.Errorf("failed to unmarshal data into model: %w", err) - } - - // Update record - query := h.db.NewUpdate().Model(hookCtx.ModelPtr).Table(hookCtx.TableName) - - // Add ID filter pkName := reflection.GetPrimaryKeyName(hookCtx.Model) - query = query.Where(fmt.Sprintf("%s = ?", pkName), hookCtx.ID) - if _, err := query.Exec(hookCtx.Context); err != nil { - return nil, fmt.Errorf("failed to update record: %w", err) + // Only the keys present in the request are written. "" and null overwrite + // the stored value unless disallowNulls is set, in which case null is skipped. + values := common.MergeUpdateValues(make(map[string]interface{}, len(updates)), updates, h.disallowNulls) + + if len(values) > 0 { + query := h.db.NewUpdate().Table(hookCtx.TableName).SetMap(values). + Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), hookCtx.ID) + + if _, err := query.Exec(hookCtx.Context); err != nil { + return nil, fmt.Errorf("failed to update record: %w", err) + } } // Fetch updated record