Refactor QpackEncoder and QpackDecoder for simplicity.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java

@@ -48,6 +48,7 @@ public class QpackDecoder implements Dumpable
     private final DecoderInstructionParser _parser;
     private final List<EncodedFieldSection> _encodedFieldSections = new ArrayList<>();
     private final NBitIntegerParser _integerDecoder = new NBitIntegerParser();
+    private final InstructionHandler _instructionHandler = new InstructionHandler();
     private final int _maxHeaderSize;
 
     private static class MetaDataNotification
@@ -76,7 +77,7 @@ public class QpackDecoder implements Dumpable
     {
         _context = new QpackContext();
         _handler = handler;
-        _parser = new DecoderInstructionParser(new InstructionHandler());
+        _parser = new DecoderInstructionParser(_instructionHandler);
         _maxHeaderSize = maxHeaderSize;
     }
 
@@ -211,56 +212,6 @@ public class QpackDecoder implements Dumpable
         }
     }
 
-    void setCapacity(int capacity)
-    {
-        _context.getDynamicTable().setCapacity(capacity);
-    }
-
-    void insert(int index) throws QpackException
-    {
-        if (LOG.isDebugEnabled())
-            LOG.debug("Duplicate: index={}", index);
-
-        DynamicTable dynamicTable = _context.getDynamicTable();
-        Entry referencedEntry = dynamicTable.get(index);
-
-        // Add the new Entry to the DynamicTable.
-        Entry entry = new Entry(referencedEntry.getHttpField());
-        dynamicTable.add(entry);
-        _instructions.add(new InsertCountIncrementInstruction(1));
-        checkEncodedFieldSections();
-    }
-
-    void insert(int nameIndex, boolean isDynamicTableIndex, String value) throws QpackException
-    {
-        if (LOG.isDebugEnabled())
-            LOG.debug("InsertNameReference: nameIndex={}, dynamic={}, value={}", nameIndex, isDynamicTableIndex, value);
-
-        StaticTable staticTable = QpackContext.getStaticTable();
-        DynamicTable dynamicTable = _context.getDynamicTable();
-        Entry referencedEntry = isDynamicTableIndex ? dynamicTable.get(nameIndex) : staticTable.get(nameIndex);
-
-        // Add the new Entry to the DynamicTable.
-        Entry entry = new Entry(new HttpField(referencedEntry.getHttpField().getHeader(), referencedEntry.getHttpField().getName(), value));
-        dynamicTable.add(entry);
-        _instructions.add(new InsertCountIncrementInstruction(1));
-        checkEncodedFieldSections();
-    }
-
-    void insert(String name, String value) throws QpackException
-    {
-        if (LOG.isDebugEnabled())
-            LOG.debug("InsertLiteralEntry: name={}, value={}", name, value);
-
-        Entry entry = new Entry(new HttpField(name, value));
-
-        // Add the new Entry to the DynamicTable.
-        DynamicTable dynamicTable = _context.getDynamicTable();
-        dynamicTable.add(entry);
-        _instructions.add(new InsertCountIncrementInstruction(1));
-        checkEncodedFieldSections();
-    }
-
     private static int decodeInsertCount(int encInsertCount, int totalNumInserts, int maxTableCapacity) throws QpackException
     {
         if (encInsertCount == 0)
@@ -319,6 +270,11 @@ public class QpackDecoder implements Dumpable
         _metaDataNotifications.clear();
     }
 
+    InstructionHandler getInstructionHandler()
+    {
+        return _instructionHandler;
+    }
+
     /**
      * This delivers notifications from the DecoderInstruction parser directly into the Decoder.
      */
