Merge pull request #117 from ehsavoie/memory

fix:Serializing subreports to JSON only once even if the global report is decorated.

api/src/main/java/com/redhat/insights/AbstractTopLevelReportBase.java

@@ -4,6 +4,7 @@
 import com.fasterxml.jackson.databind.JsonSerializer;
 import com.redhat.insights.config.InsightsConfiguration;
 import com.redhat.insights.logging.InsightsLogger;
+import java.io.IOException;
 import java.lang.management.*;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
@@ -80,6 +81,7 @@ public abstract class AbstractTopLevelReportBase implements InsightsReport {
   private final JsonSerializer<InsightsReport> serializer;
   private final InsightsLogger logger;
   private final InsightsConfiguration config;
+  private byte[] subReport;
 
   // Can't be set properly until after report has been generated
   private String idHash = "";
@@ -204,6 +206,19 @@ public void generateReport(Filtering masking) {
 
   protected abstract Package[] getPackages();
 
+  @Override
+  public byte[] getSubModulesReport() {
+    if (subReport == null) {
+      subReport = InsightsReport.super.getSubModulesReport();
+    }
+    return subReport;
+  }
+
+  @Override
+  public void close() throws IOException {
+    subReport = null;
+  }
+
   /**
    * This is the top-level name - implementors that may have to care about multi-tenancy should
    * handle that in product-specific code.

api/src/main/java/com/redhat/insights/InsightsReport.java

@@ -3,21 +3,21 @@
 
 import static com.redhat.insights.InsightsErrorCode.ERROR_SERIALIZING_TO_JSON;
 
-import com.fasterxml.jackson.core.Version;
+import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.databind.JsonSerializer;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.module.SimpleModule;
-import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+import java.io.ByteArrayOutputStream;
+import java.io.Closeable;
 import java.io.IOException;
-import java.io.StringWriter;
 import java.util.Map;
 
 /**
  * Top-level insights report.
  *
  * @see InsightsSubreport for runtime-specific sub-reports
  */
-public interface InsightsReport {
+public interface InsightsReport extends Closeable {
+  static final byte[] EMPTY_BYTE_ARRAY = new byte[0];
 
   Map<String, InsightsSubreport> getSubreports();
 
@@ -41,30 +41,68 @@ public interface InsightsReport {
 
   void decorate(String key, String value);
 
+  /**
+   * Serializes this report to JSON for transport.
+   *
+   * @return JSON serialized report as a stream of UTF-8 encoded bytes.
+   */
+  default byte[] serializeRaw() {
+    ObjectMapper mapper = ObjectMappers.createFor(this);
+
+    try {
+      return mapper.writerWithDefaultPrettyPrinter().writeValueAsBytes(this);
+    } catch (IOException e) {
+      throw new InsightsException(ERROR_SERIALIZING_TO_JSON, "JSON serialization exception", e);
+    }
+  }
+
   /**
    * Serializes this report to JSON for transport
    *
-   * @return JSON serialized report
+   * @return JSON serialized report.
+   * @deprecated use #serializeRaw instead.
    */
+  @Deprecated
   default String serialize() {
-    ObjectMapper mapper = new ObjectMapper();
-    mapper.registerModule(new JavaTimeModule());
-
-    SimpleModule simpleModule =
-        new SimpleModule(
-            "SimpleModule", new Version(1, 0, 0, null, "com.redhat.insights", "runtimes-java"));
-    simpleModule.addSerializer(InsightsReport.class, getSerializer());
-    for (InsightsSubreport subreport : getSubreports().values()) {
-      simpleModule.addSerializer(subreport.getClass(), subreport.getSerializer());
-    }
-    mapper.registerModule(simpleModule);
+    ObjectMapper mapper = ObjectMappers.createFor(this);
 
-    StringWriter writer = new StringWriter();
     try {
-      mapper.writerWithDefaultPrettyPrinter().writeValue(writer, this);
+      return mapper.writerWithDefaultPrettyPrinter().writeValueAsString(this);
+    } catch (IOException e) {
+      throw new InsightsException(ERROR_SERIALIZING_TO_JSON, "JSON serialization exception", e);
+    }
+  }
+
+  /**
+   * Serializes this report to JSON for transport
+   *
+   * @return JSON serialized report
+   */
+  default byte[] getSubModulesReport() {
+    ObjectMapper mapper = ObjectMappers.createFor(this);
+
+    try (ByteArrayOutputStream out = new ByteArrayOutputStream();
+        JsonGenerator generator = mapper.writerWithDefaultPrettyPrinter().createGenerator(out)) {
+      generator.writeStartObject();
+      for (Map.Entry<String, InsightsSubreport> entry : getSubreports().entrySet()) {
+        generator.writeObjectField(entry.getKey(), entry.getValue());
+      }
+      generator.writeEndObject();
+      generator.flush();
+      byte[] report = out.toByteArray();
+      // The subreports are in an array and are to be added to the report array.
+      // Thus we are removing the {} enclosing the subreports array and adding a ',' to append to
+      // the existing array.
+      boolean notEmptyArray = report.length > 3;
+      if (notEmptyArray) {
+        byte[] finalReport = new byte[report.length - 1];
+        finalReport[0] = ',';
+        System.arraycopy(report, 1, finalReport, 1, report.length - 2);
+        return finalReport;
+      }
+      return EMPTY_BYTE_ARRAY;
     } catch (IOException e) {
       throw new InsightsException(ERROR_SERIALIZING_TO_JSON, "JSON serialization exception", e);
     }
-    return writer.toString();
   }
 }

api/src/main/java/com/redhat/insights/InsightsReportController.java

@@ -113,7 +113,15 @@ public void generate() {
             InsightsHttpClient httpClient = httpClientSupplier.get();
             if (httpClient.isReadyToSend()) {
               generateConnectReport();
-              httpClient.sendInsightsReport(getIdHash() + "_connect", report);
+              try {
+                httpClient.sendInsightsReport(getIdHash() + "_connect", report);
+              } finally {
+                try {
+                  report.close();
+                } catch (IOException ioex) {
+                  // Nothing to be done there
+                }
+              }
             } else {
               logger.debug("Insights is not configured to send: " + configuration);
             }
@@ -163,7 +171,7 @@ public boolean isShutdown() {
   void generateAndSetReportIdHash() {
     try {
       if (!idHashHolder.isDone()) {
-        String hash = computeSha512(gzipReport(report.serialize()));
+        String hash = computeSha512(gzipReport(report.serializeRaw()));
         idHashHolder.complete(hash);
         report.setIdHash(hash);
       }

api/src/main/java/com/redhat/insights/InsightsReportSerializer.java

@@ -5,7 +5,7 @@
 import com.fasterxml.jackson.databind.JsonSerializer;
 import com.fasterxml.jackson.databind.SerializerProvider;
 import java.io.IOException;
-import java.util.Map;
+import java.nio.charset.StandardCharsets;
 
 /** JSON serializer for an {@link InsightsReport} object. */
 public class InsightsReportSerializer extends JsonSerializer<InsightsReport> {
@@ -23,9 +23,8 @@ public void serialize(
     if (!insightsReport.getBasic().isEmpty()) {
       generator.writeObjectField("basic", insightsReport.getBasic());
     }
-    for (Map.Entry<String, InsightsSubreport> entry : insightsReport.getSubreports().entrySet()) {
-      generator.writeObjectField(entry.getKey(), entry.getValue());
-    }
+    byte[] subReport = insightsReport.getSubModulesReport();
+    generator.writeRaw(new String(subReport, StandardCharsets.UTF_8));
     generator.writeEndObject();
     generator.flush();
   }

api/src/main/java/com/redhat/insights/ObjectMappers.java

@@ -0,0 +1,29 @@
+/* Copyright (C) Red Hat 2023 */
+package com.redhat.insights;
+
+import com.fasterxml.jackson.core.Version;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.module.SimpleModule;
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+
+/**
+ * And util class to provide a factory method for ObjectMapper used within {@link
+ * InsightsReport#serialize} methods
+ */
+class ObjectMappers {
+
+  public static ObjectMapper createFor(InsightsReport insightsReport) {
+    ObjectMapper mapper = new ObjectMapper();
+    mapper.registerModule(new JavaTimeModule());
+
+    SimpleModule simpleModule =
+        new SimpleModule(
+            "SimpleModule", new Version(1, 0, 0, null, "com.redhat.insights", "runtimes-java"));
+    simpleModule.addSerializer(InsightsReport.class, insightsReport.getSerializer());
+    for (InsightsSubreport subreport : insightsReport.getSubreports().values()) {
+      simpleModule.addSerializer(subreport.getClass(), subreport.getSerializer());
+    }
+    mapper.registerModule(simpleModule);
+    return mapper;
+  }
+}

api/src/main/java/com/redhat/insights/UpdateReportImpl.java

@@ -5,6 +5,7 @@
 import com.redhat.insights.jars.JarInfo;
 import com.redhat.insights.jars.JarInfoSubreport;
 import com.redhat.insights.logging.InsightsLogger;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -77,4 +78,7 @@ public void decorate(String key, String value) {
     logger.debug(
         String.format("Attempt to add %s => %s to an update report. Ignored.", key, value));
   }
+
+  @Override
+  public void close() throws IOException {}
 }

api/src/main/java/com/redhat/insights/http/InsightsFileWritingClient.java

@@ -10,7 +10,6 @@
 import com.redhat.insights.logging.InsightsLogger;
 import java.io.File;
 import java.io.IOException;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -46,15 +45,13 @@ public void decorate(InsightsReport report) {
   @Override
   public void sendInsightsReport(String filename, InsightsReport report) {
     decorate(report);
-    String reportJson = report.serialize();
-    //    logger.debug(reportJson);
 
     // Can't reuse upload path - as this may be called as part of fallback
     Path p = Paths.get(config.getArchiveUploadDir(), filename + ".json");
     try {
       Files.write(
           p,
-          reportJson.getBytes(StandardCharsets.UTF_8),
+          report.serializeRaw(),
           StandardOpenOption.WRITE,
           StandardOpenOption.CREATE,
           StandardOpenOption.TRUNCATE_EXISTING);

api/src/main/java/com/redhat/insights/http/InsightsHttpClient.java

@@ -46,21 +46,30 @@ public interface InsightsHttpClient {
   default boolean isReadyToSend() {
     return true;
   }
+
   /**
    * Static gzip helper method
    *
    * @param report
    * @return gzipped bytes
+   * @deprecated use #gzipReport(final byte[] report)
    */
   static byte[] gzipReport(final String report) {
-    try (final ByteArrayOutputStream baos = new ByteArrayOutputStream(report.length())) {
-      final byte[] buffy = report.getBytes(StandardCharsets.UTF_8);
+    return gzipReport(report.getBytes(StandardCharsets.UTF_8));
+  }
 
+  /**
+   * Static gzip helper method
+   *
+   * @param report
+   * @return gzipped bytes
+   */
+  static byte[] gzipReport(final byte[] report) {
+    try (final ByteArrayOutputStream baos = new ByteArrayOutputStream(report.length)) {
       final GZIPOutputStream gzip = new GZIPOutputStream(baos);
-      gzip.write(buffy, 0, buffy.length);
+      gzip.write(report);
       // An explicit close is necessary before we call toByteArray()
       gzip.close();
-
       return baos.toByteArray();
     } catch (IOException iox) {
       throw new InsightsException(ERROR_GZIP_FILE, "Failed to GZIP report: " + report, iox);

api/src/main/java/com/redhat/insights/jars/JarInfoSubreport.java

@@ -11,7 +11,6 @@
 import com.redhat.insights.InsightsSubreport;
 import com.redhat.insights.logging.InsightsLogger;
 import java.io.IOException;
-import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -58,12 +57,10 @@ public String serializeReport() {
     simpleModule.addSerializer(getClass(), getSerializer());
     mapper.registerModule(simpleModule);
 
-    StringWriter writer = new StringWriter();
     try {
-      mapper.writerWithDefaultPrettyPrinter().writeValue(writer, this);
+      return mapper.writerWithDefaultPrettyPrinter().writeValueAsString(this);
     } catch (IOException e) {
       throw new InsightsException(ERROR_SERIALIZING_TO_JSON, "JSON serialization exception", e);
     }
-    return writer.toString();
   }
 }

api/src/test/java/com/redhat/insights/InsightsReportControllerSimpleThreadingTest.java

@@ -13,6 +13,7 @@
 import com.redhat.insights.doubles.NoopInsightsLogger;
 import com.redhat.insights.logging.InsightsLogger;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.security.NoSuchAlgorithmException;
 import java.time.Duration;
 import org.junit.jupiter.api.Test;
@@ -50,7 +51,7 @@ public void testJarFingerprinting() throws NoSuchAlgorithmException, IOException
     report.generateReport(Filtering.DEFAULT);
 
     // First, compute the SHA 512 fingerprint without the id hash
-    String initialReportJson = report.serialize();
+    byte[] initialReportJson = report.serializeRaw();
     final byte[] initialGz = gzipReport(initialReportJson);
     final String hash = computeSha512(initialGz);
 
@@ -62,10 +63,12 @@ public void testJarFingerprinting() throws NoSuchAlgorithmException, IOException
     assertEquals(controller.getIdHash(), hash);
 
     // Reserialize with the hash
-    final String reportJson = report.serialize();
+    final byte[] reportJson = report.serializeRaw();
     final byte[] finalGz = gzipReport(reportJson);
 
-    assertNotEquals(initialReportJson, reportJson);
+    assertNotEquals(
+        new String(initialReportJson, StandardCharsets.UTF_8),
+        new String(reportJson, StandardCharsets.UTF_8));
 
     // Final check
     String initialSha512 = computeSha512(initialGz);

api/src/test/java/com/redhat/insights/http/InsightsFileWritingClientTest.java

@@ -12,6 +12,7 @@
 import com.redhat.insights.logging.InsightsLogger;
 import java.io.File;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -38,7 +39,7 @@ public String getArchiveUploadDir() {
     InsightsLogger logger = new NoopInsightsLogger();
     InsightsHttpClient client = new InsightsFileWritingClient(logger, cfg);
     InsightsReport report = mock(InsightsReport.class);
-    when(report.serialize()).thenReturn("foo");
+    when(report.serializeRaw()).thenReturn("foo".getBytes(StandardCharsets.UTF_8));
 
     client.sendInsightsReport("foo", report);
     File[] files = tmpdir.toFile().listFiles();

api/src/test/java/com/redhat/insights/http/InsightsMultiClientTest.java

@@ -13,6 +13,7 @@
 import com.redhat.insights.logging.InsightsLogger;
 import java.io.File;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Arrays;
@@ -54,7 +55,7 @@ public String getArchiveUploadDir() {
     InsightsLogger logger = new NoopInsightsLogger();
 
     InsightsReport report = mock(InsightsReport.class);
-    when(report.serialize()).thenReturn("foo");
+    when(report.serializeRaw()).thenReturn("foo".getBytes(StandardCharsets.UTF_8));
 
     InsightsHttpClient failingClient = mock(InsightsHttpClient.class);
     doThrow(new InsightsException("Failing on purpose"))
@@ -86,7 +87,7 @@ void theyAllFail() {
     InsightsLogger logger = new NoopInsightsLogger();
 
     InsightsReport report = mock(InsightsReport.class);
-    when(report.serialize()).thenReturn("foo");
+    when(report.serializeRaw()).thenReturn("foo".getBytes(StandardCharsets.UTF_8));
 
     InsightsHttpClient failingClient = mock(InsightsHttpClient.class);
     doThrow(new InsightsException("Failing on purpose"))