diff --git a/retrofit-converters/jaxb/src/main/java/retrofit2/converter/jaxb/JaxbConverterFactory.java b/retrofit-converters/jaxb/src/main/java/retrofit2/converter/jaxb/JaxbConverterFactory.java index 25b6912bc9..671c20c67f 100644 --- a/retrofit-converters/jaxb/src/main/java/retrofit2/converter/jaxb/JaxbConverterFactory.java +++ b/retrofit-converters/jaxb/src/main/java/retrofit2/converter/jaxb/JaxbConverterFactory.java @@ -21,6 +21,7 @@ import javax.xml.bind.JAXBContext; import javax.xml.bind.JAXBException; import javax.xml.bind.annotation.XmlRootElement; +import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter; import okhttp3.MediaType; import okhttp3.RequestBody; import okhttp3.ResponseBody; @@ -59,7 +60,7 @@ private JaxbConverterFactory(@Nullable JAXBContext context) { Annotation[] parameterAnnotations, Annotation[] methodAnnotations, Retrofit retrofit) { - if (type instanceof Class && ((Class) type).isAnnotationPresent(XmlRootElement.class)) { + if (type instanceof Class && isJaxbAnnotated((Class) type)) { return new JaxbRequestConverter<>(contextForType((Class) type), (Class) type); } return null; @@ -68,12 +69,17 @@ private JaxbConverterFactory(@Nullable JAXBContext context) { @Override public @Nullable Converter responseBodyConverter( Type type, Annotation[] annotations, Retrofit retrofit) { - if (type instanceof Class && ((Class) type).isAnnotationPresent(XmlRootElement.class)) { + if (type instanceof Class && isJaxbAnnotated((Class) type)) { return new JaxbResponseConverter<>(contextForType((Class) type), (Class) type); } return null; } + private static boolean isJaxbAnnotated(Class type) { + return type.isAnnotationPresent(XmlRootElement.class) + || type.isAnnotationPresent(XmlJavaTypeAdapter.class); + } + private JAXBContext contextForType(Class type) { try { return context != null ? context : JAXBContext.newInstance(type); diff --git a/retrofit-converters/jaxb/src/test/java/retrofit2/converter/jaxb/ImmutablePoint.java b/retrofit-converters/jaxb/src/test/java/retrofit2/converter/jaxb/ImmutablePoint.java new file mode 100644 index 0000000000..e9b893db49 --- /dev/null +++ b/retrofit-converters/jaxb/src/test/java/retrofit2/converter/jaxb/ImmutablePoint.java @@ -0,0 +1,71 @@ +/* + * Copyright (C) 2024 Square, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package retrofit2.converter.jaxb; + +import javax.xml.bind.annotation.XmlElement; +import javax.xml.bind.annotation.XmlRootElement; +import javax.xml.bind.annotation.adapters.XmlAdapter; +import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter; + +/** + * An immutable type (no no-arg constructor) that uses {@link XmlJavaTypeAdapter} to participate in + * JAXB serialization. This is the pattern documented at + * http://blog.bdoughan.com/2010/12/jaxb-and-immutable-objects.html + */ +@XmlJavaTypeAdapter(ImmutablePoint.Adapter.class) +final class ImmutablePoint { + final int x; + final int y; + + ImmutablePoint(int x, int y) { + this.x = x; + this.y = y; + } + + @Override + public boolean equals(Object o) { + return o instanceof ImmutablePoint + && ((ImmutablePoint) o).x == x + && ((ImmutablePoint) o).y == y; + } + + @Override + public int hashCode() { + return 31 * x + y; + } + + /** Mutable intermediate type used by JAXB. */ + @XmlRootElement(name = "point") + static final class Mutable { + @XmlElement public int x; + @XmlElement public int y; + } + + static final class Adapter extends XmlAdapter { + @Override + public ImmutablePoint unmarshal(Mutable v) { + return new ImmutablePoint(v.x, v.y); + } + + @Override + public Mutable marshal(ImmutablePoint v) { + Mutable m = new Mutable(); + m.x = v.x; + m.y = v.y; + return m; + } + } +} diff --git a/retrofit-converters/jaxb/src/test/java/retrofit2/converter/jaxb/JaxbConverterFactoryTest.java b/retrofit-converters/jaxb/src/test/java/retrofit2/converter/jaxb/JaxbConverterFactoryTest.java index f2ac1fb23e..f5bdfc6e7d 100644 --- a/retrofit-converters/jaxb/src/test/java/retrofit2/converter/jaxb/JaxbConverterFactoryTest.java +++ b/retrofit-converters/jaxb/src/test/java/retrofit2/converter/jaxb/JaxbConverterFactoryTest.java @@ -185,6 +185,28 @@ public void externalEntity() throws Exception { assertThat(server.getRequestCount()).isEqualTo(1); } + @Test + public void xmlJavaTypeAdapterIsRecognizedForRequestBody() throws Exception { + // A class annotated with @XmlJavaTypeAdapter (without @XmlRootElement) should be recognized + // by the factory and result in a non-null converter. Previously the factory only checked for + // @XmlRootElement and returned null for such types. + JaxbConverterFactory factory = JaxbConverterFactory.create(); + Retrofit retrofit = + new Retrofit.Builder().baseUrl(server.url("/")).addConverterFactory(factory).build(); + assertThat(retrofit.requestBodyConverter(ImmutablePoint.class, new java.lang.annotation.Annotation[0], new java.lang.annotation.Annotation[0])).isNotNull(); + } + + @Test + public void xmlJavaTypeAdapterIsRecognizedForResponseBody() throws Exception { + // A class annotated with @XmlJavaTypeAdapter (without @XmlRootElement) should be recognized + // by the factory and result in a non-null converter. Previously the factory only checked for + // @XmlRootElement and returned null for such types. + JaxbConverterFactory factory = JaxbConverterFactory.create(); + Retrofit retrofit = + new Retrofit.Builder().baseUrl(server.url("/")).addConverterFactory(factory).build(); + assertThat(retrofit.responseBodyConverter(ImmutablePoint.class, new java.lang.annotation.Annotation[0])).isNotNull(); + } + @Test public void externalDtd() throws Exception { server.enqueue( diff --git a/retrofit-converters/jaxb3/src/main/java/retrofit2/converter/jaxb3/JaxbConverterFactory.java b/retrofit-converters/jaxb3/src/main/java/retrofit2/converter/jaxb3/JaxbConverterFactory.java index fb5799b137..2f9d655c29 100644 --- a/retrofit-converters/jaxb3/src/main/java/retrofit2/converter/jaxb3/JaxbConverterFactory.java +++ b/retrofit-converters/jaxb3/src/main/java/retrofit2/converter/jaxb3/JaxbConverterFactory.java @@ -18,6 +18,7 @@ import jakarta.xml.bind.JAXBContext; import jakarta.xml.bind.JAXBException; import jakarta.xml.bind.annotation.XmlRootElement; +import jakarta.xml.bind.annotation.adapters.XmlJavaTypeAdapter; import java.lang.annotation.Annotation; import java.lang.reflect.Type; import javax.annotation.Nullable; @@ -59,7 +60,7 @@ private JaxbConverterFactory(@Nullable JAXBContext context) { Annotation[] parameterAnnotations, Annotation[] methodAnnotations, Retrofit retrofit) { - if (type instanceof Class && ((Class) type).isAnnotationPresent(XmlRootElement.class)) { + if (type instanceof Class && isJaxbAnnotated((Class) type)) { return new JaxbRequestConverter<>(contextForType((Class) type), (Class) type); } return null; @@ -68,12 +69,17 @@ private JaxbConverterFactory(@Nullable JAXBContext context) { @Override public @Nullable Converter responseBodyConverter( Type type, Annotation[] annotations, Retrofit retrofit) { - if (type instanceof Class && ((Class) type).isAnnotationPresent(XmlRootElement.class)) { + if (type instanceof Class && isJaxbAnnotated((Class) type)) { return new JaxbResponseConverter<>(contextForType((Class) type), (Class) type); } return null; } + private static boolean isJaxbAnnotated(Class type) { + return type.isAnnotationPresent(XmlRootElement.class) + || type.isAnnotationPresent(XmlJavaTypeAdapter.class); + } + private JAXBContext contextForType(Class type) { try { return context != null ? context : JAXBContext.newInstance(type);