Is there a cleaner way of building a MappedSuperclass Tree class in Spring JPA?

Question

I currently have several Entities that behave as a Tree and need to save them to the DB.

So in order to not have duplicated code for that I built this class:

@MappedSuperclass
public abstract class TreeStructure<T extends TreeStructure>
{
    @ManyToOne(cascade = CascadeType.PERSIST)
    private T  parent;

    @OneToMany(mappedBy = "parent", fetch = FetchType.LAZY, cascade = CascadeType.PERSIST)
    protected Set<T> children = new HashSet<>();

    /**
     * Function that is used before deleting this entity. It joins this.children to this.parent and viceversa.
     */
    @Transactional
    @PreRemove
    public void preDelete()
    {
        unregisterInParentsChildren();

        while (!children.isEmpty())
        {
            children.iterator().next().setParent(parent);
        }

    }

    public abstract long getId();

    protected void setParent(T pParent)
    {
        unregisterInParentsChildren();
        parent = pParent;
        registerInParentsChildren();
    }

    /**
     * Register this TreeStructure in the child list of its parent if it's not null.
     */
    private void registerInParentsChildren()
    {
        getParent().ifPresent((pParent) -> pParent.children.add(this));
    }

    /**
     * Unregister this TreeStructure in the child list of its parent if it's not null.
     */
    private void unregisterInParentsChildren()
    {
        getParent().ifPresent((pParent) -> pParent.children.remove(this));
    }

    /**
     * Move this TreeStructure to an new parent TreeStructure.
     *
     * @param pNewParent the new parent
     */
    public void move(final T pNewParent)
    {
        if (pNewParent == null)
        {
            throw new IllegalArgumentException("New Parent required");
        }

        if (!isProperMoveTarget(pNewParent) /* detect circles... */)
        {
            throw new IllegalArgumentException(String.format("Unable to move Object %1$s to new Object Parent %2$s", getId(), pNewParent.getId()));
        }

        setParent(pNewParent);
    }

    private boolean isProperMoveTarget(TreeStructure pParent)
    {
        if (pParent == null)
        {
            return true;
        }
        if (pParent == this)
        {
            return false;
        }

        return isProperMoveTarget(pParent.parent);
    }

    public int getLevel()
    {
        return getParent().map(pParent -> pParent.getLevel() + 1).orElse(1);
    }

    /**
     * Return the <strong>unmodifiable</strong> children of this TreeStructure.
     *
     * @return the child nodes.
     */
    public Set<T> getChildren()
    {
        return Collections.unmodifiableSet(this.children);
    }

    public Optional<T> getParent()
    {
        return Optional.ofNullable(parent);
    }

    public Optional<Long> getParentCategoryId()
    {
        return parent == null ? Optional.empty() : Optional.of(parent.getId());
    }
}

Then to actually implement it I simply do:

@Entity(name = "CATEGORY")
public class Category extends TreeStructure<Category>
{
    @Id
    @GeneratedValue(strategy = GenerationType.AUTO)
    @JsonProperty("category_id")
    private long id;

// etc...

As far as I know, everything is working like a charm but everytime I get inside the TreeStructure class Intellij highlights some errors:

mappedBy = "parent" -> Cannot resolve attribute parent.

children.iterator().next().setParent(parent) -> Unchecked call to setParent(T) as a member of raw type TreeStructure

pParent.children.add(this) -> Unchecked call to add(E) as a member of raw type java.util.Set

I also tried not using generics so I could just have abstract TreeStructure and then extend from the other classes but then I have problems with parent/children since you can not refer a MappedSuperclass from OneToMany/ManyToOne references.

So, finally getting to the point: Is there anyway of implementing this in a better/cleaner way? Are this warnings meaningfull or it's just Intellij not being smarter enough?

The unchecked calls can be ignored. The `mappedBy = "parent"` indicates that whatever `T` you use, it might not have a field called `parent` (thats how I understand it) — XtremeBaumer, Mar 28 '19 at 15:15

score 1 · Accepted Answer · answered Mar 28 '19 at 15:33

1

The issue isn't with JPA, but with the use of generics.

First, change your abstract class signature so that it has a recursive type:

public abstract class TreeStructure<T extends TreeStructure<T>>

Next, you can't reference 'this' since you don't know the implementation of 'this' so you can either just cast it to 'T' or add an abstract method with a signature like this:

public abstract T getImpl();

And in the implementation just return 'this'.

public T getImpl() {
  return this;
}

On a side node, its probably not a good idea to access the parent classes instance variables in your class. It would probably be a better idea to add an addChild and removeChild methods to you TreeStructure class.

answered Mar 28 '19 at 15:33

Zack

301
1
11

Thanks, this really fixed the Unchecked calls. Didn't know about recursive Generics so I will certainly investigate further. :) Do you know if there's a way of implementing this kind of behaviour without using Generics? – epsilonmajorquezero Mar 28 '19 at 17:10
On the other side, I need to query the tree up and down for performance so I kinda need to have the parent and children in each of the instances. – epsilonmajorquezero Mar 28 '19 at 17:13
Found this answer to be particularly helpful: https://stackoverflow.com/a/30667912/2560692 But still, which are the issues with not making the generic recursive? As far as I've seen everything was working perfectly without it. Is there any danger with parametrising using raw types? – epsilonmajorquezero Mar 28 '19 at 17:41

ValerioMC · Answer 2 · 2019-03-28T17:13:52.257

0

I had a very similar scenario and i didn't use T. Instead i had only the abstract class because i didn't need the flexibility of typed children and i had no casts. it might ground you as far as I can see (shared code) but i don't know if you have other requirements.

Another difference in my case was the abstract class is not mapped superclass, but @Inheritance(strategy = InheritanceType.SINGLE_TABLE).

If it can help, you can find a complete working example in this repository

@Inheritance(strategy = InheritanceType.SINGLE_TABLE)
public abstract class TreeStructure {

    ...

    @ManyToOne(cascade = CascadeType.PERSIST)
    private TreeStructure  parent;

    @OneToMany(mappedBy = "parent", fetch = FetchType.LAZY, cascade = CascadeType.PERSIST)
    protected Set<TreeStructure> children = new HashSet<>();

edited Mar 28 '19 at 17:13

answered Mar 28 '19 at 15:22

ValerioMC

2,926
13
24

Are you sure this works? I'm told that you can't map OneToMany to a MappedSuperclass. – epsilonmajorquezero Mar 28 '19 at 17:00
you right... my abstract class `@Inheritance(strategy = InheritanceType.SINGLE_TABLE)` i've not used `@MappedSuperclass`, it can be also for the problem you said. i will update my answer – ValerioMC Mar 28 '19 at 17:08
If I'm not wrong then all of the implementations of TreeStructure end up being in the same table which I would like to avoid also :) – epsilonmajorquezero Mar 28 '19 at 17:11
yes this depends on your business logic. in my case it was ok because entities were very similar – ValerioMC Mar 28 '19 at 17:13
Yeah, not the case at all for me appart from the underlying TreeStructure sadly. Thanks for the insights anyway! :) – epsilonmajorquezero Mar 28 '19 at 17:14

Is there a cleaner way of building a MappedSuperclass Tree class in Spring JPA?

2 Answers2