gaborkaszab commented on code in PR #15082: URL: https://github.com/apache/iceberg/pull/15082#discussion_r2707269266
########## core/src/test/java/org/apache/iceberg/rest/TestBaseWithRESTServer.java: ########## @@ -0,0 +1,173 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.ObjectReader; +import java.io.File; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.nio.file.Path; +import java.util.Map; +import java.util.UUID; +import java.util.function.Consumer; +import org.apache.hadoop.conf.Configuration; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SessionCatalog; +import org.apache.iceberg.inmemory.InMemoryCatalog; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.rest.responses.ErrorResponse; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.handler.gzip.GzipHandler; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mockito; + +public abstract class TestBaseWithRESTServer { + private static final ObjectMapper MAPPER = RESTObjectMapper.mapper(); + + protected static final Namespace NS = Namespace.of("ns"); + protected static final SessionCatalog.SessionContext DEFAULT_SESSION_CONTEXT = + new SessionCatalog.SessionContext( + UUID.randomUUID().toString(), + "user", + ImmutableMap.of("credential", "user:12345"), + ImmutableMap.of()); + + protected InMemoryCatalog backendCatalog; + protected RESTCatalogAdapter adapterForRESTServer; + protected Server httpServer; + protected RESTCatalog restCatalog; + protected ParserContext parserContext; + + @TempDir private Path temp; + + @BeforeEach + public void setupCatalog() throws Exception { + File warehouse = temp.toFile(); + this.backendCatalog = new InMemoryCatalog(); + this.backendCatalog.initialize( + "in-memory", + ImmutableMap.of(CatalogProperties.WAREHOUSE_LOCATION, warehouse.getAbsolutePath())); + + adapterForRESTServer = createAdapterForServer(); + + ServletContextHandler servletContext = + new ServletContextHandler(ServletContextHandler.NO_SESSIONS); + servletContext.addServlet( + new ServletHolder(new RESTCatalogServlet(adapterForRESTServer)), "/*"); + servletContext.setHandler(new GzipHandler()); + + this.httpServer = new Server(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0)); + httpServer.setHandler(servletContext); + httpServer.start(); + + restCatalog = initCatalog(catalogName(), ImmutableMap.of()); + } + + @AfterEach + public void teardownCatalogs() throws Exception { + if (restCatalog != null) { + restCatalog.close(); + } + + if (backendCatalog != null) { + backendCatalog.close(); + } + + if (httpServer != null) { + httpServer.stop(); + httpServer.join(); + } + } + + protected RESTCatalogAdapter createAdapterForServer() { + return Mockito.spy( + new RESTCatalogAdapter(backendCatalog) { + @Override + public <T extends RESTResponse> T execute( + HTTPRequest request, + Class<T> responseType, + Consumer<ErrorResponse> errorHandler, + Consumer<Map<String, String>> responseHeaders) { + Object body = roundTripSerialize(request.body(), "request"); + HTTPRequest req = ImmutableHTTPRequest.builder().from(request).body(body).build(); + T response = super.execute(req, responseType, errorHandler, responseHeaders); + return roundTripSerialize(response, "response"); + } + }); + } + + protected abstract String catalogName(); + + @SuppressWarnings("unchecked") + protected <T> T roundTripSerialize(T payload, String description) { + if (payload == null) { + return null; + } + + try { + if (payload instanceof RESTMessage) { + RESTMessage message = (RESTMessage) payload; + ObjectReader reader = MAPPER.readerFor(message.getClass()); + if (parserContext != null && !parserContext.isEmpty()) { Review Comment: I added the general version into the base class and an override into TestRESTScanPlanning ########## core/src/test/java/org/apache/iceberg/rest/TestFreshnessAwareLoading.java: ########## @@ -0,0 +1,797 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest; + +import static org.apache.iceberg.TestBase.FILE_A; +import static org.apache.iceberg.TestBase.SCHEMA; +import static org.apache.iceberg.rest.RESTRequestMatcher.match; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import com.github.benmanes.caffeine.cache.Cache; +import java.time.Duration; +import java.util.Map; +import java.util.Set; +import java.util.function.BiFunction; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Supplier; +import org.apache.http.HttpHeaders; +import org.apache.iceberg.BaseMetadataTable; +import org.apache.iceberg.BaseTable; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.Table; +import org.apache.iceberg.TableMetadata; +import org.apache.iceberg.catalog.SessionCatalog; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.exceptions.NoSuchTableException; +import org.apache.iceberg.exceptions.RESTException; +import org.apache.iceberg.io.FileIO; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.apache.iceberg.rest.responses.ConfigResponse; +import org.apache.iceberg.rest.responses.ErrorResponse; +import org.apache.iceberg.rest.responses.LoadTableResponse; +import org.apache.iceberg.types.Types; +import org.apache.iceberg.util.FakeTicker; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.Mockito; +import org.mockito.stubbing.Answer; + +public class TestFreshnessAwareLoading extends TestBaseWithRESTServer { + private static final ResourcePaths RESOURCE_PATHS = + ResourcePaths.forCatalogProperties( + ImmutableMap.of( + RESTCatalogProperties.NAMESPACE_SEPARATOR, + RESTCatalogAdapter.NAMESPACE_SEPARATOR_URLENCODED_UTF_8)); + private static final TableIdentifier TABLE = TableIdentifier.of(NS, "newtable"); + private static final Duration TABLE_EXPIRATION = + Duration.ofMillis(RESTCatalogProperties.TABLE_CACHE_EXPIRE_AFTER_WRITE_MS_DEFAULT); + private static final Duration HALF_OF_TABLE_EXPIRATION = TABLE_EXPIRATION.dividedBy(2); + + @Override + protected String catalogName() { + return "catalog-freshness-aware-loading"; + } + + @Test + public void eTagWithCreateAndLoadTable() { + Map<String, String> respHeaders = Maps.newHashMap(); + RESTCatalog catalog = catalogWithResponseHeaders(respHeaders); + + catalog.createNamespace(TABLE.namespace()); + catalog.createTable(TABLE, SCHEMA); + + assertThat(respHeaders).containsKey(HttpHeaders.ETAG); + String eTag = respHeaders.get(HttpHeaders.ETAG); + respHeaders.clear(); + + catalog.loadTable(TABLE); + + assertThat(respHeaders).containsEntry(HttpHeaders.ETAG, eTag); + } + + @Test + public void eTagWithDifferentTables() { + Map<String, String> respHeaders = Maps.newHashMap(); + RESTCatalog catalog = catalogWithResponseHeaders(respHeaders); + + catalog.createNamespace(TABLE.namespace()); + catalog.createTable(TABLE, SCHEMA); + + assertThat(respHeaders).containsKey(HttpHeaders.ETAG); + String eTagTbl1 = respHeaders.get(HttpHeaders.ETAG); + respHeaders.clear(); + + catalog.createTable(TableIdentifier.of(TABLE.namespace(), "table2"), SCHEMA); + + assertThat(respHeaders).containsKey(HttpHeaders.ETAG); + assertThat(eTagTbl1).isNotEqualTo(respHeaders.get(HttpHeaders.ETAG)); + } + + @Test + public void eTagAfterDataUpdate() { + Map<String, String> respHeaders = Maps.newHashMap(); + RESTCatalog catalog = catalogWithResponseHeaders(respHeaders); + + catalog.createNamespace(TABLE.namespace()); + Table tbl = catalog.createTable(TABLE, SCHEMA); + + assertThat(respHeaders).containsKey(HttpHeaders.ETAG); + String eTag = respHeaders.get(HttpHeaders.ETAG); + + respHeaders.clear(); + tbl.newAppend().appendFile(FILE_A).commit(); + + assertThat(respHeaders).containsKey(HttpHeaders.ETAG); + assertThat(eTag).isNotEqualTo(respHeaders.get(HttpHeaders.ETAG)); + } + + @Test + public void eTagAfterMetadataOnlyUpdate() { + Map<String, String> respHeaders = Maps.newHashMap(); + RESTCatalog catalog = catalogWithResponseHeaders(respHeaders); + + catalog.createNamespace(TABLE.namespace()); + Table tbl = catalog.createTable(TABLE, SCHEMA); + + assertThat(respHeaders).containsKey(HttpHeaders.ETAG); + String eTag = respHeaders.get(HttpHeaders.ETAG); + + respHeaders.clear(); + tbl.updateSchema().addColumn("extra", Types.IntegerType.get()).commit(); + + assertThat(respHeaders).containsKey(HttpHeaders.ETAG); + assertThat(eTag).isNotEqualTo(respHeaders.get(HttpHeaders.ETAG)); + } + + @Test + public void eTagWithRegisterTable() { + Map<String, String> respHeaders = Maps.newHashMap(); + RESTCatalog catalog = catalogWithResponseHeaders(respHeaders); + + catalog.createNamespace(TABLE.namespace()); + Table tbl = catalog.createTable(TABLE, SCHEMA); + + assertThat(respHeaders).containsKey(HttpHeaders.ETAG); + String eTag = respHeaders.get(HttpHeaders.ETAG); + + respHeaders.clear(); + catalog.registerTable( + TableIdentifier.of(TABLE.namespace(), "other_table"), + ((BaseTable) tbl).operations().current().metadataFileLocation()); + + assertThat(respHeaders).containsEntry(HttpHeaders.ETAG, eTag); + } + + @Test + public void notModifiedResponse() { + restCatalog.createNamespace(TABLE.namespace()); + restCatalog.createTable(TABLE, SCHEMA); + Table table = restCatalog.loadTable(TABLE); + + String eTag = + ETagProvider.of(((BaseTable) table).operations().current().metadataFileLocation()); + + Mockito.doAnswer( + invocation -> { + HTTPRequest originalRequest = invocation.getArgument(0); + + assertThat(originalRequest.headers().contains(HttpHeaders.IF_NONE_MATCH)); + assertThat( + originalRequest.headers().firstEntry(HttpHeaders.IF_NONE_MATCH).get().value()) + .isEqualTo(eTag); + + assertThat( + adapterForRESTServer.execute( + originalRequest, + LoadTableResponse.class, + invocation.getArgument(2), + invocation.getArgument(3), + ParserContext.builder().build())) + .isNull(); + + return null; + }) + .when(adapterForRESTServer) + .execute( + match(HTTPRequest.HTTPMethod.GET, RESOURCE_PATHS.table(TABLE)), + eq(LoadTableResponse.class), + any(), + any()); + + assertThat(restCatalog.loadTable(TABLE)).isNotNull(); + + TableIdentifier metadataTableIdentifier = + TableIdentifier.of(NS.toString(), TABLE.name(), "partitions"); + + assertThat(restCatalog.loadTable(metadataTableIdentifier)).isNotNull(); + + Mockito.verify(adapterForRESTServer, times(3)) + .execute( + match(HTTPRequest.HTTPMethod.GET, RESOURCE_PATHS.table(TABLE)), + eq(LoadTableResponse.class), + any(), + any()); + + verify(adapterForRESTServer) + .execute( + match(HTTPRequest.HTTPMethod.GET, RESOURCE_PATHS.table(metadataTableIdentifier)), + any(), + any(), + any()); + } + + @Test + public void freshnessAwareLoading() { + restCatalog.createNamespace(TABLE.namespace()); + restCatalog.createTable(TABLE, SCHEMA); + + Cache<RESTTableCache.SessionIdTableId, RESTTableCache.TableWithETag> tableCache = + restCatalog.sessionCatalog().tableCache().cache(); + assertThat(tableCache.estimatedSize()).isZero(); + + expectFullTableLoadForLoadTable(TABLE, adapterForRESTServer); + BaseTable tableAfterFirstLoad = (BaseTable) restCatalog.loadTable(TABLE); + + assertThat(tableCache.stats().hitCount()).isZero(); + assertThat(tableCache.asMap()) + .containsOnlyKeys( + RESTTableCache.SessionIdTableId.of(DEFAULT_SESSION_CONTEXT.sessionId(), TABLE)); + + expectNotModifiedResponseForLoadTable(TABLE, adapterForRESTServer); + BaseTable tableAfterSecondLoad = (BaseTable) restCatalog.loadTable(TABLE); + + assertThat(tableAfterFirstLoad).isNotEqualTo(tableAfterSecondLoad); + assertThat(tableAfterFirstLoad.operations().current().location()) + .isEqualTo(tableAfterSecondLoad.operations().current().location()); + assertThat( + tableCache + .asMap() + .get(RESTTableCache.SessionIdTableId.of(DEFAULT_SESSION_CONTEXT.sessionId(), TABLE)) + .supplier() + .get() + .operations() + .current() + .metadataFileLocation()) + .isEqualTo(tableAfterFirstLoad.operations().current().metadataFileLocation()); + + Mockito.verify(adapterForRESTServer, times(2)) + .execute( + match(HTTPRequest.HTTPMethod.GET, RESOURCE_PATHS.table(TABLE)), any(), any(), any()); + } + + @Test + public void freshnessAwareLoadingMetadataTables() { + restCatalog.createNamespace(TABLE.namespace()); + restCatalog.createTable(TABLE, SCHEMA); + + Cache<RESTTableCache.SessionIdTableId, RESTTableCache.TableWithETag> tableCache = + restCatalog.sessionCatalog().tableCache().cache(); + assertThat(tableCache.estimatedSize()).isZero(); + + BaseTable table = (BaseTable) restCatalog.loadTable(TABLE); + + assertThat(tableCache.stats().hitCount()).isZero(); + assertThat(tableCache.asMap()) + .containsOnlyKeys( + RESTTableCache.SessionIdTableId.of(DEFAULT_SESSION_CONTEXT.sessionId(), TABLE)); + + TableIdentifier metadataTableIdentifier = + TableIdentifier.of(TABLE.namespace().toString(), TABLE.name(), "partitions"); + + BaseMetadataTable metadataTable = + (BaseMetadataTable) restCatalog.loadTable(metadataTableIdentifier); + + assertThat(tableCache.stats().hitCount()).isEqualTo(1); + assertThat(tableCache.asMap()) + .containsOnlyKeys( + RESTTableCache.SessionIdTableId.of(DEFAULT_SESSION_CONTEXT.sessionId(), TABLE)); + + assertThat(table).isNotEqualTo(metadataTable.table()); Review Comment: Ahh, indeed, thx ########## core/src/test/java/org/apache/iceberg/rest/RESTRequestMatcher.java: ########## @@ -0,0 +1,82 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest; + +import static org.mockito.ArgumentMatchers.argThat; + +import java.util.Map; +import java.util.Objects; + +class RESTRequestMatcher { Review Comment: Done ########## core/src/test/java/org/apache/iceberg/rest/TestRESTTableCache.java: ########## @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.Map; +import org.junit.jupiter.api.Test; + +public class TestRESTTableCache { + @Test + public void invalidProperties() { + assertThatThrownBy( + () -> + new RESTTableCache( + Map.of(RESTCatalogProperties.TABLE_CACHE_EXPIRE_AFTER_WRITE_MS, "0"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid expire after write: zero or negative"); + + assertThatThrownBy( + () -> + new RESTTableCache( + Map.of(RESTCatalogProperties.TABLE_CACHE_EXPIRE_AFTER_WRITE_MS, "-1"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid expire after write: zero or negative"); + + assertThatThrownBy( + () -> new RESTTableCache(Map.of(RESTCatalogProperties.TABLE_CACHE_MAX_ENTRIES, "-1"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid max entries: negative"); + } +} Review Comment: I was hesitating to keep this PR refactor only or to add new tests here. I added them now. I think that the expiration related tests in `TestFreshnessAwareLoading` are still relevant and should be kept. WDYT? ########## core/src/test/java/org/apache/iceberg/rest/TestBaseWithRESTServer.java: ########## @@ -0,0 +1,173 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.ObjectReader; +import java.io.File; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.nio.file.Path; +import java.util.Map; +import java.util.UUID; +import java.util.function.Consumer; +import org.apache.hadoop.conf.Configuration; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SessionCatalog; +import org.apache.iceberg.inmemory.InMemoryCatalog; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.rest.responses.ErrorResponse; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.handler.gzip.GzipHandler; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mockito; + +public abstract class TestBaseWithRESTServer { + private static final ObjectMapper MAPPER = RESTObjectMapper.mapper(); + + protected static final Namespace NS = Namespace.of("ns"); + protected static final SessionCatalog.SessionContext DEFAULT_SESSION_CONTEXT = + new SessionCatalog.SessionContext( + UUID.randomUUID().toString(), + "user", + ImmutableMap.of("credential", "user:12345"), + ImmutableMap.of()); + + protected InMemoryCatalog backendCatalog; + protected RESTCatalogAdapter adapterForRESTServer; + protected Server httpServer; + protected RESTCatalog restCatalog; + protected ParserContext parserContext; + + @TempDir private Path temp; + + @BeforeEach + public void setupCatalog() throws Exception { + File warehouse = temp.toFile(); + this.backendCatalog = new InMemoryCatalog(); + this.backendCatalog.initialize( + "in-memory", + ImmutableMap.of(CatalogProperties.WAREHOUSE_LOCATION, warehouse.getAbsolutePath())); + + adapterForRESTServer = createAdapterForServer(); + + ServletContextHandler servletContext = + new ServletContextHandler(ServletContextHandler.NO_SESSIONS); + servletContext.addServlet( + new ServletHolder(new RESTCatalogServlet(adapterForRESTServer)), "/*"); + servletContext.setHandler(new GzipHandler()); + + this.httpServer = new Server(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0)); + httpServer.setHandler(servletContext); + httpServer.start(); + + restCatalog = initCatalog(catalogName(), ImmutableMap.of()); + } + + @AfterEach + public void teardownCatalogs() throws Exception { Review Comment: In fact check style fails with these names. I renamed them to before()/after() ########## core/src/test/java/org/apache/iceberg/rest/TestBaseWithRESTServer.java: ########## @@ -0,0 +1,173 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.ObjectReader; +import java.io.File; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.nio.file.Path; +import java.util.Map; +import java.util.UUID; +import java.util.function.Consumer; +import org.apache.hadoop.conf.Configuration; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SessionCatalog; +import org.apache.iceberg.inmemory.InMemoryCatalog; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.rest.responses.ErrorResponse; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.handler.gzip.GzipHandler; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mockito; + +public abstract class TestBaseWithRESTServer { + private static final ObjectMapper MAPPER = RESTObjectMapper.mapper(); + + protected static final Namespace NS = Namespace.of("ns"); + protected static final SessionCatalog.SessionContext DEFAULT_SESSION_CONTEXT = + new SessionCatalog.SessionContext( + UUID.randomUUID().toString(), + "user", + ImmutableMap.of("credential", "user:12345"), + ImmutableMap.of()); + + protected InMemoryCatalog backendCatalog; + protected RESTCatalogAdapter adapterForRESTServer; Review Comment: Adapter could be introduced 2 ways and I wanted to call out that this is the adapter that lives in the REST server. The other option would be when the adapter mocks the REST server and the REST client directly gets the answers from the adapter without using the server. I can rename this to adapter but in TestRESTCatalog I found it useful to have this differentiation with the naming to understand better what adapter I use in the tests. ########## core/src/test/java/org/apache/iceberg/rest/TestBaseWithRESTServer.java: ########## @@ -0,0 +1,173 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.ObjectReader; +import java.io.File; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.nio.file.Path; +import java.util.Map; +import java.util.UUID; +import java.util.function.Consumer; +import org.apache.hadoop.conf.Configuration; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SessionCatalog; +import org.apache.iceberg.inmemory.InMemoryCatalog; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.rest.responses.ErrorResponse; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.handler.gzip.GzipHandler; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mockito; + +public abstract class TestBaseWithRESTServer { + private static final ObjectMapper MAPPER = RESTObjectMapper.mapper(); + + protected static final Namespace NS = Namespace.of("ns"); + protected static final SessionCatalog.SessionContext DEFAULT_SESSION_CONTEXT = + new SessionCatalog.SessionContext( + UUID.randomUUID().toString(), + "user", + ImmutableMap.of("credential", "user:12345"), + ImmutableMap.of()); + + protected InMemoryCatalog backendCatalog; + protected RESTCatalogAdapter adapterForRESTServer; + protected Server httpServer; + protected RESTCatalog restCatalog; + protected ParserContext parserContext; + + @TempDir private Path temp; + + @BeforeEach + public void setupCatalog() throws Exception { + File warehouse = temp.toFile(); + this.backendCatalog = new InMemoryCatalog(); + this.backendCatalog.initialize( + "in-memory", + ImmutableMap.of(CatalogProperties.WAREHOUSE_LOCATION, warehouse.getAbsolutePath())); + + adapterForRESTServer = createAdapterForServer(); + + ServletContextHandler servletContext = + new ServletContextHandler(ServletContextHandler.NO_SESSIONS); + servletContext.addServlet( + new ServletHolder(new RESTCatalogServlet(adapterForRESTServer)), "/*"); + servletContext.setHandler(new GzipHandler()); + + this.httpServer = new Server(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0)); + httpServer.setHandler(servletContext); + httpServer.start(); + + restCatalog = initCatalog(catalogName(), ImmutableMap.of()); + } + + @AfterEach + public void teardownCatalogs() throws Exception { Review Comment: changed to setup/teardown ########## core/src/test/java/org/apache/iceberg/rest/RESTRequestMatcher.java: ########## @@ -0,0 +1,82 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest; + +import static org.mockito.ArgumentMatchers.argThat; + +import java.util.Map; +import java.util.Objects; + +class RESTRequestMatcher { + private RESTRequestMatcher() {} + + public static HTTPRequest match(HTTPRequest.HTTPMethod method) { Review Comment: Done It's a bit weird with the contains headers version: `matchesContainsHeaders`, `matchesContainHeaders`, not sure -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
