Fix UnlModify serialization

sappenin · sappenin · commit c2dd463c82d5 · 2025-02-13T15:08:53.000-07:00
diff --git a/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/jackson/modules/TransactionDeserializer.java b/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/jackson/modules/TransactionDeserializer.java
@@ -71,7 +71,7 @@ public Transaction deserialize(JsonParser jsonParser, DeserializationContext ctx
     if (UnlModify.class.isAssignableFrom(transactionTypeClass)) {
 //    if (objectNode.get("Account").isEmpty()) {
       if (objectNode.has("Account")) {
-        objectNode.remove("Account");
+        //objectNode.remove("Account");
       }
     }
 
diff --git a/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/jackson/modules/TransactionSerializer.java b/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/jackson/modules/TransactionSerializer.java
@@ -40,38 +40,38 @@
  * Custom deserializer for {@link Transaction}s, which deserializes to a specific {@link Transaction} type based on the
  * TransactionType JSON field.
  */
-public class TransactionSerializer { //} extends StdSerializer<Transaction> {
-//
-//  /**
-//   * No-args constructor.
-//   */
-//  protected TransactionSerializer() {
-//    super(Transaction.class);
-//  }
-//
-//  @Override
-//  public void serialize(Transaction value, JsonGenerator gen, SerializerProvider provider) throws IOException {
-//
-//    if (UnlModify.class.isAssignableFrom(value.getClass())) {
-//      gen.writeStartObject();
-//
-//      // 1. Delegate to Jackson for the existing fields:
-//      provider.defaultSerializeValue(value, gen); // Delegate to Jackson
-//
-////      ObjectMapperFactory.create().writeValue(gen, value);
-//
-//      // Serialize the existing fields (delegate to Jackson's default if possible)
-////      gen.writeObject((UnlModify) value); // This will handle the standard Transaction fields
-//
-//      // Add the extra "Account" field here because by marking it @Derived in the Immutable, this field does not show
-//      // up during serialization.
-//      gen.writeFieldName("Account");
-//      gen.writeString(value.account().value());
-//
-//      gen.writeEndObject();
-//    } else {
-//      // Rely on Jackson's automatic serialization
-//      gen.writeObject(value);
-//    }
-//  }
+public class TransactionSerializer extends StdSerializer<Transaction> {
+
+  /**
+   * No-args constructor.
+   */
+  protected TransactionSerializer() {
+    super(Transaction.class);
+  }
+
+  @Override
+  public void serialize(Transaction value, JsonGenerator gen, SerializerProvider provider) throws IOException {
+
+    if (UnlModify.class.isAssignableFrom(value.getClass())) {
+      gen.writeStartObject();
+
+      // 1. Delegate to Jackson for the existing fields:
+      provider.defaultSerializeValue(value, gen); // Delegate to Jackson
+
+//      ObjectMapperFactory.create().writeValue(gen, value);
+
+      // Serialize the existing fields (delegate to Jackson's default if possible)
+//      gen.writeObject((UnlModify) value); // This will handle the standard Transaction fields
+
+      // Add the extra "Account" field here because by marking it @Derived in the Immutable, this field does not show
+      // up during serialization.
+      gen.writeFieldName("Account");
+      gen.writeString(value.account().value());
+
+      gen.writeEndObject();
+    } else {
+      // Rely on Jackson's automatic serialization
+      gen.writeObject(value);
+    }
+  }
 }
diff --git a/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/jackson/modules/UnlModifyTransactionSerializer.java b/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/jackson/modules/UnlModifyTransactionSerializer.java
@@ -21,12 +21,16 @@
  */
 
 import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.SerializerProvider;
 import com.fasterxml.jackson.databind.ser.std.StdSerializer;
 import org.xrpl.xrpl4j.model.transactions.Transaction;
 import org.xrpl.xrpl4j.model.transactions.UnlModify;
+import org.xrpl.xrpl4j.model.transactions.metadata.CreatedNode;
 
 import java.io.IOException;
+import java.util.Map;
 
 /**
  * Custom deserializer for {@link Transaction}s, which deserializes to a specific {@link Transaction} type based on the
@@ -44,14 +48,28 @@ protected UnlModifyTransactionSerializer() {
   @Override
   public void serialize(UnlModify value, JsonGenerator gen, SerializerProvider provider) throws IOException {
 
+//    ObjectMapper codec = (ObjectMapper) jsonParser.getCodec();
+//    JsonNode jsonNode = jsonParser.readValueAsTree();
+//    Map.Entry<String, JsonNode> nodeFieldAndValue = jsonNode.fields().next();
+//    String affectedNodeType = nodeFieldAndValue.getKey();
+
+//    codec.getTypeFactory().constructParametricType(CreatedNode.class, ledgerObjectClass)
+
+
+//    ObjectMapper objectMapper = (ObjectMapper) gen.getCodec();
+//    String json = objectMapper.writeValueAsString(value);
+
+//    provider.defaultSerializeValue()
+
+
 //    if (UnlModify.class.isAssignableFrom(value.getClass())) {
 //      gen.writeStartObject();
 
 //    if (gen.getCurrentValue() == null
 //    gen.getCurrentValue().toString().length() == 1
 //    ) {
       // 1. Delegate to Jackson for the existing fields:
-      provider.defaultSerializeValue(value, gen); // Delegate to Jackson
+//      provider.defaultSerializeValue(value, gen); // Delegate to Jackson
 //    }
 
 //      ObjectMapperFactory.create().writeValue(gen, value);
diff --git a/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/jackson/modules/Xrpl4jModule.java b/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/jackson/modules/Xrpl4jModule.java
@@ -61,7 +61,7 @@ public Xrpl4jModule() {
     addSerializer(LedgerIndex.class, new LedgerIndexSerializer());
     addDeserializer(LedgerIndex.class, new LedgerIndexDeserializer());
 
-    addSerializer(UnlModify.class, new UnlModifyTransactionSerializer());
+//    addSerializer(UnlModify.class, new UnlModifyTransactionSerializer());
     addDeserializer(Transaction.class, new TransactionDeserializer());
 
     addDeserializer(ServerInfo.class, new ServerInfoDeserializer());
diff --git a/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/UnlModify.java b/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/UnlModify.java
@@ -28,6 +28,7 @@
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
 import com.google.common.primitives.UnsignedInteger;
 import org.immutables.value.Value;
+import org.xrpl.xrpl4j.crypto.keys.ImmutablePublicKey;
 import org.xrpl.xrpl4j.crypto.keys.PublicKey;
 import org.xrpl.xrpl4j.model.client.common.LedgerIndex;
 
@@ -92,9 +93,11 @@ default TransactionType transactionType() {
    * @return Always returns ACCOUNT_ZERO, which is the base58 encoding of the number zero.
    */
   @Override
-  @JsonProperty(value = "Account", access = Access.WRITE_ONLY)
+  @JsonProperty(value = "Account"
+    , access = Access.READ_WRITE
+  )
   @Value.Derived
-  @JsonIgnore(false)
+  @JsonSerialize
   default Address account() {
     return ACCOUNT_ZERO;
   }
diff --git a/xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/json/UnlModifyJsonTests.java b/xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/json/UnlModifyJsonTests.java
@@ -20,10 +20,12 @@
  * =========================LICENSE_END==================================
  */
 
-import static org.assertj.core.api.FactoryBasedNavigableListAssert.assertThat;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.xrpl.xrpl4j.model.AddressConstants.ACCOUNT_ZERO;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.google.common.primitives.UnsignedInteger;
+import com.google.common.primitives.UnsignedLong;
 import org.assertj.core.api.Assertions;
 import org.json.JSONException;
 import org.junit.jupiter.api.Test;
@@ -36,6 +38,9 @@
 import org.xrpl.xrpl4j.model.transactions.UnlModify;
 import org.xrpl.xrpl4j.model.transactions.XrpCurrencyAmount;
 
+import java.util.Collections;
+import java.util.Map;
+
 public class UnlModifyJsonTests extends AbstractJsonTest {
 
   @Test
@@ -64,12 +69,12 @@ public void testJsonWithAccountZero() throws JsonProcessingException, JSONExcept
   @Test
   public void testJsonWithAccountZeroAndUnknownFields() throws JsonProcessingException, JSONException {
     UnlModify unlModify = UnlModify.builder()
+      .putUnknownFields("Foo", "Bar")
       .fee(XrpCurrencyAmount.ofDrops(12))
       .sequence(UnsignedInteger.valueOf(2470665))
       .ledgerSequence(LedgerIndex.of(UnsignedInteger.valueOf(67850752)))
       .unlModifyValidator("EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539")
       .unlModifyDisabling(UnsignedInteger.valueOf(1))
-      .putUnknownFields("Foo", "Bar")
       .build();
 
     String json = "{" +
@@ -96,42 +101,79 @@ public void testJsonWithEmptyAccount() throws JsonProcessingException, JSONExcep
       .unlModifyDisabling(UnsignedInteger.valueOf(1))
       .build();
 
-    String json = "{" +
-      "\"Account\":\"\"," +
+    // What we expect the ObjectMapper to Serialize (given the above object)
+    String expectedSerializedJson = "{" +
+      "\"Account\":\"" + ACCOUNT_ZERO + "\"," +
       "\"Fee\":\"12\"," +
       "\"LedgerSequence\":67850752," +
       "\"Sequence\":2470665," +
       "\"SigningPubKey\":\"\"," +
       "\"TransactionType\":\"UNLModify\"," +
       "\"UNLModifyDisabling\":1," +
-      "\"UNLModifyValidator\":\"EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539\"}";
+      "\"UNLModifyValidator\":\"EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539\"" +
+      "}";
 
-    assertCanSerializeAndDeserialize(unlModify, json);
+    String serialized = objectMapper.writeValueAsString(unlModify);
+    JSONAssert.assertEquals(expectedSerializedJson, serialized, JSONCompareMode.STRICT);
+
+    // What we provide the ObjectMapper to Deserialize
+    String expectedDeserializedJson = "{" +
+      "\"Account\":\"\"," + // <-- The crux of the test!
+      "\"Fee\":\"12\"," +
+      "\"LedgerSequence\":67850752," +
+      "\"Sequence\":2470665," +
+      "\"SigningPubKey\":\"\"," +
+      "\"TransactionType\":\"UNLModify\"," +
+      "\"UNLModifyDisabling\":1," +
+      "\"UNLModifyValidator\":\"EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539\"" +
+      "}";
+
+    Transaction deserialized = objectMapper.readValue(expectedDeserializedJson, Transaction.class);
+    assertThat(deserialized).isEqualTo(unlModify);
   }
 
   @Test
   public void testJsonWithEmptyAccountAndUnknownFields() throws JsonProcessingException, JSONException {
     UnlModify unlModify = UnlModify.builder()
+      .putUnknownFields("Foo", "Bar")
       .fee(XrpCurrencyAmount.ofDrops(12))
       .sequence(UnsignedInteger.valueOf(2470665))
       .ledgerSequence(LedgerIndex.of(UnsignedInteger.valueOf(67850752)))
       .unlModifyValidator("EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539")
       .unlModifyDisabling(UnsignedInteger.valueOf(1))
-      .putUnknownFields("Foo", "Bar")
       .build();
 
-    String json = "{" +
-      "\"Foo\" : \"Bar\",\n" +
-      "\"Account\":\"\"," +
+    // What we expect the ObjectMapper to Serialize (given the above object)
+    String expectedSerializedJson = "{" +
+      "\"Account\":\"" + ACCOUNT_ZERO + "\"," +
+      "\"Foo\" : \"Bar\",\n" + // <-- The crux of the test!
       "\"Fee\":\"12\"," +
       "\"LedgerSequence\":67850752," +
       "\"Sequence\":2470665," +
       "\"SigningPubKey\":\"\"," +
       "\"TransactionType\":\"UNLModify\"," +
       "\"UNLModifyDisabling\":1," +
-      "\"UNLModifyValidator\":\"EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539\"}";
+      "\"UNLModifyValidator\":\"EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539\"" +
+      "}";
 
-    assertCanSerializeAndDeserialize(unlModify, json);
+    String serialized = objectMapper.writeValueAsString(unlModify);
+    JSONAssert.assertEquals(expectedSerializedJson, serialized, JSONCompareMode.STRICT);
+
+    // What we provide the ObjectMapper to Deserialize
+    String expectedDeserializedJson = "{" +
+      "\"Foo\" : \"Bar\",\n" + // <-- The crux of the test!
+      "\"Account\":\"\"," + // <-- The crux of the test!
+      "\"Fee\":\"12\"," +
+      "\"LedgerSequence\":67850752," +
+      "\"Sequence\":2470665," +
+      "\"SigningPubKey\":\"\"," +
+      "\"TransactionType\":\"UNLModify\"," +
+      "\"UNLModifyDisabling\":1," +
+      "\"UNLModifyValidator\":\"EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539\"" +
+      "}";
+
+    Transaction deserialized = objectMapper.readValue(expectedDeserializedJson, Transaction.class);
+    assertThat(deserialized).isEqualTo(unlModify);
   }
 
   @Test
@@ -144,7 +186,8 @@ public void testJsonWithMissingAccount() throws JsonProcessingException, JSONExc
       .unlModifyDisabling(UnsignedInteger.valueOf(1))
       .build();
 
-    String json = "{" +
+    String expectedJson = "{" +
+      "\"Account\":\"" + ACCOUNT_ZERO + "\"," +
       "\"Fee\":\"12\"," +
       "\"LedgerSequence\":67850752," +
       "\"Sequence\":2470665," +
@@ -153,7 +196,20 @@ public void testJsonWithMissingAccount() throws JsonProcessingException, JSONExc
       "\"UNLModifyDisabling\":1," +
       "\"UNLModifyValidator\":\"EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539\"}";
 
-    assertCanSerializeAndDeserialize(unlModify, json);
+    String serialized = objectMapper.writeValueAsString(unlModify);
+    JSONAssert.assertEquals(expectedJson, serialized, JSONCompareMode.STRICT);
+
+    String jsonForDeserialization = "{" +
+      "\"Fee\":\"12\"," +
+      "\"LedgerSequence\":67850752," +
+      "\"Sequence\":2470665," +
+      "\"SigningPubKey\":\"\"," +
+      "\"TransactionType\":\"UNLModify\"," +
+      "\"UNLModifyDisabling\":1," +
+      "\"UNLModifyValidator\":\"EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539\"}";
+
+    Transaction deserialized = objectMapper.readValue(jsonForDeserialization, Transaction.class);
+    assertThat(deserialized).isEqualTo(unlModify);
   }
 
   @Test
@@ -164,28 +220,43 @@ public void testJsonWithMissingAccountAndUnknownFields() throws JsonProcessingEx
       .ledgerSequence(LedgerIndex.of(UnsignedInteger.valueOf(67850752)))
       .unlModifyValidator("EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539")
       .unlModifyDisabling(UnsignedInteger.valueOf(1))
-      .putUnknownFields("Foo", "Bar")
       .build();
 
-    String json = "{" +
-      "\"Foo\" : \"Bar\",\n" +
+    // What we expect the ObjectMapper to Serialize (given the above object)
+    String expectedSerializedJson = "{" +
+      "\"Account\":\"" + ACCOUNT_ZERO + "\"," +
       "\"Fee\":\"12\"," +
       "\"LedgerSequence\":67850752," +
       "\"Sequence\":2470665," +
       "\"SigningPubKey\":\"\"," +
       "\"TransactionType\":\"UNLModify\"," +
       "\"UNLModifyDisabling\":1," +
-      "\"UNLModifyValidator\":\"EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539\"}";
+      "\"UNLModifyValidator\":\"EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539\"" +
+      "}";
 
-    assertCanSerializeAndDeserialize(unlModify, json);
+    String serialized = objectMapper.writeValueAsString(unlModify);
+    JSONAssert.assertEquals(expectedSerializedJson, serialized, JSONCompareMode.STRICT);
+
+    // What we provide the ObjectMapper to Deserialize
+    String expectedDeserializedJson = "{" +
+      "\"Account\":\"" + ACCOUNT_ZERO + "\"," +
+      "\"Fee\":\"12\"," +
+      "\"LedgerSequence\":67850752," +
+      "\"Sequence\":2470665," +
+      "\"SigningPubKey\":\"\"," +
+      "\"TransactionType\":\"UNLModify\"," +
+      "\"UNLModifyDisabling\":1," +
+      "\"UNLModifyValidator\":\"EDB6FC8E803EE8EDC2793F1EC917B2EE41D35255618DEB91D3F9B1FC89B75D4539\"" +
+      "}";
+
+    Transaction deserialized = objectMapper.readValue(expectedDeserializedJson, Transaction.class);
+    assertThat(deserialized).isEqualTo(unlModify);
   }
 
   // Using Transcation
   // Using Immutable
   // Using UnlModify
 
-
-
 //  @Test
 //  public void testDeserializeJson() throws JsonProcessingException {
 //    String json = "{" +

Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,7 @@ public Transaction deserialize(JsonParser jsonParser, DeserializationContext ctx`
`71`	`71`	`if (UnlModify.class.isAssignableFrom(transactionTypeClass)) {`
`72`	`72`	`// if (objectNode.get("Account").isEmpty()) {`
`73`	`73`	`if (objectNode.has("Account")) {`
`74`		`- objectNode.remove("Account");`
	`74`	`+ //objectNode.remove("Account");`
`75`	`75`	`}`
`76`	`76`	`}`
`77`	`77`