refactor(io): make FileIO resolution registry-driven

[wgtmac]

Problem

The existing FileIO design embeds scheme ownership in ResolvingFileIO and
registers the resolver itself as a special implementation. This duplicates
backend-specific scheme knowledge, prevents custom FileIOs from declaring the
schemes they support, and couples automatic routing to a special registry
entry.

Cached delegates are also returned as raw pointers while credential refresh
can invalidate the cache. In addition, treating oss:// as an S3 alias is not
safe without provider-specific endpoint and compatibility validation.

Changes

• Make FileIORegistry::Factory contain a required create callback and an
  optional accepts callback.
• Resolve normalized schemes from registered factories, with later
  registrations overriding earlier ones.
• Construct the default ResolvingFileIO directly while preserving explicit
  io-impl precedence.
• Use Java-compatible first-colon scheme parsing and remove hard-coded scheme
  mapping from the resolver.
• Keep cached delegates alive with shared_ptr across credential refreshes.
• Forward complete credential lists to delegates that support them and rebuild
  delegates after refresh.
• Limit Arrow S3 routing and credential prefix handling to s3, s3a, and
  s3n; defer OSS/COS support to a separate change.
• Add focused registry, resolver, REST, and S3 coverage.
• Document built-in and custom FileIO usage.

[Copilot]

Pull request overview

Refactors FileIO resolution so ResolvingFileIO routes locations via FileIORegistry factories (registry-driven scheme ownership), improves delegate caching across credential refreshes, narrows S3 routing to s3/s3a/s3n, and adds/updates targeted coverage plus end-user documentation.

Changes:

• Introduces FileIORegistry::Factory{create, accepts} and scheme-based FileIORegistry::Resolve() with “latest registration wins” semantics.
• Updates ResolvingFileIO to resolve schemes via the registry (Java-style first-colon parsing) and to cache delegates using shared_ptr with refreshed-credential rebuild behavior.
• Updates tests, build files, and docs to reflect the new registry-driven routing model and supported S3 schemes.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/iceberg/util/location_util.h	Adds LocationUtil::ParseScheme declaration used for scheme routing.
src/iceberg/util/location_util.cc	Implements ParseScheme for Java-style first-colon scheme parsing.
src/iceberg/file_io_registry.h	Refactors registry API to struct-based factories and adds Resolve(scheme).
src/iceberg/file_io_registry.cc	Implements ordered registrations, explicit load, and scheme resolution.
src/iceberg/resolving_file_io.h	Switches caching to shared_ptr and updates locking to shared_mutex.
src/iceberg/resolving_file_io.cc	Routes per-location via FileIORegistry::Resolve, caches delegates, rebuilds on credential refresh, and groups bulk deletes by delegate.
src/iceberg/catalog/rest/rest_file_io.cc	Defaults REST catalog FileIO to directly constructed ResolvingFileIO when io-impl is absent.
src/iceberg/arrow/arrow_register.cc	Registers built-in local/S3 FileIOs with create + accepts callbacks.
src/iceberg/arrow/s3/s3_properties.h	Centralizes S3 scheme list and helpers for scheme acceptance.
src/iceberg/arrow/s3/arrow_s3_file_io.cc	Removes OSS alias handling from S3 credential prefix logic and scheme canonicalization.
src/iceberg/util/location_util.cc	(Also) relocates scheme parsing logic into a shared util.
src/iceberg/test/rest_file_io_test.cc	Adjusts default REST FileIO expectation; adds a registry-delegation test; updates registry registration callsites.
src/iceberg/test/rest_catalog_integration_test.cc	Updates registry registration to new factory struct form.
src/iceberg/test/resolving_file_io_test.cc	Updates routing assumptions (no OSS alias), adds batch delete grouping tests, and validates no fallback after selected factory failure.
src/iceberg/test/location_util_test.cc	Adds unit test coverage for ParseScheme.
src/iceberg/test/arrow_s3_file_io_test.cc	Removes OSS from S3-compatible credential prefixes; updates endpoint scheme test data away from OSS-specific values.
src/iceberg/test/arrow_io_test.cc	Adds a registration smoke test ensuring built-in registry routing works via ResolvingFileIO.
src/iceberg/test/rest_arrow_file_io_test.cc	Removes an integration test that depended on the prior OSS/S3 routing behavior and bundle linkage.
src/iceberg/test/CMakeLists.txt	Removes bundle-only REST Arrow FileIO test wiring and USE_BUNDLE option.
src/iceberg/CMakeLists.txt	Adds util/location_util.cc to the CMake build.
src/iceberg/meson.build	Adds util/location_util.cc to the Meson build.
mkdocs/mkdocs.yml	Adds FileIO docs page to the documentation nav.
mkdocs/docs/file-io.md	Documents built-in/custom FileIO registration and selection, plus credential forwarding behavior.

