From afe85d9b4434f607724cc1067d244f4f846b2b35 Mon Sep 17 00:00:00 2001 From: Tim Fox Date: Wed, 8 Jul 2020 07:22:19 +0100 Subject: [PATCH] fix: Make sure UDTF describe shows actual function description (#5744) --- .../java/io/confluent/ksql/cli/CliTest.java | 6 ++--- .../confluent/ksql/function/UdtfLoader.java | 2 +- .../ksql/rest/server/TemporaryEngine.java | 4 ++-- .../DescribeFunctionExecutorTest.java | 24 +++++++++++++++++-- 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/ksqldb-cli/src/test/java/io/confluent/ksql/cli/CliTest.java b/ksqldb-cli/src/test/java/io/confluent/ksql/cli/CliTest.java index c8cf8801575b..93bdd0c733d3 100644 --- a/ksqldb-cli/src/test/java/io/confluent/ksql/cli/CliTest.java +++ b/ksqldb-cli/src/test/java/io/confluent/ksql/cli/CliTest.java @@ -896,13 +896,11 @@ public void shouldDescribeTableFunction() { // variations for Udfs are loaded non-deterministically. Don't assume which variation is first String expectedVariation = "\tVariation : EXPLODE(list ARRAY)\n" - + "\tReturns : T\n" - + "\tDescription : Explodes an array. This function outputs one value for each element of the array."; + + "\tReturns : T"; assertThat(outputString, containsString(expectedVariation)); expectedVariation = "\tVariation : EXPLODE(input ARRAY)\n" - + "\tReturns : DECIMAL\n" - + "\tDescription : Explodes an array. This function outputs one value for each element of the array."; + + "\tReturns : DECIMAL"; assertThat(outputString, containsString(expectedVariation)); } diff --git a/ksqldb-engine/src/main/java/io/confluent/ksql/function/UdtfLoader.java b/ksqldb-engine/src/main/java/io/confluent/ksql/function/UdtfLoader.java index c82eb6640d3c..7d818a7db44b 100644 --- a/ksqldb-engine/src/main/java/io/confluent/ksql/function/UdtfLoader.java +++ b/ksqldb-engine/src/main/java/io/confluent/ksql/function/UdtfLoader.java @@ -105,7 +105,7 @@ public void loadUdtfFromClass( final KsqlTableFunction tableFunction = createTableFunction(method, FunctionName.of(functionName), returnType, parameters, - udtfDescriptionAnnotation.description(), + annotation.description(), annotation ); factory.addFunction(tableFunction); diff --git a/ksqldb-rest-app/src/test/java/io/confluent/ksql/rest/server/TemporaryEngine.java b/ksqldb-rest-app/src/test/java/io/confluent/ksql/rest/server/TemporaryEngine.java index 3dbe15ccb9d1..b3dd8fd72b6f 100644 --- a/ksqldb-rest-app/src/test/java/io/confluent/ksql/rest/server/TemporaryEngine.java +++ b/ksqldb-rest-app/src/test/java/io/confluent/ksql/rest/server/TemporaryEngine.java @@ -192,12 +192,12 @@ public ServiceContext getServiceContext() { @UdtfDescription(name = "test_udtf1", description = "test_udtf1 description") public static class TestUdtf1 { - @Udtf + @Udtf(description = "test_udtf1 int") public List foo1(@UdfParameter(value = "foo") final int foo) { return ImmutableList.of(1); } - @Udtf + @Udtf(description = "test_udtf1 double") public List foo2(@UdfParameter(value = "foo") final double foo) { return ImmutableList.of(1.0d); } diff --git a/ksqldb-rest-app/src/test/java/io/confluent/ksql/rest/server/execution/DescribeFunctionExecutorTest.java b/ksqldb-rest-app/src/test/java/io/confluent/ksql/rest/server/execution/DescribeFunctionExecutorTest.java index b5cdca7b3581..afdd89c9ddaa 100644 --- a/ksqldb-rest-app/src/test/java/io/confluent/ksql/rest/server/execution/DescribeFunctionExecutorTest.java +++ b/ksqldb-rest-app/src/test/java/io/confluent/ksql/rest/server/execution/DescribeFunctionExecutorTest.java @@ -16,12 +16,17 @@ package io.confluent.ksql.rest.server.execution; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasSize; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import io.confluent.ksql.rest.SessionProperties; +import io.confluent.ksql.rest.entity.ArgumentInfo; import io.confluent.ksql.rest.entity.FunctionDescriptionList; +import io.confluent.ksql.rest.entity.FunctionInfo; import io.confluent.ksql.rest.entity.FunctionType; import io.confluent.ksql.rest.server.TemporaryEngine; +import java.util.Arrays; import org.hamcrest.Description; import org.hamcrest.TypeSafeMatcher; import org.junit.Rule; @@ -102,8 +107,8 @@ public void shouldDescribeUDTF() { assertThat(functionList, new TypeSafeMatcher() { @Override protected boolean matchesSafely(final FunctionDescriptionList item) { - return functionList.getName().equals("TEST_UDTF1") - && functionList.getType().equals(FunctionType.TABLE); + return item.getName().equals("TEST_UDTF1") + && item.getType().equals(FunctionType.TABLE); } @Override @@ -111,6 +116,21 @@ public void describeTo(final Description description) { description.appendText(functionList.getName()); } }); + + assertThat(functionList.getFunctions(), hasSize(2)); + + FunctionInfo expected1 = new FunctionInfo( + Arrays.asList(new ArgumentInfo("foo", "INT", "", false)), + "INT", "test_udtf1 int"); + + assertTrue(functionList.getFunctions().contains(expected1)); + + FunctionInfo expected2 = new FunctionInfo( + Arrays.asList(new ArgumentInfo("foo", "DOUBLE", "", false)), + "DOUBLE", "test_udtf1 double"); + + assertTrue(functionList.getFunctions().contains(expected2)); + } }