@@ -327,25 +283,55 @@ public class QpackDecoder implements Dumpable
         @Override
         public void onSetDynamicTableCapacity(int capacity)
         {
-            setCapacity(capacity);
+            _context.getDynamicTable().setCapacity(capacity);
         }
 
         @Override
         public void onDuplicate(int index) throws QpackException
         {
-            insert(index);
+            if (LOG.isDebugEnabled())
+                LOG.debug("Duplicate: index={}", index);
+
+            DynamicTable dynamicTable = _context.getDynamicTable();
+            Entry referencedEntry = dynamicTable.get(index);
+
+            // Add the new Entry to the DynamicTable.
+            Entry entry = new Entry(referencedEntry.getHttpField());
+            dynamicTable.add(entry);
+            _instructions.add(new InsertCountIncrementInstruction(1));
+            checkEncodedFieldSections();
         }
 
         @Override
         public void onInsertNameWithReference(int nameIndex, boolean isDynamicTableIndex, String value) throws QpackException
         {
-            insert(nameIndex, isDynamicTableIndex, value);
+            if (LOG.isDebugEnabled())
+                LOG.debug("InsertNameReference: nameIndex={}, dynamic={}, value={}", nameIndex, isDynamicTableIndex, value);
+
+            StaticTable staticTable = QpackContext.getStaticTable();
+            DynamicTable dynamicTable = _context.getDynamicTable();
+            Entry referencedEntry = isDynamicTableIndex ? dynamicTable.get(nameIndex) : staticTable.get(nameIndex);
+
+            // Add the new Entry to the DynamicTable.
+            Entry entry = new Entry(new HttpField(referencedEntry.getHttpField().getHeader(), referencedEntry.getHttpField().getName(), value));
+            dynamicTable.add(entry);
+            _instructions.add(new InsertCountIncrementInstruction(1));
+            checkEncodedFieldSections();
         }
 
         @Override
         public void onInsertWithLiteralName(String name, String value) throws QpackException
         {
-            insert(name, value);
+            if (LOG.isDebugEnabled())
+                LOG.debug("InsertLiteralEntry: name={}, value={}", name, value);
+
+            Entry entry = new Entry(new HttpField(name, value));
+
+            // Add the new Entry to the DynamicTable.
+            DynamicTable dynamicTable = _context.getDynamicTable();
+            dynamicTable.add(entry);
+            _instructions.add(new InsertCountIncrementInstruction(1));
+            checkEncodedFieldSections();
         }
     }
 }

jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java

@@ -92,6 +92,7 @@ public class QpackEncoder implements Dumpable
     private final int _maxBlockedStreams;
     private final Map<Long, StreamInfo> _streamInfoMap = new HashMap<>();
     private final EncoderInstructionParser _parser;
+    private final InstructionHandler _instructionHandler = new InstructionHandler();
     private int _knownInsertCount = 0;
     private int _blockedStreams = 0;
 
@@ -100,7 +101,7 @@ public class QpackEncoder implements Dumpable
         _handler = handler;
         _context = new QpackContext();
         _maxBlockedStreams = maxBlockedStreams;
