diff --git a/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/model/ExplainSMPolicy.kt b/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/model/ExplainSMPolicy.kt index 28158bf92..46198bfce 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/model/ExplainSMPolicy.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/model/ExplainSMPolicy.kt @@ -5,6 +5,7 @@ package org.opensearch.indexmanagement.snapshotmanagement.model +import org.opensearch.Version import org.opensearch.core.common.io.stream.StreamInput import org.opensearch.core.common.io.stream.StreamOutput import org.opensearch.core.common.io.stream.Writeable @@ -36,10 +37,15 @@ data class ExplainSMPolicy( override fun toXContent(builder: XContentBuilder, params: ToXContent.Params): XContentBuilder { metadata?.let { builder - .field(SMMetadata.CREATION_FIELD, it.creation) .optionalField(SMMetadata.DELETION_FIELD, it.deletion) .field(SMMetadata.POLICY_SEQ_NO_FIELD, it.policySeqNo) .field(SMMetadata.POLICY_PRIMARY_TERM_FIELD, it.policyPrimaryTerm) + + if (Version.CURRENT.onOrAfter(Version.V_3_3_0)) { + builder.optionalField(SMMetadata.CREATION_FIELD, it.creation) + } else { + builder.field(SMMetadata.CREATION_FIELD, it.creation) + } } return builder.field(SMPolicy.ENABLED_FIELD, enabled) } diff --git a/src/test/kotlin/org/opensearch/indexmanagement/bwc/SMBackwardsCompatibilityIT.kt b/src/test/kotlin/org/opensearch/indexmanagement/bwc/SMBackwardsCompatibilityIT.kt index 141c57764..1be4695ed 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/bwc/SMBackwardsCompatibilityIT.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/bwc/SMBackwardsCompatibilityIT.kt @@ -84,6 +84,11 @@ class SMBackwardsCompatibilityIT : SnapshotManagementRestTestCase() { val traditionalPolicy = getSMPolicy(traditionalPolicyName) assertNotNull("Traditional policy creation should exist", traditionalPolicy.creation) assertNotNull("Traditional policy deletion should exist", traditionalPolicy.deletion) + + // Verify explain response works in old version (covers old version serialization path) + val explainResponse = explainSMPolicy(traditionalPolicyName) + val explainMetadata = parseExplainResponse(explainResponse.entity.content) + assertTrue("Explain should return results", explainMetadata.isNotEmpty()) } ClusterType.MIXED -> { assertTrue(pluginNames.contains("opensearch-index-management")) @@ -92,6 +97,11 @@ class SMBackwardsCompatibilityIT : SnapshotManagementRestTestCase() { val traditionalPolicy = getSMPolicy(traditionalPolicyName) assertNotNull("Traditional policy creation should exist", traditionalPolicy.creation) assertNotNull("Traditional policy deletion should exist", traditionalPolicy.deletion) + + // Verify explain response works during rolling upgrade (mixed old/new version serialization) + val explainResponse = explainSMPolicy(traditionalPolicyName) + val explainMetadata = parseExplainResponse(explainResponse.entity.content) + assertTrue("Explain should return results", explainMetadata.isNotEmpty()) } ClusterType.UPGRADED -> { assertTrue(pluginNames.contains("opensearch-index-management")) @@ -101,6 +111,11 @@ class SMBackwardsCompatibilityIT : SnapshotManagementRestTestCase() { assertNotNull("Traditional policy creation should exist", traditionalPolicy.creation) assertNotNull("Traditional policy deletion should exist", traditionalPolicy.deletion) + // Verify explain response for traditional policy on upgraded cluster (new version serialization) + val traditionalExplainResponse = explainSMPolicy(traditionalPolicyName) + val traditionalExplainMetadata = parseExplainResponse(traditionalExplainResponse.entity.content) + assertTrue("Explain should return results for traditional policy", traditionalExplainMetadata.isNotEmpty()) + // Now test new features on upgraded cluster // Create deletion-only policy (3.2.0+ feature) @@ -109,12 +124,22 @@ class SMBackwardsCompatibilityIT : SnapshotManagementRestTestCase() { assertNull("Deletion-only policy creation should be null", deletionOnlyPolicy.creation) assertNotNull("Deletion-only policy deletion should exist", deletionOnlyPolicy.deletion) + // Verify explain response for deletion-only policy (tests null creation serialization with optionalField) + val deletionOnlyExplainResponse = explainSMPolicy(deletionOnlyPolicyName) + val deletionOnlyExplainMetadata = parseExplainResponse(deletionOnlyExplainResponse.entity.content) + assertTrue("Explain should return results for deletion-only policy", deletionOnlyExplainMetadata.isNotEmpty()) + // Create policy with snapshot pattern (3.2.0+ feature) createPatternPolicy(patternPolicyName) val patternPolicy = getSMPolicy(patternPolicyName) assertNotNull("Pattern policy creation should exist", patternPolicy.creation) assertNotNull("Pattern policy deletion should exist", patternPolicy.deletion) assertEquals("Pattern policy should have snapshot pattern", "external-backup-*", patternPolicy.deletion?.snapshotPattern) + + // Verify explain response for pattern policy + val patternExplainResponse = explainSMPolicy(patternPolicyName) + val patternExplainMetadata = parseExplainResponse(patternExplainResponse.entity.content) + assertTrue("Explain should return results for pattern policy", patternExplainMetadata.isNotEmpty()) } } break diff --git a/src/test/kotlin/org/opensearch/indexmanagement/snapshotmanagement/action/ResponseTests.kt b/src/test/kotlin/org/opensearch/indexmanagement/snapshotmanagement/action/ResponseTests.kt index 409f3b29a..d519ae4ba 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/snapshotmanagement/action/ResponseTests.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/snapshotmanagement/action/ResponseTests.kt @@ -6,9 +6,12 @@ package org.opensearch.indexmanagement.snapshotmanagement.action import org.opensearch.common.io.stream.BytesStreamOutput +import org.opensearch.common.xcontent.XContentFactory import org.opensearch.core.common.io.stream.StreamInput import org.opensearch.core.rest.RestStatus +import org.opensearch.core.xcontent.ToXContent import org.opensearch.indexmanagement.indexstatemanagement.util.XCONTENT_WITHOUT_TYPE_AND_USER +import org.opensearch.indexmanagement.opensearchapi.toMap import org.opensearch.indexmanagement.snapshotmanagement.api.transport.explain.ExplainSMPolicyResponse import org.opensearch.indexmanagement.snapshotmanagement.api.transport.get.GetSMPoliciesResponse import org.opensearch.indexmanagement.snapshotmanagement.api.transport.get.GetSMPolicyResponse @@ -17,8 +20,8 @@ import org.opensearch.indexmanagement.snapshotmanagement.model.ExplainSMPolicy import org.opensearch.indexmanagement.snapshotmanagement.randomSMMetadata import org.opensearch.indexmanagement.snapshotmanagement.randomSMPolicy import org.opensearch.indexmanagement.snapshotmanagement.smDocIdToPolicyName -import org.opensearch.indexmanagement.snapshotmanagement.toMap import org.opensearch.test.OpenSearchTestCase +import org.opensearch.indexmanagement.snapshotmanagement.toMap as toMapHelper class ResponseTests : OpenSearchTestCase() { fun `test index sm policy response`() { @@ -38,8 +41,8 @@ class ResponseTests : OpenSearchTestCase() { fun `test index sm policy toXContent`() { val smPolicy = randomSMPolicy() val res = IndexSMPolicyResponse("someid", 1L, 2L, 3L, smPolicy, RestStatus.OK) - val resMap = res.toMap() - assertEquals(resMap["sm_policy"], smPolicy.toMap(XCONTENT_WITHOUT_TYPE_AND_USER)) + val resMap = res.toMapHelper() + assertEquals(resMap["sm_policy"], smPolicy.toMapHelper(XCONTENT_WITHOUT_TYPE_AND_USER)) } fun `test get sm policy response`() { @@ -81,7 +84,32 @@ class ResponseTests : OpenSearchTestCase() { fun `test get sm policy toXContent`() { val smPolicy = randomSMPolicy() val res = GetSMPolicyResponse("someid", 1L, 2L, 3L, smPolicy) - val resMap = res.toMap() - assertEquals(resMap["sm_policy"], smPolicy.toMap(XCONTENT_WITHOUT_TYPE_AND_USER)) + val resMap = res.toMapHelper() + assertEquals(resMap["sm_policy"], smPolicy.toMapHelper(XCONTENT_WITHOUT_TYPE_AND_USER)) + } + + fun `test explain sm policy toXContent with null creation`() { + // Test that ExplainSMPolicy.toXContent() handles null creation correctly (for V_3_3_0+) + val smMetadata = randomSMMetadata() + val smMetadataWithNullCreation = smMetadata.copy(creation = null) + val explainSMPolicy = ExplainSMPolicy(smMetadataWithNullCreation, true) + + // Properly convert ToXContentFragment to map by wrapping in an object + val builder = XContentFactory.jsonBuilder().startObject() + explainSMPolicy.toXContent(builder, ToXContent.EMPTY_PARAMS) + builder.endObject() + val explainMap = builder.toMap() + + // Verify that creation field is NOT present when null (optionalField behavior for V_3_3_0+) + assertFalse("Creation field should not be present when null", explainMap.containsKey("creation")) + + // Verify deletion field IS present + assertTrue("Deletion field should be present", explainMap.containsKey("deletion")) + + // Verify other mandatory fields are present + assertTrue("Policy seq_no should be present", explainMap.containsKey("policy_seq_no")) + assertTrue("Policy primary_term should be present", explainMap.containsKey("policy_primary_term")) + assertTrue("Enabled field should be present", explainMap.containsKey("enabled")) + assertEquals(true, explainMap["enabled"]) } }