simpler solution and improvements

Author: szybia

server/src/main/java/org/elasticsearch/cluster/metadata/LifecycleExecutionState.java

@@ -291,12 +291,19 @@ public static String potentiallyTruncateLongJsonWithExplanation(String json) {
         if (json == null) {
             return null;
         }
-        // Avoid no-op truncations: we must append `"}` (2 chars) to the end to keep JSON valid
-        if (json.length() <= MAXIMUM_STEP_INFO_STRING_LENGTH + 2) {
+        if (json.length() <= MAXIMUM_STEP_INFO_STRING_LENGTH) {
             return json;
         }
-        final int actuallyRemovedCharacters = json.length() - MAXIMUM_STEP_INFO_STRING_LENGTH - 2;
-        return Strings.cleanTruncate(json, MAXIMUM_STEP_INFO_STRING_LENGTH) + "... (" + actuallyRemovedCharacters + " chars truncated)\"}";
+        assert json.startsWith("{\"") && json.endsWith("\"}") : "expected more specific JSON format";
+        // we'll now do our best to truncate the JSON and have the result be valid JSON.
+        // a long input JSON should generally only be possible with an exception turned into JSON that's well-formatted.
+        // if we fail to produce valid JSON, we might return invalid JSON in REST API in niche cases, so this isn't catastrophic.
+        final int removedChars = json.length() - MAXIMUM_STEP_INFO_STRING_LENGTH;
+        // -2 to truncate characters within the JSON (before `"}`, which we add back in, so wouldn't actually be truncating anything),
+        // and to avoid invalid JSON if we truncate one character (result would be `"... (1 chars truncated)"}`,
+        // this adds a dangling double-quote).
+        // we don't aim to arrive at a string of exactly MAXIMUM_STEP_INFO_STRING_LENGTH, that's just the condition to apply truncation.
+        return Strings.cleanTruncate(json, MAXIMUM_STEP_INFO_STRING_LENGTH - 2) + "... (" + removedChars + " chars truncated)\"}";
     }
 
     public static class Builder {

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecycleExecutionStateTests.java

@@ -29,13 +29,15 @@ public void testTruncatingStepInfo() {
         Map<String, String> custom = createCustomMetadata();
         LifecycleExecutionState state = LifecycleExecutionState.fromCustomMetadata(custom);
         assertThat(custom.get("step_info"), equalTo(state.stepInfo()));
-        String longStepInfo = randomAlphanumericOfLength(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH + 100);
+        String longStepInfo = "{\"key\": \""
+            + randomAlphanumericOfLength(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH + 100)
+            + "\"}";
         LifecycleExecutionState newState = LifecycleExecutionState.builder(state).setStepInfo(longStepInfo).build();
         // Length includes the post suffix (`... (X chars truncated)`)
-        assertThat(newState.stepInfo().length(), equalTo(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH + 26));
+        assertThat(newState.stepInfo().length(), equalTo(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH + 25));
         assertThat(
-            newState.stepInfo().substring(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH, 1050),
-            equalTo("... (98 chars truncated)\"}")
+            newState.stepInfo().substring(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH - 2, 1049),
+            equalTo("... (111 chars truncated)\"}")
         );
     }
 
@@ -147,19 +149,14 @@ public void testGetCurrentStepKey() {
     }
 
     public void testPotentiallyTruncateLongJsonWithExplanationNotTruncated() {
-        // +2 because with the JSON ending in `"}`, we'll add it back anyway so it's a NOP
-        final String input = randomAlphaOfLengthBetween(0, LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH + 2);
+        final String input = randomAlphaOfLengthBetween(0, LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH);
         assertSame(input, LifecycleExecutionState.potentiallyTruncateLongJsonWithExplanation(input));
     }
 
     public void testPotentiallyTruncateLongJsonWithExplanationOneCharTruncated() {
         final String jsonBaseFormat = "{\"key\": \"%s\"}";
         final int baseLength = Strings.format(jsonBaseFormat, "").length();
-        final String value = randomAlphanumericOfLength(
-            // +3 because +1 and +2 are no-ops, they will truncate the end of JSON `"}` and then add it back,
-            // so we need another char to truncate.
-            LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH - baseLength + 3
-        );
+        final String value = randomAlphanumericOfLength(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH - baseLength + 1);
         final String input = Strings.format(jsonBaseFormat, value);
         final String expectedOutput = Strings.format(jsonBaseFormat, value.substring(0, value.length() - 1) + "... (1 chars truncated)");
         assertEquals(expectedOutput, LifecycleExecutionState.potentiallyTruncateLongJsonWithExplanation(input));
@@ -168,11 +165,7 @@ public void testPotentiallyTruncateLongJsonWithExplanationOneCharTruncated() {
     public void testPotentiallyTruncateLongJsonWithExplanationTwoCharsTruncated() {
         final String jsonBaseFormat = "{\"key\": \"%s\"}";
         final int baseLength = Strings.format(jsonBaseFormat, "").length();
-        final String value = randomAlphanumericOfLength(
-            // +4 because +1 and +2 are no-ops, they will truncate the end of JSON `"}` and then add it back,
-            // so we need another two chars to truncate.
-            LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH - baseLength + 4
-        );
+        final String value = randomAlphanumericOfLength(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH - baseLength + 2);
         final String input = Strings.format(jsonBaseFormat, value);
         final String expectedOutput = Strings.format(jsonBaseFormat, value.substring(0, value.length() - 2) + "... (2 chars truncated)");
         assertEquals(expectedOutput, LifecycleExecutionState.potentiallyTruncateLongJsonWithExplanation(input));

x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportExplainLifecycleActionTests.java

@@ -292,14 +292,14 @@ public void testPreviousStepInfoTruncationDoesNotBreakExplainJson() throws Excep
 
             final String expectedPreviousStepInfoFormat = """
                 {"type":"illegal_argument_exception","reason":"%s"}""";
-            final int jsonCharsBeforeReasonValue = Strings.format(expectedPreviousStepInfoFormat, "").length() - 2; // -2 for `"}`
+            final int jsonBaseLength = Strings.format(expectedPreviousStepInfoFormat, "").length();
             final String expectedReason = Strings.format(
                 "%s... (%d chars truncated)",
-                "a".repeat(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH - jsonCharsBeforeReasonValue),
-                jsonCharsBeforeReasonValue
+                "a".repeat(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH - jsonBaseLength),
+                jsonBaseLength
             );
             final String expectedPreviousStepInfo = Strings.format(expectedPreviousStepInfoFormat, expectedReason);
-            assertThat(actualPreviousStepInfo, equalTo(expectedPreviousStepInfo));
+            assertEquals(actualPreviousStepInfo, expectedPreviousStepInfo);
         }
     }