Suppressed comments (1)

src/iceberg/util/location_util.cc:28

• ParseScheme currently treats any text before the first ':' as a scheme, even for local paths that may contain ':' (e.g. Windows C:\\... or a POSIX path segment with ':'). That makes ResolvingFileIO attempt registry resolution for what should be a local path and can yield kNotSupported.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/iceberg/util/location_util.cc:29

• ParseScheme treats any first-colon prefix as a scheme, which will interpret Windows drive-letter paths like C:\tmp\file.parquet / C:/tmp/file.parquet as scheme c and break local FileIO resolution (local accepts only empty or file). Consider special-casing drive-letter paths on Windows so they are treated as "no scheme".
  src/iceberg/test/location_util_test.cc:73
• ParseScheme now follows first-colon parsing; add a Windows drive-letter case to the unit test to prevent regressions for local paths like C:\\tmp\\file.parquet / C:/tmp/file.parquet.

TEST(LocationUtilTest, ParseScheme) {
  auto s3 = LocationUtil::ParseScheme("S3://bucket/path");
  EXPECT_EQ(s3, "S3");

  auto no_scheme = LocationUtil::ParseScheme("/tmp/file.parquet");
  EXPECT_TRUE(no_scheme.empty());

  auto empty_scheme = LocationUtil::ParseScheme("://bucket/path");
  EXPECT_TRUE(empty_scheme.empty());

[wgtmac]

I've made some refactoring to the FileIO resolution. Let me know what you think. @plusplusjiajia

[wgtmac]

cc @MisterRaindrop

[plusplusjiajia]

I've made some refactoring to the FileIO resolution. Let me know what you think. @plusplusjiajia

@wgtmac Thanks for looping me in — registry-driven resolution is right, and shared_ptr delegates close a real use-after-free window.

credential prefixes — agree. s3/s3a/s3n matches Java's S3FileIO.

routing — I'd keep oss. This is stricter than Java: S3URI applies no scheme whitelist, and its javadoc says it "supports any valid URI schemes ... allows users to use S3FileIO with other S3-compatible object storage services". PyIceberg routes oss the same way (map, init). ArrowS3FileIO is equally capable — ResolvePath already handles foreign schemes — so kS3Schemes is a new gate, and an unmatched scheme is a hard NotSupported, which makes such stores unusable.

[MisterRaindrop]

ResolvingFileIO does not have a bound stable registry snapshot therefore, the parsing results returned by FileIO may be affected by the process-wide mutable registry ?
example:
Step 1: A(s3) Registration
Step 2: Resolver resolves s3 -> A

Time 3: A is replaced by a record with the same name.
Result: Resolver still hit an old cache entry named "A".

I just feel maybe there is this kind of problem.

[wgtmac]

I think the contract is that FileIO registration should be done before anything. A created ResolvingFileIO should just use its cached instances to serve followup calls.

[wgtmac]

@plusplusjiajia I‘d prefer postponing the support of oss scheme to a separate PR. Correctly we have no evidence that it works. Perhaps we can provide a cli for FileIO to verify its connection.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

Suppressed comments (7)

src/iceberg/test/rest_file_io_test.cc:73

• This test used to verify that the default FileIO supports vended storage credentials via AsSupportsStorageCredentials(). The new dynamic_cast<ResolvingFileIO*> check verifies the concrete type but no longer asserts the credential-support contract that MakeTableFileIO() relies on. Consider re-adding an assertion that result.value()->AsSupportsStorageCredentials() is non-null (in addition to, or instead of, the dynamic_cast).

TEST(RestFileIOTest, MakeCatalogFileIODefaultsToResolvingFileIO) {

src/iceberg/file_io_registry.cc:111

• Resolve() takes a full snapshot copy of all registrations on every call. Since ResolvingFileIO calls into Resolve() per path operation, this can become a hot-path allocation/copy. A common pattern here is copy-on-write at registration time (e.g., store an std::shared_ptr<const std::vector<Entry>> and atomically swap it in Register), so Resolve() only reads a pointer without copying.

Result<std::string> FileIORegistry::Resolve(std::string_view scheme) {
  const std::string normalized_scheme = StringUtils::ToLower(scheme);
  const auto entries = SnapshotEntries();
  // Newest registrations take precedence.
  for (const auto& entry : std::ranges::reverse_view(entries)) {
    if (entry.factory.accepts && entry.factory.accepts(normalized_scheme)) {
      return entry.name;
    }
  }

src/iceberg/resolving_file_io.cc:41

• FileIORegistry::Resolve() already lowercases schemes internally, so lowercasing in ResolvingFileIO is redundant work on the routing path. Consider passing the raw ParseScheme() result to Resolve() (or changing Resolve()’s contract to accept already-normalized schemes, but that would broaden API impact).

  const auto scheme = StringUtils::ToLower(LocationUtil::ParseScheme(location));
  ICEBERG_ASSIGN_OR_RAISE(const auto name, FileIORegistry::Resolve(scheme));

src/iceberg/test/rest_file_io_test.cc:82

• This test used to verify that the default FileIO supports vended storage credentials via AsSupportsStorageCredentials(). The new dynamic_cast<ResolvingFileIO*> check verifies the concrete type but no longer asserts the credential-support contract that MakeTableFileIO() relies on. Consider re-adding an assertion that result.value()->AsSupportsStorageCredentials() is non-null (in addition to, or instead of, the dynamic_cast).

    EXPECT_NE(dynamic_cast<ResolvingFileIO*>(result.value().get()), nullptr);
  }
}

src/iceberg/test/CMakeLists.txt:279

• The PR removes rest_arrow_file_io_test (and its CMake wiring), which previously covered an end-to-end path: REST → MakeTableFileIO → ResolvingFileIO → registry → real Arrow-backed delegates (including local FS). The new tests add useful unit coverage, but the deleted integration coverage for 'real implementations' is not obviously replaced for the REST path. Consider adding back a minimal integration test that exercises REST + ResolvingFileIO + real local FileIO (no S3 dependency) to guard against registration/routing regressions.

if(ICEBERG_BUILD_REST)

src/iceberg/test/CMakeLists.txt:287

• The PR removes rest_arrow_file_io_test (and its CMake wiring), which previously covered an end-to-end path: REST → MakeTableFileIO → ResolvingFileIO → registry → real Arrow-backed delegates (including local FS). The new tests add useful unit coverage, but the deleted integration coverage for 'real implementations' is not obviously replaced for the REST path. Consider adding back a minimal integration test that exercises REST + ResolvingFileIO + real local FileIO (no S3 dependency) to guard against registration/routing regressions.

    cmake_parse_arguments(ARG
                          ""
                          "${oneValueArgs}"
                          "${multiValueArgs}"
                          ${ARGN})

src/iceberg/test/CMakeLists.txt:299

• The PR removes rest_arrow_file_io_test (and its CMake wiring), which previously covered an end-to-end path: REST → MakeTableFileIO → ResolvingFileIO → registry → real Arrow-backed delegates (including local FS). The new tests add useful unit coverage, but the deleted integration coverage for 'real implementations' is not obviously replaced for the REST path. Consider adding back a minimal integration test that exercises REST + ResolvingFileIO + real local FileIO (no S3 dependency) to guard against registration/routing regressions.

  add_rest_iceberg_test(rest_catalog_test

[plusplusjiajia]

@plusplusjiajia I‘d prefer postponing the support of oss scheme to a separate PR. Correctly we have no evidence that it works. Perhaps we can provide a cli for FileIO to verify its connection.

Agreed — no objection to this going in as is. I'll bring oss back in #893 with the test and a fileio.md

[wgtmac]

Thanks everyone!