IPROTO-334 Don't throw NPE when proto.lock contains a message with no…

… fields * Also introduce a ProtoLock.compatibilityCheck() method
infinispan · May 15, 2024 · ec2f1a6 · ec2f1a6
1 parent 8eca1bc
commit ec2f1a6
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 20 deletions.
diff --git a/core/src/main/java/org/infinispan/protostream/FileDescriptorSource.java b/core/src/main/java/org/infinispan/protostream/FileDescriptorSource.java
@@ -160,6 +160,11 @@ public FileDescriptorSource addProtoFile(String name, File protoFile) throws IOE
       return this;
    }
 
+
+   public FileDescriptorSource addProtoFile(File protoFile) throws IOException {
+      return addProtoFile(protoFile.getName(), protoFile);
+   }
+
    public static FileDescriptorSource fromResources(ClassLoader userClassLoader, String... classPathResources) throws IOException {
       return new FileDescriptorSource().addProtoFiles(userClassLoader, classPathResources);
    }

diff --git a/core/src/main/java/org/infinispan/protostream/descriptors/ProtoLock.java b/core/src/main/java/org/infinispan/protostream/descriptors/ProtoLock.java
@@ -12,6 +12,8 @@
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
+import org.infinispan.protostream.impl.Log;
+
 import com.fasterxml.jackson.core.JsonFactory;
 import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.core.JsonParser;
@@ -35,6 +37,25 @@ public Map<String, FileDescriptor> descriptors() {
       return descriptors;
    }
 
+   /**
+    * Checks for compatibility between all the descriptors in this ProtoLock instance against those in the supplied one
+    *
+    * @param that
+    * @param strict
+    */
+   public void checkCompatibility(ProtoLock that, boolean strict) {
+      List<String> errors = new ArrayList<>();
+      for (Map.Entry<String, FileDescriptor> descriptor : descriptors.entrySet()) {
+         FileDescriptor d2 = that.descriptors.get(descriptor.getKey());
+         if (d2 != null) {
+            descriptor.getValue().checkCompatibility(d2, strict, errors);
+         }
+      }
+      if (!errors.isEmpty()) {
+         throw Log.LOG.incompatibleSchemaChanges(String.join("\n", errors));
+      }
+   }
+
    public static ProtoLock readLockFile(InputStream is) throws IOException {
       JsonFactory jsonFactory = new JsonFactory();
       jsonFactory.setCodec(new ObjectMapper());
@@ -108,26 +129,28 @@ private static Map<String, Descriptor.Builder> readMessages(JsonNode json, Strin
             JsonNode message = messages.get(m);
             String name = message.get("name").asText();
             Descriptor.Builder mb = messageBuilders.computeIfAbsent(name, n -> new Descriptor.Builder().withName(n).withFullName(namePrefix + n));
-            ArrayNode fields = (ArrayNode) message.get("fields");
-            Map<String, OneOfDescriptor.Builder> oneOfBuilders = new HashMap<>();
-            for (int f = 0; f < fields.size(); f++) {
-               JsonNode field = fields.get(f);
-               FieldDescriptor.Builder fb = new FieldDescriptor.Builder();
-               fb.withName(field.get("name").asText());
-               fb.withNumber(field.get("id").asInt());
-               fb.withTypeName(field.get("type").asText());
-               if (field.has("is_repeated")) {
-                  fb.withLabel(Label.REPEATED);
-               }
-               if (field.has("optional")) {
-                  fb.withLabel(Label.OPTIONAL);
-               }
-               readOptions(field, fb);
-               if (message.has("oneof_parent")) {
-                  String oneOf = message.get("oneof_parent").asText();
-                  oneOfBuilders.computeIfAbsent(oneOf, n -> new OneOfDescriptor.Builder().withName(n)).addField(fb);
-               } else {
-                  mb.addField(fb);
+            if (message.has("fields")) {
+               ArrayNode fields = (ArrayNode) message.get("fields");
+               Map<String, OneOfDescriptor.Builder> oneOfBuilders = new HashMap<>();
+               for (int f = 0; f < fields.size(); f++) {
+                  JsonNode field = fields.get(f);
+                  FieldDescriptor.Builder fb = new FieldDescriptor.Builder();
+                  fb.withName(field.get("name").asText());
+                  fb.withNumber(field.get("id").asInt());
+                  fb.withTypeName(field.get("type").asText());
+                  if (field.has("is_repeated")) {
+                     fb.withLabel(Label.REPEATED);
+                  }
+                  if (field.has("optional")) {
+                     fb.withLabel(Label.OPTIONAL);
+                  }
+                  readOptions(field, fb);
+                  if (message.has("oneof_parent")) {
+                     String oneOf = message.get("oneof_parent").asText();
+                     oneOfBuilders.computeIfAbsent(oneOf, n -> new OneOfDescriptor.Builder().withName(n)).addField(fb);
+                  } else {
+                     mb.addField(fb);
+                  }
                }
             }
             if (message.has("maps")) {