jakartaee/data

[TCK Challenge]: ExtensionTests#testDataProviderWithBuildCompatibleExtension conflicts with QBN

starksm64 opened this issue · 4 comments

Specification

spec/src/main/asciidoc/repository.asciidoc

Assertion

A valid Query by Method Name exists but the TCK is expecting something different

TCK Version

SNAPSHOT (Locally built)

Implementation being tested

HIbernate

Challenge Scenario

Claims that a test assertion conflicts with the specification.

Full Description

The ExtensionTests#testDataProviderWithBuildCompatibleExtension test is expecting that it can call the injected ee.jakarta.tck.data.common.cdi.Directory#findFirstNameByIdInOrderByAgeDesc() method and obtain a list of Person.firstName values as though the repository was annotated with:

@Repository(provider = Directory.PERSON_PROVIDER)
public interface Directory extends DataRepository<Person, Long> {
    @Query("select firstName from Person where id in ?1 order by age desc")
    List<String> findFirstNameByIdInOrderByAgeDesc(List<Long> ids);
}

while findFirstNameByIdInOrderByAgeDesc is a valid QBN method that parses to:
find-expression: findFirst
ignored-text: Name
predicate: id IN
order-clause: OrderBy age Desc

which results in an interface annotated like:

public interface Directory$ extends Directory {
            @Override
            @Query("where id in ?1 order by '' limit 1")
            @OrderBy(value="age", descending = true)
            public java.util.List<java.lang.String> findFirstNameByIdInOrderByAgeDesc (java.util.List<java.lang.Long> ids);
}

but the return type should be type of Person rather than firstName values.

Additional Context

No response

Is there an existing challenge for this?

  • I have searched the existing issues

There's another thing wrong with this test. Even if:

select firstName from Person where id in ?1 order by age desc

is the intended interpretation, it's ordering by a field age which is not in the projection list.

is the intended interpretation, it's ordering by a field age which is not in the projection list.

@gavinking, does query by method name have such a restriction? Besides the findFirstissue I don't see anything in the specification that limits query by method name to sort only based on the field returned.

I can fix the findFIrst issue.

does query by method name have such a restriction?

I dunno, the spec for query by method name isn't terribly, shall we say, precise about such things.

Actually, looking at the spec, I don't see where query by method name even supports projection at all.

The relevant part of the BNF boils down to:

"find" ["First" [<positive-integer>]] [<ignored-text>] "By"

AFAICT, Name here matches to <ignored-text>, and so the query should be returning Person, not String.

I think the real purpose of this test is to use a fake provider from the TCK to ensure that multiple providers can co-exist, and probably not much attention was given to the validity of what the method itself was doing per the spec. I agree with the assessment above that it would be interpreted as finding the first Person, which was probably not the intent. Let's change the method to:

    List<Person> findByIdInOrderByAgeDesc(List<Long> ids);

and the test can easily transform the resulting list of person into a list of string for the assertion.