feat(doc) Documentation now generated with required props first & Not…

…Null on interface methods displayed (#1211) * feat: Doc generation now takes the NotNull field from interface methods * feat: Required props are now displayed first in UI
kestra-io · May 5, 2023 · bd968c3 · bd968c3
1 parent 44f1168
commit bd968c3
Show file tree

Hide file tree

Showing 8 changed files with 58 additions and 19 deletions.
diff --git a/cli/src/test/java/io/kestra/cli/commands/plugins/PluginDocCommandTest.java b/cli/src/test/java/io/kestra/cli/commands/plugins/PluginDocCommandTest.java
@@ -36,8 +36,8 @@ void run() throws IOException, URISyntaxException {
 
         FileUtils.copyFile(
             new File(Objects.requireNonNull(PluginListCommandTest.class.getClassLoader()
-                .getResource("plugins/plugin-template-test-0.6.0-SNAPSHOT.jar")).toURI()),
-            new File(URI.create("file://" + pluginsPath.toAbsolutePath() + "/plugin-template-test-0.6.0-SNAPSHOT.jar"))
+                .getResource("plugins/plugin-template-test-0.9.0-SNAPSHOT.jar")).toURI()),
+            new File(URI.create("file://" + pluginsPath.toAbsolutePath() + "/plugin-template-test-0.9.0-SNAPSHOT.jar"))
         );
 
         Path docPath = Files.createTempDirectory(PluginInstallCommandTest.class.getSimpleName());
@@ -84,7 +84,8 @@ void run() throws IOException, URISyntaxException {
 
             // check @PluginProperty from an interface
             var task = directory.toPath().resolve("tasks/io.kestra.plugin.templates.ExampleTask.md");
-            assertThat(new String(Files.readAllBytes(task)), containsString("### `example`\n" +
+            String taskDoc = new String(Files.readAllBytes(task));
+            assertThat(taskDoc, containsString("### `example`\n" +
                 "\n" +
                 "* **Type:** ==string==\n" +
                 "* **Dynamic:** ✔️\n" +
@@ -93,6 +94,16 @@ void run() throws IOException, URISyntaxException {
                 "\n" +
                 "\n" +
                 "> Example interface\n"));
+            assertThat(taskDoc, containsString("### `requiredExample`\n" +
+                "\n" +
+                "* **Type:** ==string==\n" +
+                "* **Dynamic:** ❌\n" +
+                "* **Required:** ✔️\n" +
+                "* **Min length:** `1`\n" +
+                "\n" +
+                "\n" +
+                "\n" +
+                "> Required Example interface\n"));
 
             var authenticationGuide = directory.toPath().resolve("guides/authentication.md");
             assertThat(new String(Files.readAllBytes(authenticationGuide)), containsString("This is how to authenticate for this plugin:"));

diff --git a/cli/src/test/java/io/kestra/cli/commands/plugins/PluginListCommandTest.java b/cli/src/test/java/io/kestra/cli/commands/plugins/PluginListCommandTest.java
@@ -36,8 +36,8 @@ void run() throws IOException, URISyntaxException {
 
         FileUtils.copyFile(
             new File(Objects.requireNonNull(PluginListCommandTest.class.getClassLoader()
-                .getResource("plugins/plugin-template-test-0.6.0-SNAPSHOT.jar")).toURI()),
-            new File(URI.create("file://" + pluginsPath.toAbsolutePath() + "/plugin-template-test-0.6.0-SNAPSHOT.jar"))
+                .getResource("plugins/plugin-template-test-0.9.0-SNAPSHOT.jar")).toURI()),
+            new File(URI.create("file://" + pluginsPath.toAbsolutePath() + "/plugin-template-test-0.9.0-SNAPSHOT.jar"))
         );
 
         ByteArrayOutputStream out = new ByteArrayOutputStream();

diff --git a/...s/plugin-template-test-0.6.0-SNAPSHOT.jar → ...s/plugin-template-test-0.9.0-SNAPSHOT.jar b/...s/plugin-template-test-0.6.0-SNAPSHOT.jar → ...s/plugin-template-test-0.9.0-SNAPSHOT.jar
diff --git a/core/src/main/java/io/kestra/core/docs/ClassPluginDocumentation.java b/core/src/main/java/io/kestra/core/docs/ClassPluginDocumentation.java
@@ -125,7 +125,13 @@ private static Map<String, Object> flatten(Map<String, Object> map, List<String>
 
     @SuppressWarnings("unchecked")
     private static Map<String, Object> flatten(Map<String, Object> map, List<String> required, String parentName) {
-        Map<String, Object> result = new TreeMap<>();
+        Map<String, Object> result = new TreeMap<>((key1, key2) -> {
+            boolean key1Required = required.contains(key1);
+            boolean key2Required = required.contains(key2);
+            if(key1Required == key2Required) return key1.compareTo(key2);
+
+            return key1Required ? -1 : 1;
+        });
 
         for (Map.Entry<String, Object> current : map.entrySet()) {
             Map<String, Object> finalValue = (Map<String, Object>) current.getValue();

diff --git a/core/src/main/java/io/kestra/core/docs/DocumentationGenerator.java b/core/src/main/java/io/kestra/core/docs/DocumentationGenerator.java
@@ -128,6 +128,7 @@ private static List<Document> guides(RegisteredPlugin plugin) throws IOException
                 try (var stream = Files.walk(root, 1)) {
                     return stream
                         .skip(1) // first element is the root element
+                        .sorted(Comparator.comparing(path -> path.getName(path.getParent().getNameCount()).toString()))
                         .map(throwFunction(path -> new Document(
                             pluginName + "/guides/" + path.getName(path.getParent().getNameCount()),
                             new String(Files.readAllBytes(path)),

diff --git a/core/src/main/java/io/kestra/core/docs/JsonSchemaGenerator.java b/core/src/main/java/io/kestra/core/docs/JsonSchemaGenerator.java
@@ -175,6 +175,7 @@ protected <T> void build(SchemaGeneratorConfigBuilder builder, Class<? extends T
         builder
 
             .with(new JavaxValidationModule(
+                JavaxValidationOption.NOT_NULLABLE_METHOD_IS_REQUIRED,
                 JavaxValidationOption.NOT_NULLABLE_FIELD_IS_REQUIRED,
                 JavaxValidationOption.INCLUDE_PATTERN_EXPRESSIONS
             ))

diff --git a/core/src/main/java/io/kestra/core/plugins/PluginScanner.java b/core/src/main/java/io/kestra/core/plugins/PluginScanner.java
@@ -1,6 +1,5 @@
 package io.kestra.core.plugins;
 
-import io.kestra.core.docs.Document;
 import io.kestra.core.models.conditions.Condition;
 import io.kestra.core.models.tasks.Task;
 import io.kestra.core.models.triggers.AbstractTrigger;
@@ -20,16 +19,11 @@
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;
 import java.util.jar.JarFile;
 import java.util.jar.Manifest;
 import java.util.stream.Collectors;
 
-import static io.kestra.core.utils.Rethrow.throwFunction;
-
 @Slf4j
 public class PluginScanner {
     ClassLoader parent;
@@ -146,6 +140,7 @@ private RegisteredPlugin scanClassLoader(final ClassLoader classLoader, External
                 try (var stream = Files.walk(root, 1)) {
                     stream
                         .skip(1) // first element is the root element
+                        .sorted(Comparator.comparing(path -> path.getName(path.getParent().getNameCount()).toString()))
                         .forEach(guide -> {
                             var guideName = guide.getName(guide.getParent().getNameCount()).toString();
                             guides.add(guideName.substring(0, guideName.lastIndexOf('.')));

diff --git a/core/src/test/java/io/kestra/core/docs/DocumentationGeneratorTest.java b/core/src/test/java/io/kestra/core/docs/DocumentationGeneratorTest.java
@@ -1,23 +1,25 @@
 package io.kestra.core.docs;
 
+import io.kestra.core.models.tasks.Task;
+import io.kestra.core.plugins.PluginScanner;
+import io.kestra.core.plugins.RegisteredPlugin;
 import io.kestra.core.tasks.debugs.Echo;
 import io.kestra.core.tasks.debugs.Return;
 import io.kestra.core.tasks.flows.Flow;
+import io.kestra.core.tasks.scripts.Bash;
 import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
 import org.junit.jupiter.api.Test;
-import io.kestra.core.models.tasks.Task;
-import io.kestra.core.plugins.PluginScanner;
-import io.kestra.core.plugins.RegisteredPlugin;
-import io.kestra.core.tasks.scripts.Bash;
 
 import java.io.IOException;
 import java.net.URISyntaxException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
-
-import jakarta.inject.Inject;
+import java.util.stream.Stream;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
@@ -61,6 +63,29 @@ void bash() throws IOException {
 
         assertThat(render, containsString("Bash"));
         assertThat(render, containsString("**Required:** ✔️"));
+
+        assertThat(render, containsString("`exitOnFailed`"));
+
+        int propertiesIndex = render.indexOf("Properties");
+        int outputsIndex = render.indexOf("Outputs");
+        int definitionsIndex = render.indexOf("Definitions");
+
+        assertRequiredPropsAreFirst(render.substring(propertiesIndex, outputsIndex));
+        assertRequiredPropsAreFirst(render.substring(outputsIndex, definitionsIndex));
+
+        String definitionsDoc = render.substring(definitionsIndex);
+        Arrays.stream(definitionsDoc.split("[^#]### "))
+            // first is 'Definitions' header
+            .skip(1)
+                .forEach(DocumentationGeneratorTest::assertRequiredPropsAreFirst);
+    }
+
+    private static void assertRequiredPropsAreFirst(String propertiesDoc) {
+        int lastRequiredPropIndex = propertiesDoc.lastIndexOf("* **Required:** ✔️");
+        int firstOptionalPropIndex = propertiesDoc.indexOf("* **Required:** ❌");
+        if (lastRequiredPropIndex != -1 && firstOptionalPropIndex != -1) {
+            assertThat(lastRequiredPropIndex, lessThanOrEqualTo(firstOptionalPropIndex));
+        }
     }
 
     @SuppressWarnings({"rawtypes", "unchecked"})