Skip to content

Conversation

@loicmathieu
Copy link
Contributor

No description provided.

@FroMage
Copy link
Member

FroMage commented May 18, 2020

Looks OK, but I don't understand why the existing tests didn't show this, and why we don't need a new test to show we've fixed it?

@loicmathieu
Copy link
Contributor Author

@FroMage I agree with you and I'm a little disturbed by the fact the our integration test worked as it is the same as the example that didn't works on the mailing list.

I verify it with the example on the mailing list and it passes.

Now, two options, dig inside the issue and try to understand it better ... or admit that computer science is part of the chaos theory and that we didn't understand everything and merge it ;)

Anyway, this PR is the right implementation, it's just fustrating because it shouldn't have worked in the first place ...

@FroMage
Copy link
Member

FroMage commented May 19, 2020

Are you sure the tests are run?

@loicmathieu
Copy link
Contributor Author

@FroMage yes, I double checked via a good old sysout and it's called inside

    @GET
    @Path("/search/{name}")
    public List<PersonName> searchPersons(@PathParam("name") String name) {
        return personRepository.find("lastname", name).project(PersonName.class).list();
    }

Which is more or less the same thing as what is reported as an issue ...

@FroMage
Copy link
Member

FroMage commented May 19, 2020

Watch out that this code doesn't check the return type at all. Try:

  @GET
    @Path("/search/{name}")
    public List<PersonName> searchPersons(@PathParam("name") String name) {
        List<PersonName> ret = personRepository.find("lastname", name).project(PersonName.class).list();
        PersonName p = ret.get(0); // this will throw if it's not the right type
        return ret;
    }

@loicmathieu
Copy link
Contributor Author

@FroMage good catch! Generic type erasure is tricky!
If I add this line in the current test I have the exception so I can reproduce it:

Caused by: java.lang.ClassCastException: class io.quarkus.it.mongodb.panache.person.Person cannot be cast to class io.quarkus.it.mongodb.panache.person.PersonName (io.quarkus.it.mongodb.panache.person.Person and io.quarkus.it.mongodb.panache.person.PersonName are in unnamed module of loader io.quarkus.bootstrap.classloading.QuarkusClassLoader @15e8f9b2)

I'll add something in the test.

@loicmathieu loicmathieu force-pushed the fix/panache-projection branch from a8985cf to 7c4c281 Compare May 19, 2020 09:15
@loicmathieu
Copy link
Contributor Author

I added a bunch of test code to be sure that the type is the correct one.
The new test failed in master and works with this PR

@loicmathieu loicmathieu force-pushed the fix/panache-projection branch from 7c4c281 to fb60c67 Compare May 20, 2020 10:22
@geoand geoand merged commit 3f46fda into quarkusio:master May 22, 2020
@loicmathieu loicmathieu deleted the fix/panache-projection branch May 22, 2020 07:13
@gsmet gsmet added this to the 1.5.0.Final milestone May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants