RestEasy Reactive + JAXB unable to serialize list of entities

[hakuseki]

Describe the bug

When using a call to the database with the IN operator no records found.

MigrationResource.java

   /**
     * Find by customer id uni.
     *
     * @param customerId the customer id
     * @return the uni
     */
    @GET
    @Path("/customerId/multi")
    @Produces(MediaType.APPLICATION_XML)
    @Consumes(MediaType.TEXT_PLAIN)
    public Uni<List<Accounts>> findByCustomerId(@RestQuery @Separator(",") List<String> customerId) {
        return Accounts.findByCustomerId(customerId);
    }

...

Accounts.java

package se.hakuseki.ciam.migration;

import io.quarkus.hibernate.reactive.panache.PanacheEntity;
import io.quarkus.panache.common.Parameters;
import io.quarkus.runtime.annotations.RegisterForReflection;
import io.smallrye.mutiny.Uni;
import jakarta.persistence.*;
import lombok.Getter;
import lombok.Setter;
import lombok.ToString;

import java.time.LocalDateTime;
import java.time.OffsetDateTime;
import java.util.List;
import java.util.UUID;

// tag::Accounts[]

/**
 * The type Accounts.
 */
@Entity
@Cacheable
@Getter
@Setter
@ToString
@RegisterForReflection
@NamedQueries({
        @NamedQuery(name = "Accounts.getByCustomerId", query = "from Accounts where agasCustomerNumber = ?1", hints = @QueryHint(name = "org.hibernate.cacheable", value = "true"))
})
public class Accounts extends PanacheEntity {
    /**
     * The Uuid.
     */
    @Column(name = "pisa_uid", updatable = false)
    UUID pisaUid;
    /**
     * The User name.
     */
    @Column(name = "username", updatable = false)
    String userName;
    /**
     * The Person number.
     */
    @Column(name = "personnumber", updatable = false)
    String personNumber;
    /**
     * The First name.
     */
    @Column(name = "first_name", updatable = false)
    String firstName;
    /**
     * The Last name.
     */
    @Column(name = "last_name", updatable = false)
    String lastName;
    /**
     * The Email.
     */
    @Column(updatable = false)
    String email;
    /**
     * The Password.
     */
    @Column(updatable = false)
    String password;
    /**
     * The Customer number.
     */
    @Column(name = "customernumber", updatable = false)
    String customerNumber;
    /**
     * The Company customer number.
     */
    @Column(name = "companycustomernumber", updatable = false)
    String companyCustomerNumber;
    /**
     * The Organisation customer number.
     */
    @Column(name = "customernumberorganisation", updatable = false)
    String organisationCustomerNumber;
    /**
     * The Migration state.
     */
    @Column(name = "migration_state")
    boolean migrationState;
    /**
     * The Imported timestamp.
     */
    @Column(name = "imported_timestamp", updatable = false)
    LocalDateTime importedTimestamp;
    /**
     * The Migrated timestamp.
     */
    @Column(name = "migrated_timestamp")
    OffsetDateTime migratedTimestamp;

    /**
     * The Customer number ag.
     */
    @Column(name = "agas_customernumber", updatable = false)
    String agasCustomerNumber;

    /**
     * The Last login.
     */
    @Column(name = "last_login", updatable = false)
    LocalDateTime lastLogin;



    /**
     * Find by customer id uni.
     *
     * @param customerId the customer id
     * @return the uni
     */
    public static Uni<Accounts> findByCustomerId(String customerId) {
        return find("#Accounts.getByCustomerId", customerId).singleResult();
    }

    /**
     * Find by customer id uni.
     *
     * @param customerId the customer id
     * @return the uni
     */
    public static Uni<List<Accounts>> findByCustomerId(List<String> customerId) {
        return find("agasCustomerNumber in :vals ", Parameters.with("vals", customerId)).list();
    }
}
// end::Accounts[]

Expected behavior

Records found

Actual behavior

Empty response

How to Reproduce?

Provided with a .http file

Five records in the database with agas_customernumber values from 1-5

Output of uname -a or ver

Darwin Mikaels-MacBook-Pro.local 21.6.0 Darwin Kernel Version 21.6.0: Mon Apr 24 21:10:53 PDT 2023; root:xnu-8020.240.18.701.5~1/RELEASE_X86_64 x86_64

Output of java -version

