Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Object metadata with a Map of metadata properties #32

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

jakzal
Copy link
Member

@jakzal jakzal commented Aug 6, 2022

Serialization has not worked properly with Object(s). The problem has resurfaced when trying to implement Jackson-based serialization. In past, we discussed replacing the Object with a Map of properties.

I have added the Map of properties and deprecated the Object to be removed in future versions.

Serialization has not worked properly with Object(s).

Signed-off-by: Jakub Zalas <jakub@zalas.pl>
Copy link
Contributor

@VaughnVernon VaughnVernon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jakzal Must be certain that reading from storage initializes the empty properties. Also, what if the object is set? Should it be put into properties with a key = "object", or...?

@jakzal
Copy link
Member Author

jakzal commented Aug 7, 2022

Serializing just any objects in metadata never worked as they'd be deserialized as maps:

  class Foo {
    public String item = "value";
  }
 
  @Test
  public void testSerializedMetadataObject() {
    final Object object = new Foo();
    final String serialized = JsonSerialization.serialized(Metadata.withObject(object));
    final Metadata metadata = JsonSerialization.deserialized(serialized, Metadata.class);
    assertEquals("com.google.gson.internal.LinkedTreeMap", metadata.object.getClass().getName());
    assertEquals(Collections.singletonMap("item", "value"), metadata.object);
    // NOT: assertEquals(object, metadata.object);
  }

Serializing lists and maps worked as expected:

  @Test
  public void testSerializedMetadataList() {
    final Object object = Collections.singletonList("item1");
    final String serialized = JsonSerialization.serialized(Metadata.withObject(object));
    final Metadata metadata = JsonSerialization.deserialized(serialized, Metadata.class);
    assertEquals("java.util.ArrayList", metadata.object.getClass().getName());
    assertEquals(object, metadata.object);
  }

  @Test
  public void testSerializedMetadataMap() {
    final Object object = Collections.singletonMap("key1", "value1");
    final String serialized = JsonSerialization.serialized(Metadata.withObject(object));
    final Metadata metadata = JsonSerialization.deserialized(serialized, Metadata.class);
    assertEquals("com.google.gson.internal.LinkedTreeMap", metadata.object.getClass().getName());
    assertEquals(object, metadata.object);
  }

Also, what if the object is set? Should it be put into properties with a key = "object", or...?

I haven't considered migrating it to the map. The type I chose for the properties map is Map<String,String>. Too constrainted?

I thought to put it on end users to migrate their code as I can't predict what might've been put into the object and they'll eventually need to replace calls to the object with calls to properties.

Must be certain that reading from storage initializes the empty properties.

Yes.

@VaughnVernon
Copy link
Contributor

@jakzal OK. Merge when ready.

@jakzal jakzal merged commit 14ec2e7 into master Aug 10, 2022
@jakzal jakzal deleted the metadata-properties branch August 10, 2022 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants