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

AbstractCompositeField: The non-serializable super class of a "Serializable" class should have a no-argument constructor #5082

Open
csnkishore opened this issue Feb 22, 2024 · 2 comments
Labels

Comments

@csnkishore
Copy link

Describe the bug

The issue was identified by SonarQube scan for the class com.vaadin.flow.component.AbstractCompositeField when it was used within the code like so -
public class AddressComponent extends AbstractCompositeField<Div, AddressComponent, AddressFieldsValueBean> {
.
.
}

This is from Vaadin version 14.

SonarQube description of the issue -
When a Serializable object has a non-serializable ancestor in its inheritance chain, object deserialization (re-instantiating the object from file) starts at the first non-serializable class, and proceeds down the chain, adding the properties of each subsequent child class, until the final object has been instantiated.

In order to create the non-serializable ancestor, its no-argument constructor is called. Therefore the non-serializable ancestor of a Serializable class must have a no-arg constructor. Otherwise the class is Serializable but not deserializable.

Noncompliant Code Example
public class Fruit {
private Season ripe;

public Fruit (Season ripe) {...}
public void setRipe(Season ripe) {...}
public Season getRipe() {...}
}

public class Raspberry extends Fruit
implements Serializable { // Noncompliant; nonserializable ancestor doesn't have no-arg constructor
private static final long serialVersionUID = 1;

private String variety;

public Raspberry(Season ripe, String variety) { ...}
public void setVariety(String variety) {...}
public String getVarity() {...}
}
Compliant Solution
public class Fruit {
private Season ripe;

public Fruit () {...}; // Compliant; no-arg constructor added to ancestor
public Fruit (Season ripe) {...}
public void setRipe(Season ripe) {...}
public Season getRipe() {...}
}

public class Raspberry extends Fruit
implements Serializable {
private static final long serialVersionUID = 1;

private String variety;

public Raspberry(Season ripe, String variety) {...}
public void setVariety(String variety) {...}
public String getVarity() {...}
}

Expected-behavior

The class com.vaadin.flow.component.AbstractCompositeField implements a no-arg constructor.

Reproduction

The class com.vaadin.flow.component.AbstractCompositeField does not have a no-arg constructor in Vaadin 14

System Info

Vaadin Ver. 14

@csnkishore csnkishore added the bug label Feb 22, 2024
@Legioth
Copy link
Member

Legioth commented Feb 22, 2024

I'm not sure I understand what this means. What is the "non-serializable ancestor" in this case?

@TatuLund
Copy link
Contributor

Better code example

package org.vaadin.tatu;

import java.io.Serializable;

public class Fruit {
    private Season ripe;

    public Fruit(Season ripe) {
        this.ripe = ripe;
    }

    public void setRipe(Season ripe) {
        this.ripe = ripe;
    }

    public Season getRipe() {
        return this.ripe;
    }

    @SuppressWarnings("serial")
    public static class Raspberry extends Fruit implements Serializable {

        private String variety;

        public Raspberry(Season ripe, String variety) {
            super(ripe);
            this.variety = variety;
        }

        public void setVariety(String variety) {
            this.variety = variety;
        }

        public String getVarity() {
            return variety;
        }
    }

    public enum Season {
        RIPE, NOT_RIPE;
    }
}

And unit test, which indeed fails if Fruit does not have no-args constructor.

    @Test
    public void serializableTest() throws IOException, ClassNotFoundException {
        Raspberry rasp = new Raspberry(Season.RIPE, "Red");
        var bs = new ByteArrayOutputStream();
        var os = new ObjectOutputStream(bs);
        os.writeObject(rasp);
        os.flush();
        os.close();

        var a = bs.toByteArray();
        ByteArrayInputStream bis = new ByteArrayInputStream(a);
        ObjectInputStream in = new ObjectInputStream(bis);
        var v = (Raspberry) in.readObject();
        Assert.assertEquals("Red", v.getVarity());
    }

But the real question here is that AbstractCompositeField does not have non-serializable ancestor, the root is Component which is Serializable. So what is the point here? Is there bug in SonarQube that is not picking it up as Serializable comes via mixin interfaces Component implements?

public abstract class Component
        implements HasStyle, AttachNotifier, DetachNotifier { .. }

I.e. would SonarQube be happy if it is

public abstract class Component
        implements HasStyle, AttachNotifier, DetachNotifier, Serializable { .. }

@TatuLund TatuLund added question and removed bug labels Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants