From a93ecee0685b9ebb11501137559e51cd7d5bc5ad Mon Sep 17 00:00:00 2001 From: henry201605 <31428013+henry201605@users.noreply.github.com> Date: Sat, 30 May 2026 19:25:13 +0800 Subject: [PATCH] fix(group): recognize OpenFeign @RequestLine on plain interfaces (no @FeignClient) (#1917) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(group): recognize OpenFeign @RequestLine on plain interfaces (no @FeignClient) PR #1904 gated @RequestLine consumer extraction on the enclosing interface also carrying @FeignClient. That guard is wrong: @RequestLine is a core feign.* annotation used with Feign.builder(), while @FeignClient is the Spring Cloud variant that uses Spring MVC annotations (@GetMapping etc.) — the two are effectively mutually exclusive. Requiring @FeignClient therefore excluded the annotation's primary, canonical usage, so the feature recognized nothing on real core-Feign client interfaces. Fix: drop the @FeignClient requirement for @RequestLine. The match still requires an enclosing interface (Feign proxies are always interfaces), and the `RequestLine` annotation name is itself a strong, framework-specific signal, so false-positive risk stays low. A @FeignClient(path=...) prefix is still applied when present. The @(Get|Post|...)Mapping consumer path keeps its @FeignClient requirement: those annotations are generic Spring MVC and need the Feign context to be disambiguated from provider routes. Verification (real-world, not just synthetic fixtures): - A real client-jar consumer (BigModeClientService.java: a plain interface with 12 @RequestLine methods, no @FeignClient) now yields 12 openfeign consumer contracts; it yielded 0 before this change. - End-to-end `group sync` over that consumer repo + its FastAPI provider repo (with zero hand-written links) produces 12 exact cross-links (confidence 1.0), Java @RequestLine consumer → Python route provider. - The prior test that asserted the wrong behavior ("ignores @RequestLine on interfaces without @FeignClient") is reversed into a realistic core-Feign fixture. - Full test/unit/group suite (579) green; tsc and prettier clean. * test(group): add negative cases for relaxed @RequestLine matcher Per review on #1917 — guard the no-@FeignClient relaxation with explicit negative tests: malformed @RequestLine values (no verb / no leading-slash path / unknown verb) yield no contract, and @RequestLine on a concrete class method (not an interface) is not emitted as a consumer. --------- Co-authored-by: henry Co-authored-by: Gergő Magyar --- .../group/extractors/http-patterns/java.ts | 15 ++- .../unit/group/http-route-extractor.test.ts | 96 +++++++++++++++++-- 2 files changed, 100 insertions(+), 11 deletions(-) diff --git a/gitnexus/src/core/group/extractors/http-patterns/java.ts b/gitnexus/src/core/group/extractors/http-patterns/java.ts index 0a4e7fcec..920d499aa 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/java.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/java.ts @@ -721,12 +721,19 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = { }); } - // Native OpenFeign `@RequestLine("METHOD /path")`. Method-level only; the - // enclosing interface MUST carry `@FeignClient`, otherwise the same - // annotation name in unrelated libraries would be a false positive. + // Native OpenFeign `@RequestLine("METHOD /path")`. Method-level only and + // always declared on an interface (Feign builds a proxy from the interface). + // We do NOT require an enclosing `@FeignClient`: `@RequestLine` is a core + // `feign.*` annotation used with `Feign.builder()`, whereas `@FeignClient` + // is the Spring Cloud variant that uses Spring MVC annotations instead — the + // two are effectively mutually exclusive, so requiring `@FeignClient` here + // would miss the annotation's primary use. The `RequestLine` name is itself + // a strong, framework-specific signal, so a structural interface check is + // enough to keep false positives away. A `@FeignClient(path=...)` prefix is + // still applied when present (rare, but harmless). for (const requestLine of requestLines) { const enclosingInterface = findEnclosingInterface(requestLine.methodNode); - if (!enclosingInterface || !hasAnnotation(enclosingInterface, 'FeignClient')) continue; + if (!enclosingInterface) continue; const prefix = feignPrefixByInterfaceId.get(enclosingInterface.id) ?? ''; out.push({ role: 'consumer', diff --git a/gitnexus/test/unit/group/http-route-extractor.test.ts b/gitnexus/test/unit/group/http-route-extractor.test.ts index 90f4d97e9..18b2bebcd 100644 --- a/gitnexus/test/unit/group/http-route-extractor.test.ts +++ b/gitnexus/test/unit/group/http-route-extractor.test.ts @@ -1873,17 +1873,28 @@ interface SearchClient { ).toBeUndefined(); }); - it('ignores @RequestLine on interfaces without @FeignClient', async () => { + it('extracts native @RequestLine on a plain interface without @FeignClient (Feign.builder())', async () => { + // The canonical core-Feign usage: a plain interface with `@RequestLine`, + // wired up via `Feign.builder()`. There is NO `@FeignClient` annotation + // (that is the Spring Cloud variant, which uses Spring MVC annotations and + // is mutually exclusive with `@RequestLine`). This is the shape used by + // real client-jar consumers, so it must be recognized. const dir = path.join(tmpDir, 'java-request-line-no-feign'); fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); fs.writeFileSync( - path.join(dir, 'src', 'PlainInterface.java'), + path.join(dir, 'src', 'BigModelClient.java'), ` +import feign.Headers; import feign.RequestLine; +import feign.Response; -interface PlainInterface { - @RequestLine("GET /not-a-feign-client") - String shouldNotBeExtracted(); +public interface BigModelClient { + @RequestLine("POST /ai/summarization") + @Headers("Content-Type: application/json") + Response summarize(); + + @RequestLine("GET /ai/concurrent") + Response concurrent(); } `, ); @@ -1892,8 +1903,18 @@ interface PlainInterface { const consumers = contracts.filter((c) => c.role === 'consumer'); expect( - consumers.find((c) => c.contractId === 'http::GET::/not-a-feign-client'), - ).toBeUndefined(); + consumers.find( + (c) => + c.contractId === 'http::POST::/ai/summarization' && + c.meta.framework === 'openfeign' && + c.confidence === 0.75, + ), + ).toBeDefined(); + expect( + consumers.find( + (c) => c.contractId === 'http::GET::/ai/concurrent' && c.meta.framework === 'openfeign', + ), + ).toBeDefined(); }); it('mixes @RequestLine and @GetMapping methods on the same @FeignClient interface', async () => { @@ -1993,6 +2014,67 @@ interface WrongKeyClient { ).toBeUndefined(); }); + it('ignores @RequestLine values that are not a "VERB /path" line', async () => { + // `parseRequestLine` only accepts a recognized HTTP verb followed by a + // path starting with `/`. Malformed values (no verb, no leading-slash + // path, or unknown verb) must be dropped — this guards the relaxed + // (no-@FeignClient) matcher from turning arbitrary `@RequestLine` string + // literals into bogus contracts. + const dir = path.join(tmpDir, 'java-request-line-malformed'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'MalformedClient.java'), + ` +import feign.RequestLine; + +interface MalformedClient { + @RequestLine("not a request line at all") + String noVerb(); + + @RequestLine("GET relative/no/leading/slash") + String noLeadingSlash(); + + @RequestLine("FETCH /unknown-verb") + String unknownVerb(); +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + // None of the three malformed values should yield a contract. + expect( + consumers.filter((c) => c.symbolRef.filePath.endsWith('MalformedClient.java')), + ).toHaveLength(0); + }); + + it('ignores @RequestLine on a class method (Feign proxies are interfaces only)', async () => { + // The relaxed matcher still requires an enclosing interface: Feign builds + // its proxy from an interface, so a `@RequestLine` on a concrete class + // method is not a Feign call and must not be emitted as a consumer. + const dir = path.join(tmpDir, 'java-request-line-on-class'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'NotAProxy.java'), + ` +import feign.RequestLine; + +class NotAProxy { + @RequestLine("GET /should-not-extract") + String call() { return null; } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect( + consumers.find((c) => c.contractId === 'http::GET::/should-not-extract'), + ).toBeUndefined(); + }); + it('prefers @FeignClient(path=...) over @RequestMapping when @RequestMapping appears first', async () => { // Reverse-order companion to the precedence test above: @FeignClient(path) // must win even when @RequestMapping is the first annotation in source,