Table size fixes (#21)

Fixes #20.

evict_to_size should only evict if it needs to. Previously, it would *always* evict
at least one record even if size was <= new_size.

Also, ensure we *always* send a dynamic table resize command.

Closer reading of RFC9113§4.3.1para¶4
(https://www.rfc-editor.org/rfc/rfc9113.html#section-4.3.1)
suggests that we should always send a dynamic resize.

Author: mtrudel

lib/hpax/table.ex

@@ -264,15 +264,6 @@ defmodule HPAX.Table do
   In all cases, the table's `:protocol_max_table_size` is updated accordingly
   """
   @spec resize(t(), non_neg_integer()) :: t()
-  def resize(%__MODULE__{max_table_size: max_table_size} = table, new_protocol_max_table_size)
-      when new_protocol_max_table_size >= max_table_size do
-    %__MODULE__{
-      table
-      | protocol_max_table_size: new_protocol_max_table_size,
-        max_table_size: new_protocol_max_table_size
-    }
-  end
-
   def resize(%__MODULE__{} = table, new_protocol_max_table_size) do
     pending_minimum_resize =
       case table.pending_minimum_resize do
@@ -317,6 +308,8 @@ defmodule HPAX.Table do
 
   # Removes records as necessary to have the total size of entries within the table be less than
   # or equal to the specified value. Does not change the table's max size.
+  defp evict_to_size(%__MODULE__{size: size} = table, new_size) when size <= new_size, do: table
+
   defp evict_to_size(%__MODULE__{entries: entries, size: size} = table, new_size) do
     {new_entries_reversed, new_size} =
       evict_towards_size(Enum.reverse(entries), size, new_size)

test/hpax/table_test.exs

@@ -80,25 +80,44 @@ defmodule HPAX.TableTest do
       assert table.protocol_max_table_size == 8192
     end
 
-    test "decreasing the protocol max table size not below the max table size" do
+    test "decreasing the protocol max table size but above table size" do
+      table = Table.new(4096, :never)
+      table = Table.add(table, "aaaa", "AAAA")
+      table = Table.resize(table, 2048)
+      assert table.size == 40
+      assert table.max_table_size == 2048
+      assert table.protocol_max_table_size == 2048
+    end
+
+    test "decreasing the protocol max table size below current size should evict" do
       table = Table.new(4096, :never)
       table = Table.add(table, "aaaa", "AAAA")
       table = Table.add(table, "bbbb", "BBBB")
+      table = Table.resize(table, 60)
+      assert table.size == 40
+      assert table.max_table_size == 60
+      assert table.protocol_max_table_size == 60
+    end
+  end
+
+  describe "dynamically resizing" do
+    test "decreasing the max table size but above table size" do
+      table = Table.new(4096, :never)
+      table = Table.add(table, "aaaa", "AAAA")
       table = Table.dynamic_resize(table, 2048)
-      table = Table.resize(table, 6000)
       assert table.size == 40
-      assert table.max_table_size == 6000
-      assert table.protocol_max_table_size == 6000
+      assert table.max_table_size == 2048
+      assert table.protocol_max_table_size == 4096
     end
 
-    test "decreasing the protocol max table size below the max table size" do
+    test "decreasing the protocol max table size below current size should evict" do
       table = Table.new(4096, :never)
       table = Table.add(table, "aaaa", "AAAA")
       table = Table.add(table, "bbbb", "BBBB")
-      table = Table.resize(table, 40)
+      table = Table.dynamic_resize(table, 60)
       assert table.size == 40
-      assert table.max_table_size == 40
-      assert table.protocol_max_table_size == 40
+      assert table.max_table_size == 60
+      assert table.protocol_max_table_size == 4096
     end
   end
 end

test/hpax_test.exs

@@ -109,7 +109,6 @@ defmodule HPAXTest do
 
   property "encode/3 prepends dynamic resizes at the start of a block" do
     enc_table = HPAX.new(20_000)
-    # Start with a non-empty decode table
     dec_table = HPAX.new(20_000)
 
     # Put a record in both to prime the pump. The table sizes should match
@@ -120,14 +119,45 @@ defmodule HPAXTest do
     assert enc_table.max_table_size == 20_000
     assert dec_table.max_table_size == 20_000
 
-    # Encode a record after resizing the table. We expect a dynamic resize to be
-    # encoded and the for two table sizes to be identical after decoding
+    # Scenario 1: Simulate the decoder growing the table via settings
+
+    # First, the decoder resizes its table to some maximum size
+    dec_table = HPAX.resize(dec_table, 40_000)
+
+    # It then communicates that size to the encoder, who chooses a smaller size
+    enc_table = HPAX.resize(enc_table, 30_000)
+
+    # Now, encode a header
+    {encoded, enc_table} = HPAX.encode([{:store, "lame", "LAME"}], enc_table)
+    encoded = IO.iodata_to_binary(encoded)
+
+    # Ensure that we encoded a resize on the wire
+    assert <<0b001::3, rest::bitstring>> = encoded
+    assert {:ok, 30_000, _rest} = HPAX.Types.decode_integer(rest, 5)
+
+    # Finally, ensure that the decoder makes proper sense of this encoding and that it resizes
+    # back down to the size chosen by the encoder
+    assert {:ok, _decoded, dec_table} = HPAX.decode(encoded, dec_table)
+    assert dec_table.size == enc_table.size
+    assert enc_table.max_table_size == 30_000
+    assert dec_table.max_table_size == 30_000
+
+    # Scenario 2: Simulate the decoder shrinking the table ia settings
+
+    # First, the decoder resizes its table to some maximum size
+    dec_table = HPAX.resize(dec_table, 10_000)
+
+    # It then communicates that size to the encoder, who chooses a smaller size
     enc_table = HPAX.resize(enc_table, 0)
+
+    # It then changes its mind and goes back up to a size still smaller than the decoder's choice
     enc_table = HPAX.resize(enc_table, 1234)
+
+    # Now, encode a header
     {encoded, enc_table} = HPAX.encode([{:store, "lame", "LAME"}], enc_table)
     encoded = IO.iodata_to_binary(encoded)
 
-    # Ensure that we see two resizes in order
+    # Ensure that we encoded two resizes in order on the wire
     assert <<0b001::3, rest::bitstring>> = encoded
     assert {:ok, 0, rest} = HPAX.Types.decode_integer(rest, 5)
     assert <<0b001::3, rest::bitstring>> = rest