[GitHub] [httpcomponents-client] ok2c commented on a diff in pull request #429: Improve HttpByteArrayCacheEntrySerializer class by adding new methods and enhancing performance.

["ok2c (via GitHub)" <gi...@apache.org>]

ok2c commented on code in PR #429:
URL: https://github.com/apache/httpcomponents-client/pull/429#discussion_r1154034009


##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpByteArrayCacheEntrySerializer.java:
##########
@@ -42,36 +44,68 @@
 import org.apache.hc.client5.http.cache.HttpCacheStorageEntry;
 import org.apache.hc.client5.http.cache.Resource;
 import org.apache.hc.client5.http.cache.ResourceIOException;
-import org.apache.hc.core5.annotation.Experimental;
-import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.annotation.Contract;
+import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.HttpResponse;
 import org.apache.hc.core5.http.HttpVersion;
 import org.apache.hc.core5.http.ProtocolVersion;
+import org.apache.hc.core5.http.config.Http1Config;
 import org.apache.hc.core5.http.impl.io.AbstractMessageParser;
 import org.apache.hc.core5.http.impl.io.AbstractMessageWriter;
 import org.apache.hc.core5.http.impl.io.DefaultHttpResponseParser;
 import org.apache.hc.core5.http.impl.io.SessionInputBufferImpl;
 import org.apache.hc.core5.http.impl.io.SessionOutputBufferImpl;
 import org.apache.hc.core5.http.io.SessionInputBuffer;
 import org.apache.hc.core5.http.io.SessionOutputBuffer;
+import org.apache.hc.core5.http.message.BasicHeader;
 import org.apache.hc.core5.http.message.BasicHttpRequest;
 import org.apache.hc.core5.http.message.BasicLineFormatter;
+import org.apache.hc.core5.http.message.HeaderGroup;
+import org.apache.hc.core5.http.message.LazyLineParser;
 import org.apache.hc.core5.http.message.StatusLine;
 import org.apache.hc.core5.util.CharArrayBuffer;
 import org.apache.hc.core5.util.TimeValue;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
- * Cache serializer and deserializer that uses an HTTP-like format.
+ * Serializes and deserializes byte arrays into HTTP cache entries using a default buffer size of 8192 bytes.
+ * The cache entries contain an HTTP response generated from the byte array data, which can be used to generate
+ * HTTP responses for cache hits.
+ * <p>
+ * This implementation uses the Apache HttpComponents library to perform the serialization and deserialization.
+ * <p>
+ * To serialize a byte array into an HTTP cache entry, use the {@link #serialize(HttpCacheStorageEntry)} method. To deserialize an HTTP cache
+ * entry into a byte array, use the {@link #deserialize(byte[])} method.
+ * <p>
+ * This class implements the {@link HttpCacheEntrySerializer} interface, which defines the contract for HTTP cache
+ * entry serialization and deserialization. It also includes a default buffer size of 8192 bytes, which can be
+ * overridden by specifying a different buffer size in the constructor.
+ * <p>
+ * Note that this implementation only supports HTTP responses and does not support HTTP requests or any other types of
+ * HTTP messages.
  *
- * Existing libraries for reading and writing HTTP are used, and metadata is encoded into HTTP
- * pseudo-headers for storage.
+ * @since 5.3
  */
-@Experimental
+@Contract(threading = ThreadingBehavior.STATELESS)
 public class HttpByteArrayCacheEntrySerializer implements HttpCacheEntrySerializer<byte[]> {
-    public static final HttpByteArrayCacheEntrySerializer INSTANCE = new HttpByteArrayCacheEntrySerializer();
+
+    private static final Logger LOG = LoggerFactory.getLogger(HttpByteArrayCacheEntrySerializer.class);
+
+    /**
+     * The default buffer size used for I/O operations, set to 8192 bytes.
+     */
+    private static final int DEFAULT_BUFFER_SIZE = 8192;
+
+    /**
+     * Singleton instance of this class.
+     */
+    public static final HttpByteArrayCacheEntrySerializer INSTANCE = new HttpByteArrayCacheEntrySerializer(DEFAULT_BUFFER_SIZE);
+
 
     private static final String SC_CACHE_ENTRY_PREFIX = "hc-";

Review Comment:
   @arturobernalg I suggest we use recommended naming convention and prefix our custom headers with `x-`:
   `SC_CACHE_ENTRY_PREFIX = "x-hc-";`



##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpByteArrayCacheEntrySerializer.java:
##########
@@ -81,14 +115,65 @@ public class HttpByteArrayCacheEntrySerializer implements HttpCacheEntrySerializ
     private static final String SC_HEADER_NAME_NO_CONTENT = SC_CACHE_ENTRY_PREFIX + "no-content";
     private static final String SC_HEADER_NAME_VARIANT_MAP_KEY = SC_CACHE_ENTRY_PREFIX + "varmap-key";
     private static final String SC_HEADER_NAME_VARIANT_MAP_VALUE = SC_CACHE_ENTRY_PREFIX + "varmap-val";
-
     private static final String SC_CACHE_ENTRY_PRESERVE_PREFIX = SC_CACHE_ENTRY_PREFIX + "esc-";
 
-    private static final int BUFFER_SIZE = 8192;
 
+    /**
+     * The generator used to generate cached HTTP responses.
+     */
+    private final CachedHttpResponseGenerator cachedHttpResponseGenerator;
+
+    /**
+     * The size of the buffer used for reading/writing data.
+     */
+    private final int bufferSize;
+
+    /**
+     * Use the default, ASCII-only encoder for HTTP protocol and header values.
+     * It's the only thing that's widely used, and it's not worth it to support anything else.
+     */
+    private final SessionOutputBuffer outputBuffer;

Review Comment:
   @arturobernalg Careful here. The class is expected to be stateless by the contract, but `SessionOutputBuffer` is not. It can not be an instance variable! You need to make it local.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org