Follow-up from the review of #174 (fixes #173).
Problem
The NPE fixed in #174 had no test covering it, and still does not. Nothing prevents it from regressing.
The existing coverage is MockLambdaProxyMiddlewareTest, which has a single testConstructor case asserting only the happy path:
PathTemplateMatcher.PathMatchResult<String> result = methodToMatcherMap.get("get").match("/v1/pets/123");
Assertions.assertEquals("PetsPetIdGetFunction", result.getValue());
Note that this test itself uses the exact unguarded map.get(method).match(path) pattern that #173 was about — it would NPE the same way for an unmapped method.
More importantly, MockLambdaProxyMiddleware does not share the fixed code path. It has its own execute that resolves functions with a direct map lookup:
var functionName = CONFIG.getFunctions().get(path + "@" + method);
So the mock cannot exercise LambdaProxyMiddlewares matcher logic at all, and no amount of testing through it would have caught #173 or would catch a regression.
Suggested coverage
Against the real LambdaProxyMiddleware, assert that a request whose path matches a configured function but whose method does not:
- returns a
Status with code ERR10086 rather than throwing
- includes
path@method in the status message
Worth adding at the same time:
- a path that matches no configured function at all (the other route into the same
result == null branch)
- the happy path, to confirm the guard did not change matching behaviour
The awkward part is that LambdaProxyMiddlewares constructor builds a real LambdaAsyncClient. Extracting the matcher resolution into a small package-private method that takes path and method and returns the function name or null would make this directly unit-testable without standing up a client, and would let the mock share the real logic instead of duplicating a divergent copy of it.
Follow-up from the review of #174 (fixes #173).
Problem
The NPE fixed in #174 had no test covering it, and still does not. Nothing prevents it from regressing.
The existing coverage is
MockLambdaProxyMiddlewareTest, which has a singletestConstructorcase asserting only the happy path:Note that this test itself uses the exact unguarded
map.get(method).match(path)pattern that #173 was about — it would NPE the same way for an unmapped method.More importantly,
MockLambdaProxyMiddlewaredoes not share the fixed code path. It has its ownexecutethat resolves functions with a direct map lookup:So the mock cannot exercise
LambdaProxyMiddlewares matcher logic at all, and no amount of testing through it would have caught #173 or would catch a regression.Suggested coverage
Against the real
LambdaProxyMiddleware, assert that a request whose path matches a configured function but whose method does not:Statuswith codeERR10086rather than throwingpath@methodin the status messageWorth adding at the same time:
result == nullbranch)The awkward part is that
LambdaProxyMiddlewares constructor builds a realLambdaAsyncClient. Extracting the matcher resolution into a small package-private method that takespathandmethodand returns the function name ornullwould make this directly unit-testable without standing up a client, and would let the mock share the real logic instead of duplicating a divergent copy of it.