C0urante commented on code in PR #13434:
URL: https://github.com/apache/kafka/pull/13434#discussion_r1153463147


##########
connect/runtime/src/test/java/org/apache/kafka/connect/integration/OffsetsApiIntegrationTest.java:
##########
@@ -0,0 +1,269 @@
+/*
+ * 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.kafka.connect.integration;
+
+import org.apache.kafka.clients.CommonClientConfigs;
+import org.apache.kafka.connect.runtime.ConnectorConfig;
+import org.apache.kafka.connect.runtime.SourceConnectorConfig;
+import org.apache.kafka.connect.runtime.rest.entities.ConnectorOffset;
+import org.apache.kafka.connect.runtime.rest.entities.ConnectorOffsets;
+import org.apache.kafka.connect.runtime.rest.errors.ConnectRestException;
+import org.apache.kafka.connect.storage.StringConverter;
+import org.apache.kafka.connect.util.SinkUtils;
+import org.apache.kafka.connect.util.clusters.EmbeddedConnectCluster;
+import org.apache.kafka.connect.util.clusters.EmbeddedKafkaCluster;
+import org.apache.kafka.test.IntegrationTest;
+import org.apache.kafka.test.TestUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+import static 
org.apache.kafka.connect.integration.MonitorableSourceConnector.TOPIC_CONFIG;
+import static 
org.apache.kafka.connect.runtime.ConnectorConfig.CONNECTOR_CLASS_CONFIG;
+import static 
org.apache.kafka.connect.runtime.ConnectorConfig.TASKS_MAX_CONFIG;
+import static 
org.apache.kafka.connect.runtime.SinkConnectorConfig.TOPICS_CONFIG;
+import static 
org.apache.kafka.connect.runtime.TopicCreationConfig.DEFAULT_TOPIC_CREATION_PREFIX;
+import static 
org.apache.kafka.connect.runtime.TopicCreationConfig.PARTITIONS_CONFIG;
+import static 
org.apache.kafka.connect.runtime.TopicCreationConfig.REPLICATION_FACTOR_CONFIG;
+import static 
org.apache.kafka.connect.runtime.WorkerConfig.KEY_CONVERTER_CLASS_CONFIG;
+import static 
org.apache.kafka.connect.runtime.WorkerConfig.OFFSET_COMMIT_INTERVAL_MS_CONFIG;
+import static 
org.apache.kafka.connect.runtime.WorkerConfig.VALUE_CONVERTER_CLASS_CONFIG;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Integration tests for Kafka Connect's connector offset management REST APIs
+ */
+@Category(IntegrationTest.class)
+public class OffsetsApiIntegrationTest {
+
+    private static final String CONNECTOR_NAME = "test-connector";
+    private static final String TOPIC = "test-topic";
+    private static final Integer NUM_TASKS = 2;
+    private static final long OFFSET_COMMIT_INTERVAL_MS = 
TimeUnit.SECONDS.toMillis(1);
+    private static final int NUM_WORKERS = 3;
+    private EmbeddedConnectCluster connect;
+
+    @Before
+    public void setup() {
+        // setup Connect worker properties
+        Map<String, String> workerProps = new HashMap<>();
+        workerProps.put(OFFSET_COMMIT_INTERVAL_MS_CONFIG, 
String.valueOf(OFFSET_COMMIT_INTERVAL_MS));
+
+        // build a Connect cluster backed by Kafka and Zk
+        connect = new EmbeddedConnectCluster.Builder()
+                .name("connect-cluster")
+                .numWorkers(NUM_WORKERS)
+                .workerProps(workerProps)
+                .build();
+        connect.start();
+    }
+
+    @After
+    public void tearDown() {
+        connect.stop();
+    }
+
+    @Test
+    public void testGetNonExistentConnectorOffsets() {
+        ConnectRestException e = assertThrows(ConnectRestException.class,
+                () -> connect.connectorOffsets("non-existent-connector"));
+        assertEquals(404, e.errorCode());
+    }
+
+    @Test
+    public void testGetSinkConnectorOffsets() throws Exception {
+        getAndVerifySinkConnectorOffsets(baseSinkConnectorConfigs(), 
connect.kafka());
+    }
+
+    @Test
+    public void testGetSinkConnectorOffsetsOverriddenConsumerGroupId() throws 
Exception {
+        Map<String, String> connectorConfigs = baseSinkConnectorConfigs();
+        
connectorConfigs.put(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX 
+ CommonClientConfigs.GROUP_ID_CONFIG,

Review Comment:
   The new test looks great, really nice balance of additional guarantees 
without cluttering things up with assertions for a different feature 👍



##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java:
##########
@@ -1722,6 +1727,128 @@ public void testZombieFencing() {
         verifyGenericIsolation();
     }
 
+    @Test
+    @SuppressWarnings("unchecked")
+    public void testGetSinkConnectorOffsets() throws Exception {
+        mockKafkaClusterId();
+
+        String connectorClass = SampleSinkConnector.class.getName();
+        connectorProps.put(CONNECTOR_CLASS_CONFIG, connectorClass);
+
+        worker = new Worker(WORKER_ID, new MockTime(), plugins, config, 
offsetBackingStore, executorService,
+                allConnectorClientConfigOverridePolicy);
+        worker.start();
+
+        Map<TopicPartition, OffsetAndMetadata> consumerGroupOffsets =
+                Collections.singletonMap(new TopicPartition("test-topic", 0), 
new OffsetAndMetadata(10));
+        Map<String, Map<TopicPartition, OffsetAndMetadata>> 
consumerGroupToOffsetsMap =
+                
Collections.singletonMap(SinkUtils.consumerGroupId(CONNECTOR_ID), 
consumerGroupOffsets);
+
+        Admin admin = mock(Admin.class);
+        ListConsumerGroupOffsetsResult result = 
mock(ListConsumerGroupOffsetsResult.class);
+        when(admin.listConsumerGroupOffsets(anyString(), 
any(ListConsumerGroupOffsetsOptions.class))).thenReturn(result);
+        KafkaFuture<Map<String, Map<TopicPartition, OffsetAndMetadata>>> 
adminFuture = mock(KafkaFuture.class);
+        when(result.all()).thenReturn(adminFuture);
+        when(adminFuture.whenComplete(any())).thenAnswer(invocation -> {
+            ((KafkaFuture.BiConsumer<Map<String, Map<TopicPartition, 
OffsetAndMetadata>>, Throwable>) invocation.getArgument(0))
+                    .accept(consumerGroupToOffsetsMap, null);
+            return null;
+        });
+
+        FutureCallback<ConnectorOffsets> cb = new FutureCallback<>();
+        worker.sinkConnectorOffsets(CONNECTOR_ID, sinkConnector, 
connectorProps, cb, config -> admin);
+        ConnectorOffsets offsets = cb.get(1000, TimeUnit.MILLISECONDS);
+
+        assertEquals(1, offsets.offsets().size());
+        assertEquals(10L, 
offsets.offsets().get(0).offset().get(SinkUtils.KAFKA_OFFSET_KEY));
+        assertEquals(0, 
offsets.offsets().get(0).partition().get(SinkUtils.KAFKA_PARTITION_KEY));
+        assertEquals("test-topic", 
offsets.offsets().get(0).partition().get(SinkUtils.KAFKA_TOPIC_KEY));
+
+        
verify(admin).listConsumerGroupOffsets(eq(SinkUtils.consumerGroupId(CONNECTOR_ID)),
 any(ListConsumerGroupOffsetsOptions.class));
+        verify(admin).close();
+        verifyKafkaClusterId();
+    }
+
+    @Test
+    @SuppressWarnings("unchecked")
+    public void testGetSinkConnectorOffsetsOverriddenConsumerGroupId() throws 
Exception {
+        mockKafkaClusterId();
+
+        String connectorClass = SampleSinkConnector.class.getName();
+        connectorProps.put(CONNECTOR_CLASS_CONFIG, connectorClass);
+        
connectorProps.put(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX + 
CommonClientConfigs.GROUP_ID_CONFIG, "overridden-group-id");
+
+        worker = new Worker(WORKER_ID, new MockTime(), plugins, config, 
offsetBackingStore, executorService,
+                allConnectorClientConfigOverridePolicy);
+        worker.start();
+
+        Map<String, Map<TopicPartition, OffsetAndMetadata>> 
consumerGroupToOffsetsMap = new HashMap<>();
+        consumerGroupToOffsetsMap.put("overridden-group-id", 
Collections.singletonMap(new TopicPartition("test-topic", 0), new 
OffsetAndMetadata(10)));
+        consumerGroupToOffsetsMap.put(SinkUtils.consumerGroupId(CONNECTOR_ID),
+                Collections.singletonMap(new TopicPartition("test-topic-2", 
1), new OffsetAndMetadata(0)));
+
+        Admin admin = mock(Admin.class);
+        ListConsumerGroupOffsetsResult result = 
mock(ListConsumerGroupOffsetsResult.class);
+        when(admin.listConsumerGroupOffsets(anyString(), 
any(ListConsumerGroupOffsetsOptions.class))).thenReturn(result);
+        KafkaFuture<Map<String, Map<TopicPartition, OffsetAndMetadata>>> 
adminFuture = mock(KafkaFuture.class);
+        when(result.all()).thenReturn(adminFuture);
+        when(adminFuture.whenComplete(any())).thenAnswer(invocation -> {
+            ((KafkaFuture.BiConsumer<Map<String, Map<TopicPartition, 
OffsetAndMetadata>>, Throwable>) invocation.getArgument(0))
+                    .accept(consumerGroupToOffsetsMap, null);
+            return null;
+        });
+
+        FutureCallback<ConnectorOffsets> cb = new FutureCallback<>();
+        worker.sinkConnectorOffsets(CONNECTOR_ID, sinkConnector, 
connectorProps, cb, config -> admin);
+        ConnectorOffsets offsets = cb.get(1000, TimeUnit.MILLISECONDS);
+
+        assertEquals(1, offsets.offsets().size());
+        assertEquals(10L, 
offsets.offsets().get(0).offset().get(SinkUtils.KAFKA_OFFSET_KEY));
+        assertEquals(0, 
offsets.offsets().get(0).partition().get(SinkUtils.KAFKA_PARTITION_KEY));
+        assertEquals("test-topic", 
offsets.offsets().get(0).partition().get(SinkUtils.KAFKA_TOPIC_KEY));
+
+        verify(admin).listConsumerGroupOffsets(eq("overridden-group-id"), 
any(ListConsumerGroupOffsetsOptions.class));
+        verify(admin).close();
+        verifyKafkaClusterId();
+    }
+
+    @Test
+    public void testGetSourceConnectorOffsets() throws Exception {

Review Comment:
   Looks great, thanks 👍



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to