-        _parser = new EncoderInstructionParser(new InstructionHandler());
+        _parser = new EncoderInstructionParser(_instructionHandler);
     }
 
     /**
@@ -361,52 +362,6 @@ public class QpackEncoder implements Dumpable
         }
     }
 
-    void insertCountIncrement(int increment) throws QpackException
-    {
-        if (LOG.isDebugEnabled())
-            LOG.debug("InsertCountIncrement: increment={}", increment);
-
-        int insertCount = _context.getDynamicTable().getInsertCount();
-        if (_knownInsertCount + increment > insertCount)
-            throw new QpackException.SessionException(QPACK_ENCODER_STREAM_ERROR, "KnownInsertCount incremented over InsertCount");
-        _knownInsertCount += increment;
-    }
-
-    void sectionAcknowledgement(long streamId) throws QpackException
-    {
-        if (LOG.isDebugEnabled())
-            LOG.debug("SectionAcknowledgement: streamId={}", streamId);
-
-        StreamInfo streamInfo = _streamInfoMap.get(streamId);
-        if (streamInfo == null)
-            throw new QpackException.SessionException(QPACK_ENCODER_STREAM_ERROR, "No StreamInfo for " + streamId);
-
-        // The KnownInsertCount should be updated to the earliest sent RequiredInsertCount on that stream.
-        StreamInfo.SectionInfo sectionInfo = streamInfo.acknowledge();
-        sectionInfo.release();
-        _knownInsertCount = Math.max(_knownInsertCount, sectionInfo.getRequiredInsertCount());
-
-        // If we have no more outstanding section acknowledgments remove the StreamInfo.
-        if (streamInfo.isEmpty())
-            _streamInfoMap.remove(streamId);
-    }
-
-    void streamCancellation(long streamId) throws QpackException
-    {
-        if (LOG.isDebugEnabled())
-            LOG.debug("StreamCancellation: streamId={}", streamId);
-
-        StreamInfo streamInfo = _streamInfoMap.remove(streamId);
-        if (streamInfo == null)
-            throw new QpackException.SessionException(QPACK_ENCODER_STREAM_ERROR, "No StreamInfo for " + streamId);
-
-        // Release all referenced entries outstanding on the stream that was cancelled.
-        for (StreamInfo.SectionInfo sectionInfo : streamInfo)
-        {
-            sectionInfo.release();
-        }
-    }
-
     private boolean referenceEntry(Entry entry, StreamInfo streamInfo)
     {
         if (entry == null)
@@ -463,24 +418,60 @@ public class QpackEncoder implements Dumpable
         _instructions.clear();
     }
 
+    InstructionHandler getInstructionHandler()
+    {
+        return _instructionHandler;
+    }
+
     class InstructionHandler implements EncoderInstructionParser.Handler
     {
         @Override
         public void onSectionAcknowledgement(long streamId) throws QpackException
         {
-            sectionAcknowledgement(streamId);
+            if (LOG.isDebugEnabled())
+                LOG.debug("SectionAcknowledgement: streamId={}", streamId);
+
+            StreamInfo streamInfo = _streamInfoMap.get(streamId);
+            if (streamInfo == null)
+                throw new QpackException.SessionException(QPACK_ENCODER_STREAM_ERROR, "No StreamInfo for " + streamId);
+
+            // The KnownInsertCount should be updated to the earliest sent RequiredInsertCount on that stream.
+            StreamInfo.SectionInfo sectionInfo = streamInfo.acknowledge();
+            sectionInfo.release();
+            _knownInsertCount = Math.max(_knownInsertCount, sectionInfo.getRequiredInsertCount());
+
+            // If we have no more outstanding section acknowledgments remove the StreamInfo.
+            if (streamInfo.isEmpty())
+                _streamInfoMap.remove(streamId);
         }
 
         @Override
         public void onStreamCancellation(long streamId) throws QpackException
         {
-            streamCancellation(streamId);
+            if (LOG.isDebugEnabled())
+                LOG.debug("StreamCancellation: streamId={}", streamId);
+
+            StreamInfo streamInfo = _streamInfoMap.remove(streamId);
+            if (streamInfo == null)
+                throw new QpackException.SessionException(QPACK_ENCODER_STREAM_ERROR, "No StreamInfo for " + streamId);
+
+            // Release all referenced entries outstanding on the stream that was cancelled.
+            for (StreamInfo.SectionInfo sectionInfo : streamInfo)
+            {
+                sectionInfo.release();
+            }
         }
 
         @Override
         public void onInsertCountIncrement(int increment) throws QpackException
         {
-            insertCountIncrement(increment);
+            if (LOG.isDebugEnabled())
+                LOG.debug("InsertCountIncrement: increment={}", increment);
+
+            int insertCount = _context.getDynamicTable().getInsertCount();
+            if (_knownInsertCount + increment > insertCount)
+                throw new QpackException.SessionException(QPACK_ENCODER_STREAM_ERROR, "KnownInsertCount incremented over InsertCount");
+            _knownInsertCount += increment;
         }
     }
 

jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/DecoderParserDebugHandler.java

@@ -26,6 +26,7 @@ public class DecoderParserDebugHandler implements DecoderInstructionParser.Handl
     public Queue<ReferencedEntry> referencedNameEntries = new LinkedList<>();
 
     private final QpackDecoder _decoder;
+    private final DecoderInstructionParser.Handler _decoderHandler;
 
     public DecoderParserDebugHandler()
     {
@@ -35,6 +36,7 @@ public class DecoderParserDebugHandler implements DecoderInstructionParser.Handl
     public DecoderParserDebugHandler(QpackDecoder decoder)
     {
         _decoder = decoder;
+        _decoderHandler = decoder == null ? null : decoder.getInstructionHandler();
     }
 
     public static class LiteralEntry
@@ -64,11 +66,11 @@ public class DecoderParserDebugHandler implements DecoderInstructionParser.Handl
     }
 
     @Override
-    public void onSetDynamicTableCapacity(int capacity)
+    public void onSetDynamicTableCapacity(int capacity) throws QpackException
     {
         setCapacities.add(capacity);
         if (_decoder != null)
-            _decoder.setCapacity(capacity);
+            _decoderHandler.onSetDynamicTableCapacity(capacity);
     }
 
     @Override
@@ -76,7 +78,7 @@ public class DecoderParserDebugHandler implements DecoderInstructionParser.Handl
     {
         duplicates.add(index);
         if (_decoder != null)
-            _decoder.insert(index);
+            _decoderHandler.onDuplicate(index);
     }
 
     @Override
@@ -84,7 +86,7 @@ public class DecoderParserDebugHandler implements DecoderInstructionParser.Handl
     {
         referencedNameEntries.add(new ReferencedEntry(nameIndex, isDynamicTableIndex, value));
         if (_decoder != null)
-            _decoder.insert(nameIndex, isDynamicTableIndex, value);
+            _decoderHandler.onInsertNameWithReference(nameIndex, isDynamicTableIndex, value);
     }
 
     @Override
@@ -92,7 +94,7 @@ public class DecoderParserDebugHandler implements DecoderInstructionParser.Handl
     {
         literalNameEntries.add(new LiteralEntry(name, value));
         if (_decoder != null)
-            _decoder.insert(name, value);
+            _decoderHandler.onInsertWithLiteralName(name, value);
     }
 
     public boolean isEmpty()

jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncodeDecodeTest.java

@@ -148,6 +148,6 @@ public class EncodeDecodeTest
         instruction = _encoderHandler.getInstruction();
         assertThat(instruction, instanceOf(LiteralNameEntryInstruction.class));
         assertThat(QpackTestUtil.toHexString(instruction), QpackTestUtil.equalsHex("4a63 7573 746f 6d2d 6b65 790c 6375 7374 6f6d 2d76 616c 7565"));
-        _encoder.insertCountIncrement(1);
+        _encoder.getInstructionHandler().onInsertCountIncrement(1);
     }
 }

jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncoderParserDebugHandler.java

@@ -25,6 +25,7 @@ public class EncoderParserDebugHandler implements EncoderInstructionParser.Handl
     public Queue<Integer> insertCountIncrements = new LinkedList<>();
 
     private final QpackEncoder _encoder;
+    private final QpackEncoder.InstructionHandler _instructionHandler;
 
     public EncoderParserDebugHandler()
     {
@@ -34,6 +35,7 @@ public class EncoderParserDebugHandler implements EncoderInstructionParser.Handl
     public EncoderParserDebugHandler(QpackEncoder encoder)
     {
         _encoder = encoder;
+        _instructionHandler = encoder == null ? null : encoder.getInstructionHandler();
     }
 
     @Override
@@ -41,7 +43,7 @@ public class EncoderParserDebugHandler implements EncoderInstructionParser.Handl
     {
         sectionAcknowledgements.add(streamId);
         if (_encoder != null)
-            _encoder.sectionAcknowledgement(streamId);
+            _instructionHandler.onSectionAcknowledgement(streamId);
     }
 
     @Override
@@ -49,7 +51,7 @@ public class EncoderParserDebugHandler implements EncoderInstructionParser.Handl
     {
         streamCancellations.add(streamId);
         if (_encoder != null)
-            _encoder.streamCancellation(streamId);
+            _instructionHandler.onStreamCancellation(streamId);
     }
 
     @Override
@@ -57,7 +59,7 @@ public class EncoderParserDebugHandler implements EncoderInstructionParser.Handl
     {
         insertCountIncrements.add(increment);
         if (_encoder != null)
-            _encoder.insertCountIncrement(increment);
+            _instructionHandler.onInsertCountIncrement(increment);
     }
 
     public boolean isEmpty()