Fix display empty string when json cell is null (#132)

* Fix display empty when json cell is null * Decrease output cells complexity * Imp coverage
michelin · May 31, 2024 · e2bdb8f · e2bdb8f
1 parent d2104a6
commit e2bdb8f
Show file tree

Hide file tree

Showing 6 changed files with 158 additions and 50 deletions.
diff --git a/src/main/java/com/michelin/kafkactl/model/format/AgoFormat.java b/src/main/java/com/michelin/kafkactl/model/format/AgoFormat.java
@@ -0,0 +1,34 @@
+package com.michelin.kafkactl.model.format;
+
+import static io.micronaut.core.util.StringUtils.EMPTY_STRING;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.util.StdDateFormat;
+import java.text.ParseException;
+import java.util.Date;
+import lombok.AllArgsConstructor;
+import org.ocpsoft.prettytime.PrettyTime;
+
+/**
+ * Ago format.
+ */
+@AllArgsConstructor
+public class AgoFormat implements OutputFormatStrategy {
+    private String jsonPointer;
+
+    @Override
+    public String display(JsonNode node) {
+        String output;
+        JsonNode cell = node.at(this.jsonPointer);
+
+        try {
+            StdDateFormat sdf = new StdDateFormat();
+            Date d = sdf.parse(cell.asText());
+            output = new PrettyTime().format(d);
+        } catch (ParseException e) {
+            output = EMPTY_STRING;
+        }
+
+        return output;
+    }
+}
diff --git a/src/main/java/com/michelin/kafkactl/model/format/DefaultFormat.java b/src/main/java/com/michelin/kafkactl/model/format/DefaultFormat.java
@@ -0,0 +1,33 @@
+package com.michelin.kafkactl.model.format;
+
+import static io.micronaut.core.util.StringUtils.EMPTY_STRING;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.JsonNodeType;
+import java.util.ArrayList;
+import java.util.List;
+import lombok.AllArgsConstructor;
+
+/**
+ * Default format.
+ */
+@AllArgsConstructor
+public class DefaultFormat implements OutputFormatStrategy {
+    private String jsonPointer;
+
+    @Override
+    public String display(JsonNode node) {
+        String output;
+        JsonNode cell = node.at(this.jsonPointer);
+
+        if (cell.isArray()) {
+            List<String> children = new ArrayList<>();
+            cell.elements().forEachRemaining(jsonNode -> children.add(jsonNode.asText()));
+            output = String.join(",", children);
+        } else {
+            output = cell.getNodeType().equals(JsonNodeType.NULL) ? EMPTY_STRING : cell.asText();
+        }
+
+        return output;
+    }
+}
diff --git a/src/main/java/com/michelin/kafkactl/model/format/OutputFormatStrategy.java b/src/main/java/com/michelin/kafkactl/model/format/OutputFormatStrategy.java
@@ -0,0 +1,10 @@
+package com.michelin.kafkactl.model.format;
+
+import com.fasterxml.jackson.databind.JsonNode;
+
+/**
+ * Output format strategy.
+ */
+public interface OutputFormatStrategy {
+    String display(JsonNode node);
+}
diff --git a/src/main/java/com/michelin/kafkactl/model/format/PeriodFormat.java b/src/main/java/com/michelin/kafkactl/model/format/PeriodFormat.java
@@ -0,0 +1,36 @@
+package com.michelin.kafkactl.model.format;
+
+import static io.micronaut.core.util.StringUtils.EMPTY_STRING;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import java.util.concurrent.TimeUnit;
+import lombok.AllArgsConstructor;
+
+/**
+ * Period format.
+ */
+@AllArgsConstructor
+public class PeriodFormat implements OutputFormatStrategy {
+    private String jsonPointer;
+
+    @Override
+    public String display(JsonNode node) {
+        String output;
+        JsonNode cell = node.at(this.jsonPointer);
+
+        try {
+            long ms = Long.parseLong(cell.asText());
+            long days = TimeUnit.MILLISECONDS.toDays(ms);
+            long hours = TimeUnit.MILLISECONDS.toHours(ms - TimeUnit.DAYS.toMillis(days));
+            long minutes = TimeUnit.MILLISECONDS.toMinutes(
+                ms - TimeUnit.DAYS.toMillis(days) - TimeUnit.HOURS.toMillis(hours));
+            output = days > 0 ? (days + "d") : "";
+            output += hours > 0 ? (hours + "h") : "";
+            output += minutes > 0 ? (minutes + "m") : "";
+        } catch (NumberFormatException e) {
+            output = EMPTY_STRING;
+        }
+
+        return output;
+    }
+}
diff --git a/src/main/java/com/michelin/kafkactl/service/FormatService.java b/src/main/java/com/michelin/kafkactl/service/FormatService.java
@@ -1,26 +1,23 @@
 package com.michelin.kafkactl.service;
 
-import static io.micronaut.core.util.StringUtils.EMPTY_STRING;
-
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.util.StdDateFormat;
 import com.michelin.kafkactl.config.KafkactlConfig;
 import com.michelin.kafkactl.model.Resource;
 import com.michelin.kafkactl.model.Status;
+import com.michelin.kafkactl.model.format.AgoFormat;
+import com.michelin.kafkactl.model.format.DefaultFormat;
+import com.michelin.kafkactl.model.format.OutputFormatStrategy;
+import com.michelin.kafkactl.model.format.PeriodFormat;
 import io.micronaut.core.annotation.ReflectiveAccess;
 import io.micronaut.core.naming.conventions.StringConvention;
 import io.micronaut.http.client.exceptions.HttpClientResponseException;
 import jakarta.inject.Inject;
 import jakarta.inject.Singleton;
-import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Date;
 import java.util.List;
 import java.util.Optional;
-import java.util.concurrent.TimeUnit;
-import org.ocpsoft.prettytime.PrettyTime;
 import org.yaml.snakeyaml.DumperOptions;
 import org.yaml.snakeyaml.Yaml;
 import org.yaml.snakeyaml.nodes.Tag;
@@ -234,63 +231,36 @@ public String toString() {
 
         static class PrettyTextTableColumn {
             private final String header;
-            private final String jsonPointer;
-            private final String transform;
             private final int indent;
             private int size = -1;
+            private OutputFormatStrategy outputFormat;
 
             public PrettyTextTableColumn(int indent, String... elements) {
                 this.header = elements[0];
                 this.indent = indent;
 
-                if (elements[1].contains("%")) {
-                    this.jsonPointer = elements[1].split("%")[0];
-                    this.transform = elements[1].split("%")[1];
+                String[] field = elements[1].split("%");
+                if (field.length > 1) {
+                    switch (field[1]) {
+                        case "AGO":
+                            this.outputFormat = new AgoFormat(field[0]);
+                            break;
+                        case "PERIOD":
+                            this.outputFormat = new PeriodFormat(field[0]);
+                            break;
+                        default:
+                            break;
+                    }
                 } else {
-                    this.jsonPointer = elements[1];
-                    this.transform = "NONE";
+                    this.outputFormat = new DefaultFormat(field[0]);
                 }
+
                 // Size should consider headers
                 this.size = Math.max(this.size, this.header.length() + indent);
             }
 
             public String transform(JsonNode node) {
-                String output;
-                JsonNode cell = node.at(this.jsonPointer);
-                switch (this.transform) {
-                    case "AGO" -> {
-                        try {
-                            StdDateFormat sdf = new StdDateFormat();
-                            Date d = sdf.parse(cell.asText());
-                            output = new PrettyTime().format(d);
-                        } catch (ParseException e) {
-                            output = EMPTY_STRING;
-                        }
-                    }
-                    case "PERIOD" -> {
-                        try {
-                            long ms = Long.parseLong(cell.asText());
-                            long days = TimeUnit.MILLISECONDS.toDays(ms);
-                            long hours = TimeUnit.MILLISECONDS.toHours(ms - TimeUnit.DAYS.toMillis(days));
-                            long minutes = TimeUnit.MILLISECONDS.toMinutes(
-                                ms - TimeUnit.DAYS.toMillis(days) - TimeUnit.HOURS.toMillis(hours));
-                            output = days > 0 ? (days + "d") : "";
-                            output += hours > 0 ? (hours + "h") : "";
-                            output += minutes > 0 ? (minutes + "m") : "";
-                        } catch (NumberFormatException e) {
-                            output = EMPTY_STRING;
-                        }
-                    }
-                    default -> {
-                        if (cell.isArray()) {
-                            List<String> children = new ArrayList<>();
-                            cell.elements().forEachRemaining(jsonNode -> children.add(jsonNode.asText()));
-                            output = String.join(",", children);
-                        } else {
-                            output = cell.asText();
-                        }
-                    }
-                }
+                String output = this.outputFormat.display(node);
                 // Check size for later
                 size = Math.max(size, output.length() + indent);
                 return output;

diff --git a/src/test/java/com/michelin/kafkactl/service/FormatServiceTest.java b/src/test/java/com/michelin/kafkactl/service/FormatServiceTest.java
@@ -53,6 +53,31 @@ void shouldDisplayListTable() {
         assertTrue(sw.toString().contains("prefix.topic  1m         delete"));
     }
 
+    @Test
+    void shouldDisplayEmptyInsteadOfNull() {
+        Resource resource = Resource.builder()
+            .kind("Topic")
+            .apiVersion("v1")
+            .metadata(Metadata.builder()
+                .name(null)
+                .creationTimestamp(Date.from(Instant.parse("2000-01-01T01:00:00.00Z")))
+                .build())
+            .spec(Map.of("configs",
+                Map.of("retention.ms", "60000",
+                    "cleanup.policy", "delete"
+                )))
+            .build();
+
+        CommandLine cmd = new CommandLine(new Kafkactl());
+        StringWriter sw = new StringWriter();
+        cmd.setOut(new PrintWriter(sw));
+
+        formatService.displayList("Topic", Collections.singletonList(resource), TABLE, cmd.getCommandSpec());
+
+        assertTrue(sw.toString().contains("TOPIC  RETENTION  POLICY  AGE"));
+        assertTrue(sw.toString().contains("       1m         delete"));
+    }
+
     @Test
     void shouldDisplayListTableEmptySpecs() {
         Resource resource = Resource.builder()