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

Unexpected path "id" generated in EntityPathBase for JPA class #965

Closed
virgo47 opened this Issue Sep 26, 2014 · 7 comments

Comments

Projects
None yet
3 participants
@virgo47

virgo47 commented Sep 26, 2014

Using com.mysema.maven:apt-maven-plugin:1.1.1 there is a case that an entity without any "id" column (in our case Currency) gets EntityPathBase implementation QCurrency with "id" path in it.

More in the picture. There is getId() overriding abstract method from Currency's superclass BaseObject, but there is no field "id" anywhere (all fields including inherited are shown on the right in a popup). @Id is on the field (called name), so getters/setters should be ignored. JPA's Property access is not specified in any other way.

querydsl-id-issue

There should be "name" path (and it is), but no "id" path. Now I can write query:

    public static final QCurrency $ = QCurrency.currency;
        return queryFrom($)
            .leftJoin($.dateBasis).fetch()
            .where($.id.eq(id))
            .uniqueResult($);

That throws quite expected:

Exception Description: Problem compiling [select currency
from Currency currency
  left join fetch currency.dateBasis
where currency.id = ?1]. 
[80, 91] The state field path 'currency.id' cannot be resolved to a valid type.
    at org.eclipse.persistence.internal.jpa.EntityManagerImpl.createQuery(EntityManagerImpl.java:1605)
    at sun.reflect.GeneratedMethodAccessor24.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:483)
@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Sep 26, 2014

Member

In JPAAnnotationProcessor, line 46 we make a JPAConfiguration object with the data that is supplied.

In JPAConfiguration.getConfig()

