From 7bb80eb3633a41d5fd3c58bd18d978d550d5efd6 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Fri, 22 Mar 2024 13:40:48 -0400 Subject: [PATCH 1/2] Expose a TypeMarker factory to wrap an arbitrary Type This mirrors a change we've just introduced to conjure-undertow here: https://github.com/palantir/conjure-java/pull/2264 Dialogue takes advantage of this new factory method in ConjureBodySerDe to avoid holding class references in serializer caches, which introduced some friction in our dns-polling lifecycle management. I haven't reverted the change to specify `weakKeys` in the caches, however the keys should never be freed unless classes are unloaded (e.g. plugins). --- .../java/dialogue/serde/ConjureBodySerDe.java | 17 +++++++++-------- .../java/com/palantir/dialogue/TypeMarker.java | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java index 90d7df8f1..b738b8529 100644 --- a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java +++ b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java @@ -40,6 +40,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Comparator; import java.util.List; @@ -56,8 +57,8 @@ final class ConjureBodySerDe implements BodySerDe { private final Deserializer binaryInputStreamDeserializer; private final Deserializer> optionalBinaryInputStreamDeserializer; private final Deserializer emptyBodyDeserializer; - private final LoadingCache, Serializer> serializers; - private final LoadingCache, Deserializer> deserializers; + private final LoadingCache> serializers; + private final LoadingCache> deserializers; /** * Selects the first (based on input order) of the provided encodings that @@ -86,11 +87,11 @@ final class ConjureBodySerDe implements BodySerDe { this.emptyBodyDeserializer = new EmptyBodyDeserializer(errorDecoder); // Class unloading: Not supported, Jackson keeps strong references to the types // it sees: https://github.com/FasterXML/jackson-databind/issues/489 - this.serializers = - Caffeine.from(cacheSpec).build(token -> new EncodingSerializerRegistry<>(defaultEncoding, token)); + this.serializers = Caffeine.from(cacheSpec) + .build(type -> new EncodingSerializerRegistry<>(defaultEncoding, TypeMarker.of(type))); this.deserializers = Caffeine.from(cacheSpec) - .build(token -> new EncodingDeserializerRegistry<>( - encodingsSortedByWeight, errorDecoder, emptyContainerDeserializer, token)); + .build(type -> new EncodingDeserializerRegistry<>( + encodingsSortedByWeight, errorDecoder, emptyContainerDeserializer, TypeMarker.of(type))); } private static List decorateEncodings(List input) { @@ -112,13 +113,13 @@ private ImmutableList sortByWeight(List encodings) { @Override @SuppressWarnings("unchecked") public Serializer serializer(TypeMarker token) { - return (Serializer) serializers.get(token); + return (Serializer) serializers.get(token.getType()); } @Override @SuppressWarnings("unchecked") public Deserializer deserializer(TypeMarker token) { - return (Deserializer) deserializers.get(token); + return (Deserializer) deserializers.get(token.getType()); } @Override diff --git a/dialogue-target/src/main/java/com/palantir/dialogue/TypeMarker.java b/dialogue-target/src/main/java/com/palantir/dialogue/TypeMarker.java index ba6d3ccb0..a8a548997 100644 --- a/dialogue-target/src/main/java/com/palantir/dialogue/TypeMarker.java +++ b/dialogue-target/src/main/java/com/palantir/dialogue/TypeMarker.java @@ -16,6 +16,7 @@ package com.palantir.dialogue; +import com.palantir.logsafe.Preconditions; import com.palantir.logsafe.SafeArg; import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import java.lang.reflect.ParameterizedType; @@ -45,6 +46,10 @@ protected TypeMarker() { } } + private TypeMarker(Type type) { + this.type = Preconditions.checkNotNull(type, "Type is required"); + } + public final Type getType() { return type; } @@ -70,4 +75,15 @@ public final int hashCode() { public final String toString() { return "TypeMarker{type=" + type + '}'; } + + /** Create a new {@link TypeMarker} instance wrapping the provided {@link Type}. */ + public static TypeMarker of(Type type) { + return new WrappingTypeMarker(type); + } + + private static final class WrappingTypeMarker extends TypeMarker { + private WrappingTypeMarker(Type type) { + super(type); + } + } } From e9d2d29b43bebfd2201c2bfced6fa67736e2016f Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Fri, 22 Mar 2024 17:50:05 +0000 Subject: [PATCH 2/2] Add generated changelog entries --- changelog/@unreleased/pr-2221.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-2221.v2.yml diff --git a/changelog/@unreleased/pr-2221.v2.yml b/changelog/@unreleased/pr-2221.v2.yml new file mode 100644 index 000000000..c3f5e9115 --- /dev/null +++ b/changelog/@unreleased/pr-2221.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Expose a TypeMarker factory to wrap an arbitrary Type + links: + - https://github.com/palantir/dialogue/pull/2221