[runtime] Resolve relative file: URLs when loading classpath skills (#966)

[Olawoyin007]

What is the purpose of the change

Fixes #966.

A Flink deployment can add user-code JARs relative to the TaskManager working directory, so a classloader may expose a skill resource through a relative file: URL such as file:../../flink/usrlib/job.jar (or jar:file:../../flink/usrlib/job.jar!/diagnosis-skills). Such a URL parses to an opaque URI, which new File(URI) / Paths.get(URI) reject with IllegalArgumentException: URI is not hierarchical, so ClasspathSkillRepository initialization fails and skills never load.

Brief change log

• Add LocalUrls.toLocalFile(URL), a single place that converts a local file: URL to a File:
  • absolute hierarchical file: URLs keep their existing new File(uri) behavior;
  • opaque relative file: URLs are resolved against the process working directory via the decoded scheme-specific part;
  • non-file URLs are rejected explicitly.
• Reuse it in the three affected sites so their behavior can't drift:
  • ClasspathSkillRepository.materializeFileUrl (direct file/dir/zip resource),
  • SkillMaterializer.copyJarEntries (JAR entry extraction),
  • ClasspathSkillRepository.findAllMatches (URLClassLoader fallback scan).
• Remove now-unused imports.

Verifying this change

• New LocalUrlsTest covers absolute resolution, relative/opaque resolution against the working directory, and non-file rejection.
• New ClasspathSkillRepositoryTest#loadFromRelativeJarUrl loads skills end-to-end through a relative jar URL (exercises both the fallback scan and jar extraction). Verified that this test fails against the pre-fix code with the reported Classpath resource not found / URI is not hierarchical error.
• Full runtime module test suite passes (668 tests, 0 failures).

Does this pull request potentially affect one of the following parts

• Dependencies (does it add or upgrade a dependency): no
• The public API: no
• The serializers: no
• The runtime per-record code paths: no
• Anything that affects deployment or recovery: no
• The threading model: no

Documentation

• Does this pull request introduce a new feature? no

[wenjin272]

Thanks for working on this fix! The overall approach looks good. I left two inline comments about an exception-handling edge case and regression-test coverage.

[wenjin272]

Could we also wrap the IllegalArgumentException thrown by new File(uri)? For example, file://host/share/job.jar is hierarchical, but new File(uri) throws IllegalArgumentException: URI has an authority component. Since the callers only catch IOException, this escapes the fallback scan instead of being skipped and also changes the previous copyJarEntries() exception contract. Shall we keep the conversion inside the try and wrap both URISyntaxException and IllegalArgumentException as IOException?

[Olawoyin007]

Good catch, fixed in 05b4a46. new File(uri) is now inside the try, and the catch handles both URISyntaxException and IllegalArgumentException, wrapping them as IOException. So a hierarchical-but-non-representable URL like file://host/share/job.jar now surfaces as IOException and is skipped by the fallback scan, restoring the previous copyJarEntries contract instead of escaping unchecked. Added a LocalUrlsTest case asserting the authority-component URL comes back as IOException.

[wenjin272]

This test covers the URLClassLoader fallback scan and JAR extraction paths, but the PR also changes direct resource materialization. Could we add regression cases where getResources() directly returns a relative jar:file: URL and a relative file: directory URL? That would exercise all three affected call paths described in the PR.

[Olawoyin007]

Agreed, added in 05b4a46. Two new regression tests feed the URLs through getResources() directly (via a non-URLClassLoader loader, so the fallback scan is bypassed and each path is exercised in isolation): loadFromDirectRelativeJarUrl returns a relative jar:file: URL to drive SkillMaterializer.copyJarEntries, and loadFromDirectRelativeDirectoryUrl returns a relative file: directory URL to drive ClasspathSkillRepository.materializeFileUrl. Together with the existing fallback-scan test, all three affected call paths are now covered.