fix: handling inner nested proto in change detection

core/changedetector/proto_change_detector_service.go

@@ -80,27 +80,59 @@ func appendImpactedDependents(sce *stencilv1beta1.SchemaChangedEvent, key string
 }
 
 func setDirectlyImpactedSchemasAndFields(currentFds, prevFds *descriptor.FileDescriptorSet, sce *stencilv1beta1.SchemaChangedEvent) {
-	packageMessageMap := getPackageMessageMap(prevFds)
+	prevMessageMap := collectAllMessages(prevFds)
 	packageEnumMap := getPackageEnumMap(prevFds)
 	for _, fileDesc := range currentFds.GetFile() {
+		pkg := fileDesc.GetPackage()
 		for _, newMessageDesc := range fileDesc.GetMessageType() {
-			messageName := fileDesc.GetPackage() + "." + newMessageDesc.GetName()
-			oldMessageDesc := getMessageDescriptor(packageMessageMap, fileDesc.GetPackage(), newMessageDesc.GetName())
-			if oldMessageDesc == nil {
-				sce.UpdatedSchemas = append(sce.UpdatedSchemas, messageName)
-				appendImpactedFields(sce, messageName, GetImpactedMessageFields(oldMessageDesc, newMessageDesc))
-				continue
-			}
-			if !proto.Equal(oldMessageDesc, newMessageDesc) {
-				sce.UpdatedSchemas = append(sce.UpdatedSchemas, messageName)
-				appendImpactedFields(sce, messageName, GetImpactedMessageFields(oldMessageDesc, newMessageDesc))
-			}
-			compareEnumDescInMessageDesc(oldMessageDesc, newMessageDesc, messageName, sce)
+			compareMessageTree(pkg, newMessageDesc, prevMessageMap, sce)
 		}
 		compareEnumDescriptors(fileDesc, packageEnumMap, sce)
 	}
 }
 
+// collectAllMessages returns a flat map of fully-qualified name -> descriptor
+// for ALL messages (top-level and nested) across all files in the descriptor set.
+func collectAllMessages(fds *descriptor.FileDescriptorSet) map[string]*descriptor.DescriptorProto {
+	result := make(map[string]*descriptor.DescriptorProto)
+	for _, fileDesc := range fds.GetFile() {
+		pkg := fileDesc.GetPackage()
+		for _, msgDesc := range fileDesc.GetMessageType() {
+			walkMessageTree(pkg, msgDesc, result)
+		}
+	}
+	return result
+}
+
+// walkMessageTree recursively registers a message and all its nested messages
+// into the result map under their fully-qualified names (e.g. "test.Outer.Inner").
+func walkMessageTree(prefix string, msg *descriptor.DescriptorProto, result map[string]*descriptor.DescriptorProto) {
+	fullName := prefix + "." + msg.GetName()
+	result[fullName] = msg
+	for _, nested := range msg.GetNestedType() {
+		walkMessageTree(fullName, nested, result)
+	}
+}
+
+// compareMessageTree recursively compares a message and all its nested messages,
+// recording any changes into the SchemaChangedEvent.
+func compareMessageTree(prefix string, newMsg *descriptor.DescriptorProto, prevMessageMap map[string]*descriptor.DescriptorProto, sce *stencilv1beta1.SchemaChangedEvent) {
+	fullName := prefix + "." + newMsg.GetName()
+	oldMsg := prevMessageMap[fullName]
+	if oldMsg == nil {
+		sce.UpdatedSchemas = append(sce.UpdatedSchemas, fullName)
+		appendImpactedFields(sce, fullName, GetImpactedMessageFields(oldMsg, newMsg))
+	} else if !proto.Equal(oldMsg, newMsg) {
+		sce.UpdatedSchemas = append(sce.UpdatedSchemas, fullName)
+		appendImpactedFields(sce, fullName, GetImpactedMessageFields(oldMsg, newMsg))
+		compareEnumDescInMessageDesc(oldMsg, newMsg, fullName, sce)
+	}
+	// Always recurse into nested messages so each level is independently evaluated.
+	for _, nested := range newMsg.GetNestedType() {
+		compareMessageTree(fullName, nested, prevMessageMap, sce)
+	}
+}
+
 func compareEnumDescInMessageDesc(oldMessageDesc, newMessageDesc *descriptorpb.DescriptorProto, messageName string, sce *stencilv1beta1.SchemaChangedEvent) {
 	for _, newEnumDesc := range newMessageDesc.GetEnumType() {
 		enumName := messageName + "." + newEnumDesc.GetName()
@@ -147,24 +179,6 @@ func compareEnumDescriptors(fds *descriptorpb.FileDescriptorProto, packageEnumMa
 	}
 }
 
-/*
-packageMessageMap is map having all the messages inside a package
-[com.goto.bookinglog][BookingLogMessage]=BookingLogMessageDescriptor
-*/
-func getPackageMessageMap(fileDescriptorSet *descriptor.FileDescriptorSet) map[string]map[string]*descriptor.DescriptorProto {
-	packageMessageMap := make(map[string]map[string]*descriptor.DescriptorProto)
-	for _, fileDescriptor := range fileDescriptorSet.GetFile() {
-		pkgName := fileDescriptor.GetPackage()
-		if _, ok := packageMessageMap[pkgName]; !ok {
-			packageMessageMap[pkgName] = make(map[string]*descriptor.DescriptorProto)
-		}
-		for _, messageDescriptor := range fileDescriptor.GetMessageType() {
-			packageMessageMap[pkgName][messageDescriptor.GetName()] = messageDescriptor
-		}
-	}
-	return packageMessageMap
-}
-
 /*
 packageEnumMap is map having all the enums inside a package
 [com.goto.bookinglog][ServiceTypeEnum]=ServiceTypeEnumDescriptor
@@ -183,15 +197,6 @@ func getPackageEnumMap(fileDescriptorSet *descriptor.FileDescriptorSet) map[stri
 	return packageEnumMap
 }
 
-func getMessageDescriptor(packageMessageMap map[string]map[string]*descriptor.DescriptorProto, packageName, messageName string) *descriptor.DescriptorProto {
-	if packageMap, found := packageMessageMap[packageName]; found {
-		if descriptor, found := packageMap[messageName]; found {
-			return descriptor
-		}
-	}
-	return nil
-}
-
 func getEnumDescriptor(packageEnumMap map[string]map[string]*descriptor.EnumDescriptorProto, packageName, enumName string) *descriptor.EnumDescriptorProto {
 	if packageMap, found := packageEnumMap[packageName]; found {
 		if descriptor, found := packageMap[enumName]; found {
@@ -213,23 +218,33 @@ func findEnumDescriptorFromMessageDescriptor(messageDescriptor *descriptor.Descr
 func getReverseDependencies(fileDescriptorSet *descriptor.FileDescriptorSet) map[string][]string {
 	reverseDependencies := make(map[string][]string)
 	for _, fileDescriptor := range fileDescriptorSet.GetFile() {
+		pkg := fileDescriptor.GetPackage()
 		for _, messageDescriptor := range fileDescriptor.GetMessageType() {
-			messageName := fileDescriptor.GetPackage() + "." + messageDescriptor.GetName()
-			for _, fieldDescriptor := range messageDescriptor.GetField() {
-				fieldType := fieldDescriptor.GetTypeName()
-				/*Check if the field type is a message (nested message or imported message)
-				Ref:https://cloud.google.com/java/docs/reference/protobuf/latest/com.google.protobuf.DescriptorProtos.FieldDescriptorProto#com_google_protobuf_DescriptorProtos_FieldDescriptorProto_getType__:~:text=The%20type.-,getTypeName(),-public%20String%20getTypeName
-				*/
-				if fieldType != "" && fieldType[0] == '.' {
-					dependentMessage := fieldType[1:]
-					reverseDependencies[dependentMessage] = append(reverseDependencies[dependentMessage], messageName)
-				}
-			}
+			buildReverseDepsFromMessage(pkg, messageDescriptor, reverseDependencies)
 		}
 	}
 	return reverseDependencies
 }
 
+// buildReverseDepsFromMessage recursively registers reverse dependencies for a message
+// and all its nested messages, so that e.g. "test.Outer.Inner" is a valid lookup key.
+func buildReverseDepsFromMessage(prefix string, msg *descriptor.DescriptorProto, reverseDeps map[string][]string) {
+	msgFullName := prefix + "." + msg.GetName()
+	for _, fieldDescriptor := range msg.GetField() {
+		fieldType := fieldDescriptor.GetTypeName()
+		/*Check if the field type is a message (nested message or imported message)
+		Ref:https://cloud.google.com/java/docs/reference/protobuf/latest/com.google.protobuf.DescriptorProtos.FieldDescriptorProto#com_google_protobuf_DescriptorProtos_FieldDescriptorProto_getType__:~:text=The%20type.-,getTypeName(),-public%20String%20getTypeName
+		*/
+		if fieldType != "" && fieldType[0] == '.' {
+			dependentMessage := fieldType[1:]
+			reverseDeps[dependentMessage] = append(reverseDeps[dependentMessage], msgFullName)
+		}
+	}
+	for _, nested := range msg.GetNestedType() {
+		buildReverseDepsFromMessage(msgFullName, nested, reverseDeps)
+	}
+}
+
 func getDependentImpactedSchemas(reverseDependencies map[string][]string, impactedSchema string, depth int32) []string {
 	visitedMessages := make(map[string]bool)
 	var dependentImpactedSchemas []string

core/changedetector/proto_change_detector_service_test.go

@@ -250,6 +250,53 @@ func TestIdentifySchemaChange(t *testing.T) {
 	})
 }
 
+func TestIdentifySchemaChangeNestedProto(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("should detect field change inside nested message and report parent as impacted", func(t *testing.T) {
+		svc, newRelic := getSvc()
+		request.Depth = 10
+		var called bool
+		// v1: Outer.Inner has only `value`. v2: Outer.Inner gains `extra_field`.
+		// Expected: both test.Outer and test.Outer.Inner appear in updated_schemas.
+		// test.Outer.Inner.updated_fields should contain "extra_field".
+		// test.Outer.Inner.impacted_schemas should contain test.Outer (it uses Inner).
+		oldData := getDescriptorData(t, "./testdata/input", true, []string{"schema_with_nested_message_v1.proto"})
+		newData := getDescriptorData(t, "./testdata/input", true, []string{"schema_with_nested_message_v2.proto"})
+		request.OldData = oldData
+		request.NewData = newData
+		newRelic.On("StartGenericSegment", mock.Anything, "Identify Schema Change").Return(func() { called = true })
+		actual, err := svc.IdentifySchemaChange(ctx, request)
+		expected := getSchemaChangeEvent("./testdata/output/sce_nested_message_changed.json")
+		assert.Nil(t, err)
+		newRelic.AssertExpectations(t)
+		assert.True(t, called)
+		assert.NotNil(t, actual)
+		assertSchemaChangeEvent(t, expected, actual)
+	})
+
+	t.Run("should detect AnotherMessage as impacted when it references Outer.Inner which changed", func(t *testing.T) {
+		svc, newRelic := getSvc()
+		request.Depth = -1
+		var called bool
+		// v1: Outer.Inner has only `value`. AnotherMessage uses Outer.Inner directly.
+		// v2: Outer.Inner gains `extra_field`.
+		// Expected: test.AnotherMessage appears in impacted_schemas["test.Outer.Inner"].
+		oldData := getDescriptorData(t, "./testdata/input", true, []string{"schema_with_nested_cross_ref_v1.proto"})
+		newData := getDescriptorData(t, "./testdata/input", true, []string{"schema_with_nested_cross_ref_v2.proto"})
+		request.OldData = oldData
+		request.NewData = newData
+		newRelic.On("StartGenericSegment", mock.Anything, "Identify Schema Change").Return(func() { called = true })
+		actual, err := svc.IdentifySchemaChange(ctx, request)
+		expected := getSchemaChangeEvent("./testdata/output/sce_nested_cross_ref_changed.json")
+		assert.Nil(t, err)
+		newRelic.AssertExpectations(t)
+		assert.True(t, called)
+		assert.NotNil(t, actual)
+		assertSchemaChangeEvent(t, expected, actual)
+	})
+}
+
 func assertSchemaChangeEvent(t *testing.T, expected, actual *stencilv1beta1.SchemaChangedEvent) {
 	t.Helper()
 	assert.Equal(t, expected.NamespaceName, actual.NamespaceName)

core/changedetector/testdata/input/schema_with_nested_cross_ref_v1.proto

@@ -0,0 +1,20 @@
+syntax = "proto3";
+
+package test;
+
+// Inner is defined INSIDE Outer
+message Outer {
+  string id = 1;
+
+  message Inner {
+    string value = 2;
+  }
+
+  Inner inner = 3;
+}
+
+// AnotherMessage uses Outer.Inner directly — cross-referencing a nested message
+message AnotherMessage {
+  Outer.Inner inner_ref = 1;
+}
+

core/changedetector/testdata/input/schema_with_nested_cross_ref_v2.proto

@@ -0,0 +1,21 @@
+syntax = "proto3";
+
+package test;
+
+// Inner gains a new field `extra_field`
+message Outer {
+  string id = 1;
+
+  message Inner {
+    string value = 2;
+    string extra_field = 3;
+  }
+
+  Inner inner = 3;
+}
+
+// AnotherMessage still uses Outer.Inner — should be impacted by Inner's change
+message AnotherMessage {
+  Outer.Inner inner_ref = 1;
+}
+

core/changedetector/testdata/input/schema_with_nested_message_v1.proto

@@ -0,0 +1,14 @@
+syntax = "proto3";
+
+package test;
+
+message Outer {
+  string id = 1;
+
+  message Inner {
+    string value = 2;
+  }
+
+  Inner inner = 3;
+}
+

core/changedetector/testdata/input/schema_with_nested_message_v2.proto

@@ -0,0 +1,15 @@
+syntax = "proto3";
+
+package test;
+
+message Outer {
+  string id = 1;
+
+  message Inner {
+    string value = 2;
+    string extra_field = 3;  // new field added to nested message
+  }
+
+  Inner inner = 3;
+}
+

core/changedetector/testdata/output/sce_nested_cross_ref_changed.json

@@ -0,0 +1,36 @@
+{
+  "namespace_name": "testNamespace",
+  "schema_name": "testSchemaName",
+  "updated_schemas": [
+    "test.Outer",
+    "test.Outer.Inner"
+  ],
+  "updated_fields": {
+    "test.Outer": {},
+    "test.Outer.Inner": {
+      "field_names": [
+        "extra_field"
+      ]
+    }
+  },
+  "impacted_schemas": {
+    "test.Outer": {
+      "schema_names": [
+        "test.Outer"
+      ]
+    },
+    "test.Outer.Inner": {
+      "schema_names": [
+        "test.Outer.Inner",
+        "test.Outer",
+        "test.AnotherMessage"
+      ]
+    }
+  },
+  "version": 1,
+  "metadata": {
+    "source_url": "https://github.com/some-repo",
+    "commit_sha": "some-commit-sha"
+  }
+}
+

core/changedetector/testdata/output/sce_nested_message_changed.json

@@ -0,0 +1,35 @@
+{
+  "namespace_name": "testNamespace",
+  "schema_name": "testSchemaName",
+  "updated_schemas": [
+    "test.Outer",
+    "test.Outer.Inner"
+  ],
+  "updated_fields": {
+    "test.Outer": {},
+    "test.Outer.Inner": {
+      "field_names": [
+        "extra_field"
+      ]
+    }
+  },
+  "impacted_schemas": {
+    "test.Outer": {
+      "schema_names": [
+        "test.Outer"
+      ]
+    },
+    "test.Outer.Inner": {
+      "schema_names": [
+        "test.Outer.Inner",
+        "test.Outer"
+      ]
+    }
+  },
+  "version": 1,
+  "metadata": {
+    "source_url": "https://github.com/some-repo",
+    "commit_sha": "some-commit-sha"
+  }
+}
+

ui/embed.go

@@ -4,5 +4,5 @@ import "embed"
 
 // Assets embed build folder
 //
-//go:embed build/*
+//go:embed all:build
 var Assets embed.FS