diff --git a/CHANGELOG.md b/CHANGELOG.md index ab11976..e25136f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## Unreleased + +### Bug Fixes + +- Fixed `put_newer/5` and `put_all_newer/3` raising `ArgumentError` ("not a + valid match specification") when the key contains a map. The key is now + bound to a match variable and compared in the guard, since ETS rejects raw + maps at the key position of a `select_replace/2` match head. + [#18](https://github.com/appcues/partitioned_buffer/issues/18). + ## Release 0.4.1 ### Bug Fixes diff --git a/lib/partitioned_buffer/partition.ex b/lib/partitioned_buffer/partition.ex index 33c4acf..2477d8f 100644 --- a/lib/partitioned_buffer/partition.ex +++ b/lib/partitioned_buffer/partition.ex @@ -518,25 +518,27 @@ defmodule PartitionedBuffer.Partition do end defp replace_match_spec(key, value, version) do - # Performance note: The key in the match head is a literal (bound value), - # not a pattern variable. This allows ETS to use its hash index for O(1) - # lookup rather than scanning the entire table. + # The key is bound to "$3" and compared in the guard rather than embedded + # as a literal in the match head. ETS rejects raw maps in a match head at + # the key position (see :ets.select_replace/2), so keys that contain a map + # would raise "not a valid match specification". Comparing via the guard + # works for any term shape. # - # In match spec bodies, bare tuples are interpreted as operations/function - # calls, NOT as literal data. We wrap key and value with ms_literal/1 so - # tuples use the {{...}} constructor form and maps use {:const, map} that - # ETS understands. This handles tuples, maps, and lists (including nested - # combinations). + # In match spec bodies and guards, bare tuples are interpreted as + # operations/function calls, NOT as literal data. We wrap key and value + # with ms_literal/1 so tuples use the {{...}} constructor form and maps use + # {:const, map} that ETS understands. This handles tuples, maps, and lists + # (including nested combinations). [ { - # Match: {entry, key, value, existing_version, updates} where key is literal - entry(key: key, value: :_, version: :"$1", updates: :"$2"), - # Guard (update only if): new_version > existing_version - [{:>, version, :"$1"}], + # Match: {entry, key, value, existing_version, updates} + entry(key: :"$3", value: :_, version: :"$1", updates: :"$2"), + # Guard (update only if): key matches and new_version > existing_version + [{:>, version, :"$1"}, {:"=:=", :"$3", ms_literal(key)}], # Result: the new entry with incremented updates counter [ {entry( - key: ms_literal(key), + key: :"$3", value: ms_literal(value), version: version, updates: {:+, :"$2", 1} diff --git a/test/partitioned_buffer/map_test.exs b/test/partitioned_buffer/map_test.exs index 3359241..6450691 100644 --- a/test/partitioned_buffer/map_test.exs +++ b/test/partitioned_buffer/map_test.exs @@ -514,6 +514,22 @@ defmodule PartitionedBuffer.MapTest do M.put_all_newer(buff, entries) end end + + test "ok: updates existing entry with a key containing a map", %{buffer: buff} do + key = {:user, %{id: 1}} + + assert M.put_newer(buff, key, "v1", 100) == :ok + assert M.put_newer(buff, key, "v2", 200) == :ok + + assert M.size(buff) == 1 + assert M.get(buff, key) == "v2" + + assert_receive {@processing_stop_event, %{duration: _, size: 1}, + %{buffer: ^buff, partition: _}}, + @default_timeout + + assert_receive {:process_completed, [{^key, "v2", 200, 1}]}, @default_timeout + end end describe "processing" do