Core: CatalogTests: invalidate cache after renaming metadata file

[adutra]

This fix prevents test failures when catalogs have a table cache. This issue may arise e.g. with RESTCatalog since the recent introduction of RESTTableCache in 72d5fd6.

[pvary]

@adutra: Is this flaky? Or how can we not detect this issue during the tests there?

CC: @gaborkaszab

[adutra]

@pvary I don't think it's flaky, it's just that the issue will manifest only with a RESTCatalog that has caching by ETag enabled, which is not the case in TestRESTCatalog (but is the case in the Apache Polaris repo, that's where I spotted the issue).

[pvary]

@pvary I don't think it's flaky, it's just that the issue will manifest only with a RESTCatalog that has caching by ETag enabled, which is not the case in TestRESTCatalog (but is the case in the Apache Polaris repo, that's where I spotted the issue).

Shall we add a test which fails without the change? Otherwise what would prevent someone to break this again for Polaris?

[adutra]

Shall we add a test which fails without the change? Otherwise what would prevent someone to break this again for Polaris?

I think the proposed fix is enough and is the best option.

Let me explain the issue differently:

When this test is executed with an Apache Polaris server:

1. The metadata file is deleted manually
2. RESTCatalog.loadTable is called
3. The RESTCatalog client sends a request with an IfNoneMatch header
4. The server loads the table metadata from its cache without hitting the storage layer
5. The server sends back a 304 (Not Changed) response
6. The RESTCatalog client returns the cached Table instance
7. The test fails.

This issue doesn't happen in Iceberg REST tests because:

1. The test was overridden in TestRESTCatalog to use InMemoryFileIO to delete the file
2. RESTCatalogAdapter loads the table using the same InMemoryFileIO and thus "notices" the file deletion
3. The test passes.

There is imho a fundamental problem in the test since it relies on the fact that the loadTable call will NOT be cached, and will NOT be served from memory. But this assumption relies on implementation details that differ from catalog to catalog, thus making the test outcome unreliable.

[adutra]

Another problem that I'm not fixing for now is that the test only works for catalogs with local storage since it creates files on the local filesystem. It does not work with S3FileIO for example (we have plenty of those in Polaris tests).

If possible I also would like to rewrite the test as follows:

    ... // same setup
    table.io().deleteFile(metadataFileLocation);
    catalog.invalidateTable(TBL);
    assertThatThrownBy(() -> catalog.loadTable(TBL))
        .isInstanceOf(NotFoundException.class)
        .hasMessageContaining(metadataFileLocation);

@gaborkaszab do you know why the test needs to use Path instances? Do you agree with the modified version above? Thanks!