fix(sarif): QD-8410: Fix ignore properties during deserialization

sarif/src/main/java/com/jetbrains/qodana/sarif/SarifUtil.java

@@ -23,10 +23,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.time.Instant;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
 
 public class SarifUtil {
     private SarifUtil() {
@@ -41,17 +38,14 @@ public static SarifReport readReport(Reader reader, boolean readResults) {
     }
 
     public static SarifReport readReport(Reader reader, boolean readResults, Iterable<String> skippedProperties) {
-        GsonBuilder gsonBuilder = createGsonBuilder();
+        GsonBuilder gsonBuilder = createGsonBuilder(skippedProperties);
         Iterator<String> skippedPropertiesIterator = skippedProperties.iterator();
         if (!readResults || skippedPropertiesIterator.hasNext()) {
             gsonBuilder.registerTypeAdapterFactory(StreamJsonRunsListTypeAdapter.makeFactory());
         }
         if (!readResults) {
             gsonBuilder.addDeserializationExclusionStrategy(StreamingFieldsExclusionStrategy.results());
         }
-        skippedPropertiesIterator.forEachRemaining(property ->
-                gsonBuilder.addDeserializationExclusionStrategy(StreamingFieldsExclusionStrategy.property(property))
-        );
         return gsonBuilder.create().fromJson(reader, SarifReport.class);
     }
 
@@ -103,11 +97,7 @@ public static Gson createGson() {
     }
 
     public static GsonBuilder createGsonBuilder() {
-        return new GsonBuilder()
-                .setPrettyPrinting()
-                .disableHtmlEscaping()
-                .registerTypeAdapter(PropertyBag.class, new PropertyBag.PropertyBagTypeAdapter().nullSafe())
-                .registerTypeAdapter(Instant.class, new IsoInstantTypeAdapter().nullSafe());
+        return createGsonBuilder(Collections.emptyList());
     }
 
     /**
@@ -128,4 +118,14 @@ public static Iterator<Result> lazyReadResults(Reader reader, int runIndexInRepo
     public static Iterator<IndexedResult> lazyReadIndexedResults(Reader reader) {
         return new IndexedResultIterator(reader);
     }
+
+    private static GsonBuilder createGsonBuilder(Iterable<String> ignoreKeys) {
+        Collection<String> ignoreKeysArray = new ArrayList<>();
+        ignoreKeys.iterator().forEachRemaining(ignoreKeysArray::add);
+        return new GsonBuilder()
+                .setPrettyPrinting()
+                .disableHtmlEscaping()
+                .registerTypeAdapterFactory(new PropertyBag.PropertyBagTypeAdapterFactory(ignoreKeysArray))
+                .registerTypeAdapter(Instant.class, new IsoInstantTypeAdapter().nullSafe());
+    }
 }

sarif/src/main/java/com/jetbrains/qodana/sarif/model/PropertyBag.java

@@ -1,11 +1,12 @@
 package com.jetbrains.qodana.sarif.model;
 
-import com.google.gson.Gson;
-import com.google.gson.GsonBuilder;
-import com.google.gson.TypeAdapter;
+import com.google.gson.*;
+import com.google.gson.reflect.TypeToken;
 import com.google.gson.stream.JsonReader;
+import com.google.gson.stream.JsonToken;
 import com.google.gson.stream.JsonWriter;
 
+import java.io.IOException;
 import java.util.*;
 
 
@@ -102,28 +103,94 @@ public int hashCode() {
         return Objects.hash(properties, tags);
     }
 
+    public static class PropertyBagTypeAdapterFactory implements TypeAdapterFactory {
 
-    public static class PropertyBagTypeAdapter extends TypeAdapter<PropertyBag> {
-        private static final Gson embedded = new GsonBuilder().setPrettyPrinting().disableHtmlEscaping().create();
+        private final Set<String> ignoreKeys;
 
-        public void write(JsonWriter out, PropertyBag bag) {
-            HashMap<String, Object> toSerialize = new HashMap<>(bag);
-            if (!bag.tags.isEmpty()) {
-                toSerialize.put(TAGS_KEY, bag.getTags());
+        public PropertyBagTypeAdapterFactory(Collection<String> ignoreKeys) {
+            this.ignoreKeys = new HashSet<>(ignoreKeys);
+        }
+
+        @Override
+        @SuppressWarnings("unchecked")
+        public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> typeToken) {
+            Class<? super T> rawType = typeToken.getRawType();
+            if (!PropertyBag.class.equals(rawType)) {
+                return null;
+            } else {
+                return (TypeAdapter<T>) new PropertyBag.PropertyBagTypeAdapterFactory.PropertyBagTypeAdapter(ignoreKeys, gson);
             }
-            embedded.toJson(toSerialize, Map.class, out);
         }
 
-        public PropertyBag read(JsonReader reader) {
-            PropertyBag result = new PropertyBag();
-            Map<String, Object> map = embedded.fromJson(reader, Map.class);
-            Object tags = map.remove(TAGS_KEY);
-            result.putAll(map);
-            if (tags instanceof List) {
-                //noinspection unchecked
-                result.tags.addAll((Collection<? extends String>) tags);
+        public static class PropertyBagTypeAdapter extends TypeAdapter<PropertyBag> {
+            private final Gson gson;
+            private final Set<String> ignoreKeys;
+
+            public PropertyBagTypeAdapter(Set<String> ignoreKeys, Gson gson) {
+                this.ignoreKeys = ignoreKeys;
+                this.gson = gson;
+            }
+
+            public void write(JsonWriter out, PropertyBag bag) throws IOException {
+                if (bag == null) {
+                    out.nullValue();
+                    return;
+                }
+                HashMap<String, Object> toSerialize = new HashMap<>(bag);
+
+                if (!bag.tags.isEmpty()) {
+                    toSerialize.put(TAGS_KEY, bag.getTags());
+                }
+
+                TypeAdapter<Object> objectTypeAdapter = gson.getAdapter(Object.class);
+
+                out.beginObject();
+
+                for (Map.Entry<String, Object> entry : toSerialize.entrySet()) {
+                    if (!ignoreKeys.contains(entry.getKey())) {
+                        out.name(String.valueOf(entry.getKey()));
+                        objectTypeAdapter.write(out, entry.getValue());
+                    }
+                }
+
+                out.endObject();
+            }
+
+            public PropertyBag read(JsonReader in) throws IOException {
+                JsonToken peek = in.peek();
+                if (peek == JsonToken.NULL) {
+                    in.nextNull();
+                    return null;
+                }
+                PropertyBag result = new PropertyBag();
+                Map<String, Object> serializedMap = new HashMap<>();
+                TypeAdapter<Object> objectTypeAdapter = gson.getAdapter(Object.class);
+                in.beginObject();
+
+                while (in.hasNext()) {
+                    String key = in.nextName();
+                    if (!ignoreKeys.contains(key)) {
+                        Object value = objectTypeAdapter.read(in);
+                        Object replaced = serializedMap.put(key, value);
+                        if (replaced != null) {
+                            throw new JsonSyntaxException("Duplicate key: " + key);
+                        }
+                    } else {
+                        in.skipValue();
+                    }
+                }
+
+                in.endObject();
+
+                Object tags = serializedMap.remove(TAGS_KEY);
+                result.putAll(serializedMap);
+                if (tags instanceof List) {
+                    //noinspection unchecked
+                    result.tags.addAll((Collection<? extends String>) tags);
+                }
+                return result;
             }
-            return result;
         }
     }
+
 }

sarif/src/test/java/com/jetbrains/qodana/sarif/DeserializeTest.java

@@ -1,16 +1,21 @@
 package com.jetbrains.qodana.sarif;
 
 import com.jetbrains.qodana.sarif.model.*;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 import java.io.IOException;
+import java.io.Reader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.time.LocalDate;
 import java.time.OffsetDateTime;
 import java.time.ZoneOffset;
 import java.time.temporal.ChronoUnit;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -77,6 +82,17 @@ public void testDateFormat() throws IOException {
         assertEquals(expect.toInstant(), invocations.get(1).getEndTimeUtc());
     }
 
+    @Test
+    public void testIgnoreProperties() throws IOException {
+        Path target = Paths.get("src/test/resources/testData/serializeTest/real.sarif.json");
+        try (Reader reader = Files.newBufferedReader(target, StandardCharsets.UTF_8)) {
+            SarifReport report = SarifUtil.readReport(reader, false, Collections.singletonList("qodana.sanity.results"));
+            Assertions.assertFalse(report.getRuns().get(0).getProperties().containsKey("qodana.sanity.results"));
+            Assertions.assertTrue(report.getRuns().get(0).getProperties().containsKey("qodana.promo.results"));
+            Assertions.assertTrue(report.getRuns().get(0).getProperties().containsKey("deviceId"));
+        }
+    }
+
 
     private void doTest(Path targetJsonPath, SarifReport expected) throws IOException {
         SarifReport sarifReport = SarifUtil.readReport(targetJsonPath);

sarif/src/test/java/com/jetbrains/qodana/sarif/StreamParseCorrectnessTest.java

@@ -1,18 +1,24 @@
 package com.jetbrains.qodana.sarif;
 
+import com.google.gson.Gson;
+import com.jetbrains.qodana.sarif.model.PropertyBag;
 import com.jetbrains.qodana.sarif.model.Result;
+import com.jetbrains.qodana.sarif.model.Run;
 import com.jetbrains.qodana.sarif.model.SarifReport;
 import com.jetbrains.qodana.sarif.model.streaming.IndexedResult;
 import com.jetbrains.qodana.sarif.model.streaming.ResultLocation;
 import org.junit.jupiter.api.Test;
 
 import java.io.IOException;
 import java.io.Reader;
+import java.util.Collections;
 import java.util.Iterator;
+import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.*;
 
 public class StreamParseCorrectnessTest extends StreamParseTest {
+    private static final Gson gson = new Gson();
     @Test
     public void testStreamParseMakeSameAsSimpleParseReadingResultsOneByOne() {
         forEachInput(inputPath -> {
@@ -24,13 +30,17 @@ public void testStreamParseMakeSameAsSimpleParseReadingResultsOneByOne() {
                 new StreamingSarifReport(actualReport, () -> makeReader(inputPath)),
                 (expected, actual, index) -> {
                     Iterator<Result> expectedResults = expected.getResults().iterator();
-                    Iterator<Result> actualResults = SarifUtil.lazyReadResults(actual.getReader(), index);
-                    while (expectedResults.hasNext()) {
-                        assertTrue(actualResults.hasNext());
-                        assertEquals(expectedResults.next(), actualResults.next());
-                    }
-                    assertEquals(expected.withResults(null), actual.getRun().withResults(null));
-                    assertFalse(actualResults.hasNext());
+                    actual.getReader(reader -> {
+                        Iterator<Result> actualResults = SarifUtil.lazyReadResults(reader, index);
+                        while (expectedResults.hasNext()) {
+                            assertTrue(actualResults.hasNext());
+                            assertEquals(expectedResults.next(), actualResults.next());
+                        }
+                        assertEquals(expected.withResults(null).withProperties(null), actual.getRun().withResults(null).withProperties(null));
+                        assertFalse(actualResults.hasNext());
+
+                    });
+                    assertRunPropertyBag(expected, actual, index);
                 }
             );
         });
@@ -48,19 +58,22 @@ public void testStreamParseMakeSameAsSimpleParseReadingResultsTogether() {
                 expectedReport,
                 new StreamingSarifReport(actualReport, () -> makeReader(inputPath)),
                 (expected, actual, index) -> {
-                    Iterator<Result> expectedResults = expected.getResults().iterator();
-                    Iterator<IndexedResult> actualResults = SarifUtil.lazyReadIndexedResults(actual.getReader());
-                    while (expectedResults.hasNext()) {
-                        IndexedResult indexedResult;
-                        do {
-                            assertTrue(actualResults.hasNext());
-                            indexedResult = actualResults.next();
-                        } while (indexedResult.getIndex() != index);
-                        assertEquals((int)index, indexedResult.getIndex());
-                        assertEquals(expectedResults.next(), indexedResult.getResult());
-                        anyLeftAtLastRun[0] = actualResults.hasNext();
-                    }
-                    assertEquals(expected.withResults(null), actual.getRun().withResults(null));
+                    actual.getReader(reader -> {
+                        Iterator<Result> expectedResults = expected.getResults().iterator();
+                        Iterator<IndexedResult> actualResults = SarifUtil.lazyReadIndexedResults(reader);
+                        while (expectedResults.hasNext()) {
+                            IndexedResult indexedResult;
+                            do {
+                                assertTrue(actualResults.hasNext());
+                                indexedResult = actualResults.next();
+                            } while (indexedResult.getIndex() != index);
+                            assertEquals((int) index, indexedResult.getIndex());
+                            assertEquals(expectedResults.next(), indexedResult.getResult());
+                            anyLeftAtLastRun[0] = actualResults.hasNext();
+                        }
+                        assertEquals(expected.withResults(null).withProperties(null), actual.getRun().withResults(null).withProperties(null));
+                    });
+                    assertRunPropertyBag(expected, actual, index);
                 }
             );
             assertFalse(anyLeftAtLastRun[0]);
@@ -87,4 +100,27 @@ public void testStreamParseReadsSanityCorrectly() throws IOException {
             assertFalse(actualResults.hasNext());
         }
     }
+
+    private static void assertRunPropertyBag(Run expected, StreamingRun actual, Integer runIndex) throws IOException {
+        if (expected.getProperties() != null) {
+            actual.getReader(reader -> {
+                Iterator<Object> expectedSanityResults = ((List<Object>) expected.getProperties().getOrDefault(
+                        "qodana.sanity.results",
+                        Collections.emptyList()
+                )).iterator();
+                Iterator<Result> actualSanityResults = SarifUtil.lazyReadResultsFromLocation(
+                        reader,
+                        new ResultLocation.InProperties(runIndex, "qodana.sanity.results")
+                );
+                while (expectedSanityResults.hasNext()) {
+                    assertTrue(actualSanityResults.hasNext());
+                    Result expectedSanityResult = gson.fromJson(gson.toJson(expectedSanityResults.next()), Result.class);
+                    assertEquals(expectedSanityResult, actualSanityResults.next());
+                }
+                PropertyBag expectedProperties = expected.getProperties();
+                expectedProperties.remove("qodana.sanity.results");
+                assertEquals(expectedProperties, actual.getRun().getProperties());
+            });
+        }
+    }
 }

sarif/src/test/java/com/jetbrains/qodana/sarif/StreamParseTest.java

@@ -60,13 +60,11 @@ protected void assertEqualContents(
         int currentIndex = 0;
         while (expectedRuns.hasNext()) {
             assertTrue(actualRuns.hasNext());
-            try (Reader reader = actual.getMakeReader().get()) {
-                assertEqualRunContents.accept(
-                    expectedRuns.next(),
-                    new StreamingRun(actualRuns.next(), reader),
-                    currentIndex
-                );
-            }
+            assertEqualRunContents.accept(
+                expectedRuns.next(),
+                new StreamingRun(actualRuns.next(), actual.makeReader),
+                currentIndex
+            );
             currentIndex++;
         }
         assertEquals(expected.withRuns(null), actual.getSarifReport().withRuns(null));
@@ -92,7 +90,7 @@ protected Reader makeReader(String path) throws FileNotFoundException {
 
     @FunctionalInterface
     protected interface TriConsumer<A, B, C> {
-        void accept(A a, B b, C c);
+        void accept(A a, B b, C c) throws IOException;
     }
 
     protected static class StreamingSarifReport {
@@ -115,19 +113,21 @@ public ExceptionSupplier<Reader> getMakeReader() {
 
     protected static class StreamingRun {
         private final Run run;
-        private final Reader reader;
+        private final ExceptionSupplier<Reader> makeReader;
 
-        public StreamingRun(Run run, Reader reader) {
+        public StreamingRun(Run run,  ExceptionSupplier<Reader> makeReader) {
             this.run = run;
-            this.reader = reader;
+            this.makeReader = makeReader;
         }
 
         public Run getRun() {
             return run;
         }
 
-        public Reader getReader() {
-            return reader;
+        public void getReader(Consumer<Reader> withReader) throws IOException {
+            try (Reader reader = makeReader.get()) {
+                withReader.accept(reader);
+            }
         }
     }
 }