openjdk version "17.0.5" 2022-10-18 OpenJDK Runtime Environment GraalVM CE 22.3.0 (build 17.0.5+8-jvmci-22.3-b08) OpenJDK 64-Bit Server VM GraalVM CE 22.3.0 (build 17.0.5+8-jvmci-22.3-b08, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

3.1.0.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.1 (2e178502fcdbffc201671fb2537d0cb4b4cc58f8)

Additional information

Use this repo for tests

https://github.com/hakuseki/ciammigrering

[quarkus-bot]

/cc @FroMage (panache), @loicmathieu (panache)

[yrodiere]

Thanks for the report.

I just had a look, and Panache + Hibernate ORM is affected as well: https://github.com/yrodiere/ciammigrering/tree/hibernate-orm

I'll try to set up a Hibernate-ORM-only reproducer, to see whether the problem is in Panache (doubtful) or not (likely)...

[yrodiere]

Ok, so actually this is not a Hibernate ORM problem, this is a serialization problem. If you debug your reproducer, you'll see the accounts are correctly returned by Panache, but then for some reason the (non-empty) list is serialized as an empty list.

Serializing an array, on the other hand, will work fine: https://github.com/yrodiere/ciammigrering/tree/serialization-array
So it really has to do with serializing lists, even serializing entities works fine.

At this point I'd ask someone knowledgeable with quarkus-resteasy-reactive-jaxb to have a look... ? Seems weird that serialization of arraylists to XML is not supported out of the box.

@geoand @Sgitario, any opinion? The reproducer is there: https://github.com/hakuseki/ciammigrering

[Sgitario]

@geoand @Sgitario, any opinion? The reproducer is there: https://github.com/hakuseki/ciammigrering

Thanks for the reproducer!

Even though it seems to work for arrays, the resulting output:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?><accounts[]><item><id>1</id><agasCustomerNumber>1</agasCustomerNumber><email>[email protected]</email><firstName>Mikael</firstName><lastName>Andersson Wigander</lastName><migrationState>false</migrationState><personNumber>101010-1010</personNumber></item><item><id>51</id><agasCustomerNumber>2</agasCustomerNumber><email>[email protected]</email><firstName>Justin</firstName><lastName>Case</lastName><migrationState>false</migrationState><password>{SSHA}Sj8ybg83UfHrnTsLwuMjBgXC4COynbuJ1MjBmQ==</password><userName>DustinJustin</userName></item><item><id>101</id><agasCustomerNumber>3</agasCustomerNumber><email>[email protected]</email><firstName>Kalle</firstName><lastName>Kula</lastName><migrationState>true</migrationState><personNumber>19791231-2452</personNumber><userName>kallekula</userName></item></accounts[]>

It's not XML complaint:

[Fatal Error] :1:65: El tipo de elemento "accounts" debe ir seguido de una de estas especificaciones de atributo: ">" o "/>".

jakarta.xml.bind.UnmarshalException
 - with linked exception:
[org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 65; El tipo de elemento "accounts" debe ir seguido de una de estas especificaciones de atributo: ">" o "/>".]
	at jakarta.xml.bind.helpers.AbstractUnmarshallerImpl.createUnmarshalException(AbstractUnmarshallerImpl.java:290)

• Sorry for the Spanish messages :)

So, this is a "limitation" of JAXB that does not directly support List, Set or Arrays. The way to go is to wrap these entities in a root element like:

@XmlRootElement(name = "accounts")
@XmlAccessorType(XmlAccessType.FIELD)
public class ListOfAccounts {
    @XmlElement(name = "account")
    List<Accounts> accounts;

    public ListOfAccounts() {

    }

    public ListOfAccounts(List<Accounts> accounts) {
        this.accounts = accounts;
    }
}

And return it to the resource like:

@GET
@Path("/customerId/multi")
@Produces(MediaType.APPLICATION_XML)
@Consumes(MediaType.TEXT_PLAIN)
public Uni<ListOfAccounts> findByCustomerId(@RestQuery @Separator(",") List<String> customerId) {
    return Accounts.findByCustomerId( customerId ).onItem().transform(ListOfAccounts::new);
}

And now, the resource will return a valid XML resource regardless if you use an array, list, or set.

Now, the question is if the Quarkus RESTeasy Reactive JAXB should "alter" the way how JAXB works to support arrays, lists or sets without needing to wrap the collection. Still, users would need to provide somehow the name of the root element. Though I see the benefit, plus it would work similarly to other libraries like Jackson or Jsonb support it. Wdyt? @gsmet @geoand

[FroMage]

OK, sorry about falsely accusing ORM, after I ruled out parameter deserialisation. I didn't think it would be serialisation, my bad. There's always too many pieces in there :(

@Sgitario: how does the JSON serialisation work when serialising arrays, lists or sets? I guess JSON has types for arrays.

Frankly, I would expect XML to serialise List<Accounts> as a <accounts><account>…</account></accounts>. In any event it would be much better for users if we manage to do that than the current weirdness, no?

If that's consistent with how lists/arrays/sets are serialised elsewhere in XML.

[hakuseki]

Returning JSON instead of XML makes it work:

[
  {
    "id": 1,
    "pisaUid": null,
    "userName": null,
    "personNumber": "101010-1010",
    "firstName": "Mikael",
    "lastName": "Andersson Wigander",
    "email": "[email protected]",
    "password": null,
    "customerNumber": null,
    "companyCustomerNumber": null,
    "organisationCustomerNumber": null,
    "migrationState": false,
    "importedTimestamp": null,
    "migratedTimestamp": null,
    "agasCustomerNumber": "1",
    "lastLogin": null
  },
  {
    "id": 51,
    "pisaUid": null,
    "userName": "DustinJustin",
    "personNumber": null,
    "firstName": "Justin",
    "lastName": "Case",
    "email": "[email protected]",
    "password": "{SSHA}Sj8ybg83UfHrnTsLwuMjBgXC4COynbuJ1MjBmQ==",
    "customerNumber": null,
    "companyCustomerNumber": null,
    "organisationCustomerNumber": null,
    "migrationState": false,
    "importedTimestamp": null,
    "migratedTimestamp": null,
    "agasCustomerNumber": "2",
    "lastLogin": null
  },
  {
    "id": 101,
    "pisaUid": null,
    "userName": "kallekula",
    "personNumber": "19791231-2452",
    "firstName": "Kalle",
    "lastName": "Kula",
    "email": "[email protected]",
    "password": null,
    "customerNumber": null,
    "companyCustomerNumber": null,
    "organisationCustomerNumber": null,
    "migrationState": true,
    "importedTimestamp": null,
    "migratedTimestamp": null,
    "agasCustomerNumber": "3",
    "lastLogin": null
  }
]

[geoand]

Now, the question is if the Quarkus RESTeasy Reactive JAXB should "alter" the way how JAXB works to support arrays, lists or sets without needing to wrap the collection. Still, users would need to provide somehow the name of the root element. Though I see the benefit, plus it would work similarly to other libraries like Jackson or Jsonb support it. Wdyt?

I do not think we should be altering such fundamentals. Maybe we can offer some kind of new mode, but changing the default seems pretty heavy handed to me...

[Sgitario]

Now, the question is if the Quarkus RESTeasy Reactive JAXB should "alter" the way how JAXB works to support arrays, lists or sets without needing to wrap the collection. Still, users would need to provide somehow the name of the root element. Though I see the benefit, plus it would work similarly to other libraries like Jackson or Jsonb support it. Wdyt?

I do not think we should be altering such fundamentals. Maybe we can offer some kind of new mode, but changing the default seems pretty heavy handed to me...

+1000

[FroMage]

but changing the default seems pretty heavy handed to me...

Well, isn't it the case that the default is "completely wrong and confusing"? It looks to me like the current default serialisation results in an empty list being silently returned, or invalid XML.

In this case, changing the default seems like a good move, no? At the minimum, we should just throw a detailed server error when we know that serialisation just won't do what people expect it to, no?

[geoand]

I am against changing the default given that JAXB is a standard and thus some people are very likely to be familiar with and expect the behavior we don't like

[FroMage]

Just to be clear what the behaviour is ATM (as I understand it):

• JAXB doesn't support serialising lists
• We don't throw any warning or error if the user returns a list and uses JAXB and pass it on
• JAXB doesn't produce any error or warning, then goes on to serialise it either as a:
  -- 4-element list when the original list had 3
  -- 0-element list when the original had more than 0
  -- Invalid XML

This all seems pretty random too, and took us a while to figure out. Are you really sure this is the standard behaviour we should keep?

[geoand]

My point is we shouldn't be changing standard behavior.

+1 for warnings

[Sgitario]

I also think printing a warning or even throwing an exception is the best approach instead of changing the standard behavior.

Note that the JAXB does support lists and other collections, but if the collection is wrapped into a root element (aka class). So, the warning or the exception should help users to go into this direction.

[FroMage]

Yeah, so let's make it an error to return a list or array when serialising to JAXB.