slachiewicz opened a new pull request, #521:
URL: https://github.com/apache/maven-build-cache-extension/pull/521
`maven-build-cache-extension` compiled against Wagon for one thing:
recognising "the remote cache does not have this item" so a miss stays quiet
instead of logging an error.
```java
catch (ResourceDoesNotExistException e) // RemoteCacheRepositoryImpl,
around transporter.get()
```
Resolver's own SPI answers that question directly, and `maven-resolver-spi`
is already a `provided` dependency:
```java
if (transporter.classify(e) == Transporter.ERROR_NOT_FOUND) { ... }
```
Every transporter that can be in play implements it — `WagonTransporter`
returns `ERROR_NOT_FOUND` for exactly `ResourceDoesNotExistException`, and the
HTTP transporters for 404 **and 410**.
Three things improve as a result:
- The reflective `getStatusCode()` call and the class-**name**-walking in
`isHttpResponseException` both go. That hack existed to dodge a
`ClassCastException` across class realms; `classify()` is the real fix, because
the transport does the `instanceof` inside its own realm.
- `410 Gone` is now treated as a miss, which it previously was not.
- 47 lines out, 14 in.
`wagon-provider-api` is no longer declared.
### What is deliberately *not* changed
`wagon-webdav-jackrabbit` stays, and the POM now says why. It looks unused —
nothing imports it — but it is what supplies the `dav:` transport at runtime:
- No Maven distribution ships it. `apache-maven-3.9.16/lib/` and the Maven 4
`lib/` carry `wagon-file`, `wagon-http`, `wagon-http-shared` and
`wagon-provider-api` only. This POM is the sole source of the `dav` role hint
on a user's machine.
- There is no fallback: Resolver's native HTTP transporter accepts
`http`/`https` and throws `NoTransporterException` for anything else, and a
`dav:http://…` URL has protocol `dav:http`.
- It is documented at `src/site/markdown/remote-cache.md:150-160` and
exercised by `RemoteCacheDavTest` against a real WebDAV container.
Removing it would silently take `dav:` remote caches away from users. The
comment is there so the next reader does not reach the "unused dependency"
conclusion again.
### Verification
| | before | after |
|---|---|---|
| `mvn test` | 88 run, 0 failures, 0 errors, 4 skipped | 88 / 0 / 0 / 4 |
| `-Prun-its,run-its-smoke verify` | 121 / 0 / 0 / 0 | 121 / 0 / 0 / 0 |
`dependency:list` is byte-for-byte identical before and after —
`wagon-provider-api` is still on the classpath, now transitively via the webdav
wagon. This removes a declaration, not a jar.
**On whether those tests prove anything:** no unit test instantiates
`RemoteCacheRepositoryImpl`, so the unit suite is not evidence here. The
behaviour is covered by ITs that stub a 404 — `BaselineDiffTest` and
`SaveFinalRemoteTest`. To confirm they genuinely guard it rather than merely
passing, the not-found branch was mutated to `return false`: `BaselineDiffTest`
then failed with `Error downloading cache item`. The mutation was reverted.
**One gap:** `RemoteCacheDavTest` self-skips without a Docker daemon, so the
Wagon branch of `classify()` and the dav provider loading were not exercised
locally. That test in CI is the check worth having before merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