public VisitorConfig getConfig(TypeElement e, List<? extends Element> elements) {

we look for the query properties you need by looking at the property access type you request. But if you don't specify an access type, we iterate the members of your entity and look for relevant annotations.

Are you sure your methods of your BaseObject aren't annotated in any way that could make the AnnotationProcessor jump into ALL mode?

Member

Shredder121 commented Sep 26, 2014

In JPAAnnotationProcessor, line 46 we make a JPAConfiguration object with the data that is supplied.

In JPAConfiguration.getConfig()

public VisitorConfig getConfig(TypeElement e, List<? extends Element> elements) {

we look for the query properties you need by looking at the property access type you request. But if you don't specify an access type, we iterate the members of your entity and look for relevant annotations.

Are you sure your methods of your BaseObject aren't annotated in any way that could make the AnnotationProcessor jump into ALL mode?

@virgo47

This comment has been minimized.

Show comment
Hide comment
@virgo47

virgo47 Sep 26, 2014

Hi! I just debugged the Maven build to get there, went through my Currency entity, got out of this call VisitorConfig config = configuration.getConfig(element, elements); (in TypeElementHandler) and config says 'FIELDS_ONLY'. There is that MethodSymbol getId, but no VarSymbol called id at all.

Further section behind if (config.visitMethodProperties()) { is skipped...

The trouble seems to be related to BaseObject indeed - this one has nothing relevent in it, just that abstract getId, but even that one does not determine the access type - and that switches BaseObject to ALL, and generates public final SimplePath<java.io.Serializable> id = createSimple("id", java.io.Serializable.class);, because BaseObject is defined as:

public abstract class BaseObject<T extends Serializable> implements Serializable

Probably this somehow "generates" id path to QCurrency?

It can be resolved by explicitely adding @Access(AccessType.FIELD) above BaseObject. But I'm rather confused here - JPA has no problem understanding that all our concrete classes are field access. Even if BaseClass has some id path in it, is there any reason to put it into QCurrency? Or is there any other reason for it?

virgo47 commented Sep 26, 2014

Hi! I just debugged the Maven build to get there, went through my Currency entity, got out of this call VisitorConfig config = configuration.getConfig(element, elements); (in TypeElementHandler) and config says 'FIELDS_ONLY'. There is that MethodSymbol getId, but no VarSymbol called id at all.

Further section behind if (config.visitMethodProperties()) { is skipped...

The trouble seems to be related to BaseObject indeed - this one has nothing relevent in it, just that abstract getId, but even that one does not determine the access type - and that switches BaseObject to ALL, and generates public final SimplePath<java.io.Serializable> id = createSimple("id", java.io.Serializable.class);, because BaseObject is defined as:

public abstract class BaseObject<T extends Serializable> implements Serializable

Probably this somehow "generates" id path to QCurrency?

It can be resolved by explicitely adding @Access(AccessType.FIELD) above BaseObject. But I'm rather confused here - JPA has no problem understanding that all our concrete classes are field access. Even if BaseClass has some id path in it, is there any reason to put it into QCurrency? Or is there any other reason for it?

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Sep 26, 2014

Member

@virgo47 Could you provide an independent pair of classes that replicates the issue?

Member

timowest commented Sep 26, 2014

@virgo47 Could you provide an independent pair of classes that replicates the issue?

@virgo47

This comment has been minimized.

Show comment
Hide comment
@virgo47

virgo47 Sep 26, 2014

Sure:

package sk.finrisk.persistence.model;

import java.io.Serializable;

@SuppressWarnings("serial")
//@Access(AccessType.FIELD)
public abstract class BaseX<T extends Serializable> implements Serializable {

    public abstract T getId();
}

And:

package sk.finrisk.persistence.model;

import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.Table;

@SuppressWarnings("serial")
@Entity
@Table(name = "X")
public class ConcreteX extends BaseX<String> {

    @Id
    @Column(name = "name", nullable = false)
    private String name;

    @Override
    public String getId() {
        return name;
    }
}

Produces Q classes:

@Generated("com.mysema.query.codegen.EmbeddableSerializer")
public class QBaseX extends BeanPath<BaseX<? extends java.io.Serializable>> {

    private static final long serialVersionUID = -1316361335L;

    public static final QBaseX baseX = new QBaseX("baseX");

    public final SimplePath<java.io.Serializable> id = createSimple("id", java.io.Serializable.class);

    @SuppressWarnings("all")
    public QBaseX(String variable) {
        super((Class)BaseX.class, forVariable(variable));
    }

    @SuppressWarnings("all")
    public QBaseX(Path<? extends BaseX> path) {
        super((Class)path.getType(), path.getMetadata());
    }

    @SuppressWarnings("all")
    public QBaseX(PathMetadata<?> metadata) {
        super((Class)BaseX.class, metadata);
    }
}

And:

@Generated("com.mysema.query.codegen.EntitySerializer")
public class QConcreteX extends EntityPathBase<ConcreteX> {

    private static final long serialVersionUID = -193915563L;

    public static final QConcreteX concreteX = new QConcreteX("concreteX");

    public final QBaseX _super = new QBaseX(this);

    public final StringPath id = createString("id");

    public final StringPath name = createString("name");

    public QConcreteX(String variable) {
        super(ConcreteX.class, forVariable(variable));
    }

    public QConcreteX(Path<? extends ConcreteX> path) {
        super(path.getType(), path.getMetadata());
    }

    public QConcreteX(PathMetadata<?> metadata) {
        super(ConcreteX.class, metadata);
    }
}

The same with uncommented @Access produces virtually the same, but with id path gone from both Q classes.

virgo47 commented Sep 26, 2014

Sure:

package sk.finrisk.persistence.model;

import java.io.Serializable;

@SuppressWarnings("serial")
//@Access(AccessType.FIELD)
public abstract class BaseX<T extends Serializable> implements Serializable {

    public abstract T getId();
}

And:

package sk.finrisk.persistence.model;

import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.Table;

@SuppressWarnings("serial")
@Entity
@Table(name = "X")
public class ConcreteX extends BaseX<String> {

    @Id
    @Column(name = "name", nullable = false)
    private String name;

    @Override
    public String getId() {
        return name;
    }
}

Produces Q classes:

@Generated("com.mysema.query.codegen.EmbeddableSerializer")
public class QBaseX extends BeanPath<BaseX<? extends java.io.Serializable>> {

    private static final long serialVersionUID = -1316361335L;

    public static final QBaseX baseX = new QBaseX("baseX");

    public final SimplePath<java.io.Serializable> id = createSimple("id", java.io.Serializable.class);

    @SuppressWarnings("all")
    public QBaseX(String variable) {
        super((Class)BaseX.class, forVariable(variable));
    }

    @SuppressWarnings("all")
    public QBaseX(Path<? extends BaseX> path) {
        super((Class)path.getType(), path.getMetadata());
    }

    @SuppressWarnings("all")
    public QBaseX(PathMetadata<?> metadata) {
        super((Class)BaseX.class, metadata);
    }
}

And:

@Generated("com.mysema.query.codegen.EntitySerializer")
public class QConcreteX extends EntityPathBase<ConcreteX> {

    private static final long serialVersionUID = -193915563L;

    public static final QConcreteX concreteX = new QConcreteX("concreteX");

    public final QBaseX _super = new QBaseX(this);

    public final StringPath id = createString("id");

    public final StringPath name = createString("name");

    public QConcreteX(String variable) {
        super(ConcreteX.class, forVariable(variable));
    }

    public QConcreteX(Path<? extends ConcreteX> path) {
        super(path.getType(), path.getMetadata());
    }

    public QConcreteX(PathMetadata<?> metadata) {
        super(ConcreteX.class, metadata);
    }
}

The same with uncommented @Access produces virtually the same, but with id path gone from both Q classes.

@virgo47

This comment has been minimized.

Show comment
Hide comment
@virgo47

virgo47 Sep 26, 2014

BTW: I'm on Querydsl 3.3.3 (BOM from Spring IO 1.0.1), but I tried with 3.4.3 too and the result was the same.

virgo47 commented Sep 26, 2014

BTW: I'm on Querydsl 3.3.3 (BOM from Spring IO 1.0.1), but I tried with 3.4.3 too and the result was the same.

@timowest timowest added this to the 3.5.0 milestone Sep 27, 2014

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Sep 27, 2014

Member

I added a PR, which disables handling of annotationless supertypes for JPA. The same option is also available for the GenericExporter based code generation.

Member

timowest commented Sep 27, 2014

I added a PR, which disables handling of annotationless supertypes for JPA. The same option is also available for the GenericExporter based code generation.

@Shredder121 Shredder121 closed this in #967 Sep 30, 2014

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Sep 30, 2014

Member

Released in 3.5.0

Member

timowest commented Sep 30, 2014

Released in 3.5.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment