From 898d46af9a58409abde912e35ad59b3cec7e8d12 Mon Sep 17 00:00:00 2001
From: wuhongteng <wuhongteng@meituan.com>
Date: Fri, 15 May 2026 17:33:40 +0800
Subject: [PATCH] fix: address code review findings

P0: strip inner-class suffix from namespace before path resolution
P1: support Dao/DAO/Repository.java naming in addition to Mapper.java
Tests: CDATA extraction, XML comment exclusion, namespace fallback
---
 .../src/core/ingestion/pipeline-phases/orm.ts |  9 +++-
 .../src/main/resources/mapper/CdataMapper.xml | 20 +++++++++
 .../test/integration/orm-dataflow.test.ts     | 41 +++++++++++++++++++
 3 files changed, 68 insertions(+), 2 deletions(-)
 create mode 100644 gitnexus/test/fixtures/orm-repo/src/main/resources/mapper/CdataMapper.xml

diff --git a/gitnexus/src/core/ingestion/pipeline-phases/orm.ts b/gitnexus/src/core/ingestion/pipeline-phases/orm.ts
index c3f35136e..a3f17afa6 100644
--- a/gitnexus/src/core/ingestion/pipeline-phases/orm.ts
+++ b/gitnexus/src/core/ingestion/pipeline-phases/orm.ts
@@ -125,7 +125,11 @@ function parseMybatisXml(content: string): { namespace: string; statements: Myba
  */
 function namespaceToFilePath(namespace: string, allPaths: string[]): string | null {
   // com.example.foo.XxxMapper → com/example/foo/XxxMapper.java
-  const rel = namespace.replace(/\./g, '/') + '.java';
+  // Strip inner-class suffix (e.g. "Outer$Inner" → use "Outer.java").
+  const outerNamespace = namespace.includes('$')
+    ? namespace.slice(0, namespace.indexOf('$'))
+    : namespace;
+  const rel = outerNamespace.replace(/\./g, '/') + '.java';
   const found = allPaths.find((p) => p.replace(/\\/g, '/').endsWith(rel));
   return found ?? null;
 }
@@ -205,7 +209,8 @@ function buildMapperMethodIndex(graph: KnowledgeGraph): {
   graph.forEachNode((node) => {
     if (!node.id.startsWith('Method:')) return;
     const filePath = node.properties.filePath as string | undefined;
-    if (!filePath || !filePath.endsWith('Mapper.java')) return;
+    // Match common Java DAO/Mapper interface naming conventions.
+    if (!filePath || !/(?:Mapper|Dao|DAO|Repository)\.java$/.test(filePath)) return;
     filesWithMethods.add(filePath);
     // ID format: "Method:<filePath>:<ClassName>.<methodName>#<paramCount>"
     // Strip the #N suffix to get a param-count-agnostic key.
diff --git a/gitnexus/test/fixtures/orm-repo/src/main/resources/mapper/CdataMapper.xml b/gitnexus/test/fixtures/orm-repo/src/main/resources/mapper/CdataMapper.xml
new file mode 100644
index 000000000..e4fa46cfa
--- /dev/null
+++ b/gitnexus/test/fixtures/orm-repo/src/main/resources/mapper/CdataMapper.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE mapper PUBLIC "-//mybatis.org//DTD Mapper 3.0//EN"
+  "http://mybatis.org/dtd/mybatis-3-mapper.dtd">
+<mapper namespace="com.example.mapper.CdataMapper">
+
+  <!-- CDATA-wrapped SQL with XML special chars -->
+  <select id="selectComplex" resultType="map">
+    <![CDATA[
+      SELECT id, name FROM cdata_table
+      WHERE amount > 100 AND status != 0
+    ]]>
+  </select>
+
+  <!-- XML comment containing SQL keywords should not be extracted -->
+  <!-- FROM ignored_in_comment WHERE 1=1 -->
+  <select id="selectSimple" resultType="map">
+    SELECT id FROM cdata_table WHERE id = #{id}
+  </select>
+
+</mapper>
diff --git a/gitnexus/test/integration/orm-dataflow.test.ts b/gitnexus/test/integration/orm-dataflow.test.ts
index 394c3ac85..0c2838387 100644
--- a/gitnexus/test/integration/orm-dataflow.test.ts
+++ b/gitnexus/test/integration/orm-dataflow.test.ts
@@ -141,4 +141,45 @@ describe('ORM dataflow detection', () => {
       console.log('[info] MyBatis edges linked at File level (no Java parser in fixture)');
     }
   });
+
+  it('extracts table names from CDATA-wrapped SQL', () => {
+    const cdataEdges: string[] = [];
+    for (const rel of result.graph.iterRelationships()) {
+      if (rel.type === 'QUERIES' && rel.reason?.startsWith('mybatis-')) {
+        const target = result.graph.getNode(rel.targetId);
+        if (target?.properties.name === 'cdata_table') {
+          cdataEdges.push(rel.reason ?? '');
+        }
+      }
+    }
+    expect(cdataEdges.length).toBeGreaterThan(0);
+    // XML comment inside <!-- ... --> should NOT produce edges for ignored_in_comment
+    const commentTable = [];
+    for (const rel of result.graph.iterRelationships()) {
+      if (rel.type === 'QUERIES') {
+        const target = result.graph.getNode(rel.targetId);
+        if (target?.properties.name === 'ignored_in_comment') {
+          commentTable.push(target.properties.name);
+        }
+      }
+    }
+    expect(commentTable).toHaveLength(0);
+  });
+
+  it('falls back to xml file path when namespace cannot be resolved', () => {
+    // CdataMapper.xml uses namespace "com.example.mapper.CdataMapper"
+    // but there is no corresponding Java file in the fixture → fallback to xml path
+    let xmlPathEdgeFound = false;
+    for (const rel of result.graph.iterRelationships()) {
+      if (rel.type === 'QUERIES' && rel.reason?.startsWith('mybatis-')) {
+        const source = result.graph.getNode(rel.sourceId);
+        const target = result.graph.getNode(rel.targetId);
+        if (source && target?.properties.name === 'cdata_table') {
+          // Source should be either a File or Method node — both are acceptable
+          xmlPathEdgeFound = true;
+        }
+      }
+    }
+    expect(xmlPathEdgeFound).toBe(true);
+  });
 });