From 58af7c71b6e9ee22665fb3e9b812b90c6025de6a Mon Sep 17 00:00:00 2001 From: Scott Busche Date: Mon, 1 Apr 2024 22:12:26 -0500 Subject: [PATCH 1/6] Fix test on Windows shouldOkWhenConsumerGroupIsNotActive was returning a 400 instead of a 200 without the try with resources block, once I added it, the test consistently passes. --- .../io/kafbat/ui/KafkaConsumerGroupTests.java | 81 +++++++++---------- 1 file changed, 40 insertions(+), 41 deletions(-) diff --git a/api/src/test/java/io/kafbat/ui/KafkaConsumerGroupTests.java b/api/src/test/java/io/kafbat/ui/KafkaConsumerGroupTests.java index b7bd2dcb3..38698790d 100644 --- a/api/src/test/java/io/kafbat/ui/KafkaConsumerGroupTests.java +++ b/api/src/test/java/io/kafbat/ui/KafkaConsumerGroupTests.java @@ -10,7 +10,6 @@ import java.util.List; import java.util.Properties; import java.util.UUID; -import java.util.stream.Collectors; import java.util.stream.Stream; import lombok.extern.slf4j.Slf4j; import lombok.val; @@ -32,7 +31,6 @@ public class KafkaConsumerGroupTests extends AbstractIntegrationTest { @Test void shouldNotFoundWhenNoSuchConsumerGroupId() { String groupId = "groupA"; - String expError = "The group id does not exist"; webTestClient .delete() .uri("/api/clusters/{clusterName}/consumer-groups/{groupId}", LOCAL, groupId) @@ -47,12 +45,13 @@ void shouldOkWhenConsumerGroupIsNotActive() { //Create a consumer and subscribe to the topic String groupId = UUID.randomUUID().toString(); - val consumer = createTestConsumerWithGroupId(groupId); - consumer.subscribe(List.of(topicName)); - consumer.poll(Duration.ofMillis(100)); + try (val consumer = createTestConsumerWithGroupId(groupId)) { + consumer.subscribe(List.of(topicName)); + consumer.poll(Duration.ofMillis(100)); - //Unsubscribe from all topics to be able to delete this consumer - consumer.unsubscribe(); + //Unsubscribe from all topics to be able to delete this consumer + consumer.unsubscribe(); + } //Delete the consumer when it's INACTIVE and check webTestClient @@ -69,24 +68,24 @@ void shouldBeBadRequestWhenConsumerGroupIsActive() { //Create a consumer and subscribe to the topic String groupId = UUID.randomUUID().toString(); - val consumer = createTestConsumerWithGroupId(groupId); - consumer.subscribe(List.of(topicName)); - consumer.poll(Duration.ofMillis(100)); + try (val consumer = createTestConsumerWithGroupId(groupId)) { + consumer.subscribe(List.of(topicName)); + consumer.poll(Duration.ofMillis(100)); - //Try to delete the consumer when it's ACTIVE - String expError = "The group is not empty"; - webTestClient - .delete() - .uri("/api/clusters/{clusterName}/consumer-groups/{groupId}", LOCAL, groupId) - .exchange() - .expectStatus() - .isBadRequest(); + //Try to delete the consumer when it's ACTIVE + webTestClient + .delete() + .uri("/api/clusters/{clusterName}/consumer-groups/{groupId}", LOCAL, groupId) + .exchange() + .expectStatus() + .isBadRequest(); + } } @Test void shouldReturnConsumerGroupsWithPagination() throws Exception { - try (var groups1 = startConsumerGroups(3, "cgPageTest1"); - var groups2 = startConsumerGroups(2, "cgPageTest2")) { + try (var ignored = startConsumerGroups(3, "cgPageTest1"); + var ignored1 = startConsumerGroups(2, "cgPageTest2")) { webTestClient .get() .uri("/api/clusters/{clusterName}/consumer-groups/paged?perPage=3&search=cgPageTest", LOCAL) @@ -114,19 +113,19 @@ void shouldReturnConsumerGroupsWithPagination() throws Exception { }); webTestClient - .get() - .uri("/api/clusters/{clusterName}/consumer-groups/paged?perPage=10&&search" - + "=cgPageTest&orderBy=NAME&sortOrder=DESC", LOCAL) - .exchange() - .expectStatus() - .isOk() - .expectBody(ConsumerGroupsPageResponseDTO.class) - .value(page -> { - assertThat(page.getPageCount()).isEqualTo(1); - assertThat(page.getConsumerGroups().size()).isEqualTo(5); - assertThat(page.getConsumerGroups()) - .isSortedAccordingTo(Comparator.comparing(ConsumerGroupDTO::getGroupId).reversed()); - }); + .get() + .uri("/api/clusters/{clusterName}/consumer-groups/paged?perPage=10&&search" + + "=cgPageTest&orderBy=NAME&sortOrder=DESC", LOCAL) + .exchange() + .expectStatus() + .isOk() + .expectBody(ConsumerGroupsPageResponseDTO.class) + .value(page -> { + assertThat(page.getPageCount()).isEqualTo(1); + assertThat(page.getConsumerGroups().size()).isEqualTo(5); + assertThat(page.getConsumerGroups()) + .isSortedAccordingTo(Comparator.comparing(ConsumerGroupDTO::getGroupId).reversed()); + }); webTestClient .get() @@ -149,14 +148,14 @@ private Closeable startConsumerGroups(int count, String consumerGroupPrefix) { String topicName = createTopicWithRandomName(); var consumers = Stream.generate(() -> { - String groupId = consumerGroupPrefix + RandomStringUtils.randomAlphabetic(5); - val consumer = createTestConsumerWithGroupId(groupId); - consumer.subscribe(List.of(topicName)); - consumer.poll(Duration.ofMillis(100)); - return consumer; - }) - .limit(count) - .collect(Collectors.toList()); + String groupId = consumerGroupPrefix + RandomStringUtils.randomAlphabetic(5); + val consumer = createTestConsumerWithGroupId(groupId); + consumer.subscribe(List.of(topicName)); + consumer.poll(Duration.ofMillis(100)); + return consumer; + }) + .limit(count) + .toList(); return () -> { consumers.forEach(KafkaConsumer::close); deleteTopic(topicName); From d64328a6923f22744d2bbfdfced4cda4c6cf7a61 Mon Sep 17 00:00:00 2001 From: Scott Busche Date: Mon, 1 Apr 2024 22:36:52 -0500 Subject: [PATCH 2/6] Checkstyle --- .../io/kafbat/ui/KafkaConsumerGroupTests.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/api/src/test/java/io/kafbat/ui/KafkaConsumerGroupTests.java b/api/src/test/java/io/kafbat/ui/KafkaConsumerGroupTests.java index 38698790d..5f97317f2 100644 --- a/api/src/test/java/io/kafbat/ui/KafkaConsumerGroupTests.java +++ b/api/src/test/java/io/kafbat/ui/KafkaConsumerGroupTests.java @@ -148,14 +148,14 @@ private Closeable startConsumerGroups(int count, String consumerGroupPrefix) { String topicName = createTopicWithRandomName(); var consumers = Stream.generate(() -> { - String groupId = consumerGroupPrefix + RandomStringUtils.randomAlphabetic(5); - val consumer = createTestConsumerWithGroupId(groupId); - consumer.subscribe(List.of(topicName)); - consumer.poll(Duration.ofMillis(100)); - return consumer; - }) - .limit(count) - .toList(); + String groupId = consumerGroupPrefix + RandomStringUtils.randomAlphabetic(5); + val consumer = createTestConsumerWithGroupId(groupId); + consumer.subscribe(List.of(topicName)); + consumer.poll(Duration.ofMillis(100)); + return consumer; + }) + .limit(count) + .toList(); return () -> { consumers.forEach(KafkaConsumer::close); deleteTopic(topicName); From cbb84a373ed8e631ec71e1c8364b2eadf1df9edb Mon Sep 17 00:00:00 2001 From: Scott Busche Date: Tue, 2 Apr 2024 19:12:40 -0500 Subject: [PATCH 3/6] Update test to use OS specific line separators --- .../io/kafbat/ui/service/acl/AclCsvTest.java | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/api/src/test/java/io/kafbat/ui/service/acl/AclCsvTest.java b/api/src/test/java/io/kafbat/ui/service/acl/AclCsvTest.java index c6b725283..a9648f11c 100644 --- a/api/src/test/java/io/kafbat/ui/service/acl/AclCsvTest.java +++ b/api/src/test/java/io/kafbat/ui/service/acl/AclCsvTest.java @@ -6,6 +6,7 @@ import io.kafbat.ui.exception.ValidationException; import java.util.Collection; import java.util.List; +import java.util.stream.Stream; import org.apache.kafka.common.acl.AccessControlEntry; import org.apache.kafka.common.acl.AclBinding; import org.apache.kafka.common.acl.AclOperation; @@ -15,6 +16,8 @@ import org.apache.kafka.common.resource.ResourceType; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; class AclCsvTest { @@ -29,22 +32,26 @@ class AclCsvTest { ); @ParameterizedTest - @ValueSource(strings = { - "Principal,ResourceType, PatternType, ResourceName,Operation,PermissionType,Host\n" - + "User:test1,TOPIC,LITERAL,*,READ,ALLOW,*\n" - + "User:test2,GROUP,PREFIXED,group1,DESCRIBE,DENY,localhost", - - //without header - "User:test1,TOPIC,LITERAL,*,READ,ALLOW,*\n" - + "\n" - + "User:test2,GROUP,PREFIXED,group1,DESCRIBE,DENY,localhost" - + "\n" - }) + @MethodSource void parsesValidInputCsv(String csvString) { Collection parsed = AclCsv.parseCsv(csvString); assertThat(parsed).containsExactlyInAnyOrderElementsOf(TEST_BINDINGS); } + private static Stream parsesValidInputCsv() { + return Stream.of( + Arguments.of( + "Principal,ResourceType, PatternType, ResourceName,Operation,PermissionType,Host" + System.lineSeparator() + + "User:test1,TOPIC,LITERAL,*,READ,ALLOW,*" + System.lineSeparator() + + "User:test2,GROUP,PREFIXED,group1,DESCRIBE,DENY,localhost"), + Arguments.of( + //without header + "User:test1,TOPIC,LITERAL,*,READ,ALLOW,*" + System.lineSeparator() + + System.lineSeparator() + + "User:test2,GROUP,PREFIXED,group1,DESCRIBE,DENY,localhost" + + System.lineSeparator())); + } + @ParameterizedTest @ValueSource(strings = { // columns > 7 From 3d44395cfdb2494f8018385938b4f0f4c4549014 Mon Sep 17 00:00:00 2001 From: Scott Busche Date: Tue, 2 Apr 2024 19:19:18 -0500 Subject: [PATCH 4/6] Update another test for OS separators --- .../test/java/io/kafbat/ui/service/acl/AclsServiceTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/test/java/io/kafbat/ui/service/acl/AclsServiceTest.java b/api/src/test/java/io/kafbat/ui/service/acl/AclsServiceTest.java index 5f43f51cd..189e7c060 100644 --- a/api/src/test/java/io/kafbat/ui/service/acl/AclsServiceTest.java +++ b/api/src/test/java/io/kafbat/ui/service/acl/AclsServiceTest.java @@ -68,8 +68,8 @@ void testSyncAclWithAclCsv() { aclsService.syncAclWithAclCsv( CLUSTER, - "Principal,ResourceType, PatternType, ResourceName,Operation,PermissionType,Host\n" - + "User:test1,TOPIC,LITERAL,*,READ,ALLOW,*\n" + "Principal,ResourceType, PatternType, ResourceName,Operation,PermissionType,Host" + System.lineSeparator() + + "User:test1,TOPIC,LITERAL,*,READ,ALLOW,*" + System.lineSeparator() + "User:test3,GROUP,PREFIXED,groupNew,DESCRIBE,DENY,localhost" ).block(); From b1d829291e6ef3263f10ec5dc74e83cdce93d8ee Mon Sep 17 00:00:00 2001 From: Scott Busche Date: Tue, 2 Apr 2024 21:03:19 -0500 Subject: [PATCH 5/6] Resolve fun pathing issue on Windows Not replacing it, means `language/language.proto` is not found in the Map on Windows. Updating just the key, results in a duplicate file found with different path, as it still loads the import and from the directory pathing. --- .../java/io/kafbat/ui/serdes/builtin/ProtobufFileSerde.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/io/kafbat/ui/serdes/builtin/ProtobufFileSerde.java b/api/src/main/java/io/kafbat/ui/serdes/builtin/ProtobufFileSerde.java index e2fc105b9..8c0ec8369 100644 --- a/api/src/main/java/io/kafbat/ui/serdes/builtin/ProtobufFileSerde.java +++ b/api/src/main/java/io/kafbat/ui/serdes/builtin/ProtobufFileSerde.java @@ -408,7 +408,7 @@ private Map loadFilesWithLocations() { files.filter(p -> !Files.isDirectory(p) && p.toString().endsWith(".proto")) .forEach(path -> { // relative path will be used as "import" statement - String relativePath = baseLocation.relativize(path).toString(); + String relativePath = baseLocation.relativize(path).toString().replace("\\", "/"); var protoFileElement = ProtoParser.Companion.parse( Location.get(baseLocation.toString(), relativePath), readFileAsString(path) From a61a6c1d1f32d390d8c053ffe472182461924d2d Mon Sep 17 00:00:00 2001 From: Roman Zabaluev Date: Sun, 19 May 2024 22:30:58 +0300 Subject: [PATCH 6/6] Revert unnecessary changes --- .../java/io/kafbat/ui/serdes/builtin/ProtobufFileSerde.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/io/kafbat/ui/serdes/builtin/ProtobufFileSerde.java b/api/src/main/java/io/kafbat/ui/serdes/builtin/ProtobufFileSerde.java index 8c0ec8369..e2fc105b9 100644 --- a/api/src/main/java/io/kafbat/ui/serdes/builtin/ProtobufFileSerde.java +++ b/api/src/main/java/io/kafbat/ui/serdes/builtin/ProtobufFileSerde.java @@ -408,7 +408,7 @@ private Map loadFilesWithLocations() { files.filter(p -> !Files.isDirectory(p) && p.toString().endsWith(".proto")) .forEach(path -> { // relative path will be used as "import" statement - String relativePath = baseLocation.relativize(path).toString().replace("\\", "/"); + String relativePath = baseLocation.relativize(path).toString(); var protoFileElement = ProtoParser.Companion.parse( Location.get(baseLocation.toString(), relativePath), readFileAsString(path)