mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-11-12 08:41:34 +00:00
fix(group): recognize OpenFeign @RequestLine on plain interfaces (no @FeignClient) (#1917)
* 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 <zhangwei2017@unipus.cn> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
This commit is contained in:
parent
66daf27910
commit
a93ecee068
2 changed files with 100 additions and 11 deletions
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue