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.

Sign up for GitHub

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

Jump to bottom

Bean reflection doesn't check parent class #84

Closed
decebals opened this issue Aug 4, 2015 · 14 comments
Closed

Bean reflection doesn't check parent class #84

decebals opened this issue Aug 4, 2015 · 14 comments

Comments

@decebals
Copy link

decebals commented Aug 4, 2015

I have a CompanyService with a method get(long id):Company, I injected the companyService in my template model and I I try something like this:

{% for customer in customers %}
<tr>
   <td>{{ companyService.get(customer.companyId) }}</td>
</tr>
{% endfor %}

where Customer has a property Long companyId (my idea is to create a quick prototype of application without a fat CustomerDto that contains a Company).
The result is an empty ("") string without any message in console.
Any idea?
@decebals
Copy link
Author

decebals commented Aug 5, 2015

Anybody?
I want to continue to use this excellent library but without a minimal support I cannot go forward.

@UshakovVasilii
Copy link
Contributor

I think empty string is possible if companyService.get(customer.companyId) is null or toString method return empty string.
please check the following:
<td>{{ companyService.get(customer.companyId) is null }}</td>
<td>{{ companyService.get(customer.companyId).toString() is empty }}</td>

@jcarvalho
Copy link
Contributor

If any of those fail, you can also fire up the Debugger, and break on PrintNode's render method.

This way you will be able to track your method invocation and see the returned value.

@decebals
Copy link
Author

decebals commented Aug 5, 2015

After some debugging I can say that problems is here. My method customerService.get(Long id) is not found and I don't know why.
In my opinion pebble must throw an warning message in console if it not found a method, because it's very hard to debug this kind of problems.

@jcarvalho
Copy link
Contributor

Perhaps there is an issue with primitive types?

According to your original issue, your get method receives a primitive long, whereas your getCompanyId method returns a boxed Long. I don't believe Pebble automatically takes care of that conversion.

@decebals
Copy link
Author

decebals commented Aug 5, 2015

I changed my companyService.get(long id) to companyService(Long id) because I feel that pebble is not prepared for this 😄

My test is very simple.
PebbleTest.java

public static void main(String[] args) {
    List<Company> companies = new ArrayList<>();
    companies.add(new Company(1) // id
        .setName("decisoft")
        .setFiscalCode("111111111"));
    CompanyService companyService = new InMemoryCompanyService(companies);

    PebbleEngine engine = new PebbleEngine();
    try {
        PebbleTemplate compiledTemplate = engine.getTemplate("templates/test.peb");
        Writer writer = new StringWriter();

        Map<String, Object> context = new HashMap<>();
        context.put("companyService", companyService);
        compiledTemplate.evaluate(writer, context);

        String output = writer.toString();
        System.out.println("output = " + output);
    } catch (PebbleException e) {
        e.printStackTrace();
    } catch (IOException e) {
        e.printStackTrace();
    }
}

and test.peb is:

<!DOCTYPE html>
<html>
    <head>
        <title>Pebble Test</title>
    </head>

    <body>
        <!--
        <span>{{ companyService.get(1) }}</span>
        -->
        <span>{{ companyService.getByFiscalCode("111111111") }}</span>
    </body>
</html>

The result is:

  • {{ companyService.getByFiscalCode("111111111") }} WORKS
  • {{ companyService.get(1) }} DOESN"T WORK

Has anyone tried to use a parameter with type number/long?

@decebals
Copy link
Author

decebals commented Aug 7, 2015

I found where is the problem. The problem is the same founded by me here.
In a sentence the get() method is a method from a superclass that it's not found with the current pebble implementation clazz.getMethod(attributeName, parameterTypes);
So, InMemoryCompanyService is a class that extends InMemoryGenericService.
In conclusion I think that it's a bug in pebble.
Can someone fix this issue, please?

@decebals
Copy link
Author

decebals commented Aug 7, 2015

I have the same problem if I try to use {{ companies.get(customer.companyId) }} or most simple {{ companies.get(1) }} where companies variable is a Map<Long, Company>.

@mbosecke
Copy link
Collaborator

mbosecke commented Aug 7, 2015

That makes sense that it wouldn't find inherited fields or methods; I would consider this a bug. It will have to recursively check parent classes until it hits the Object class. Hopefully I'll have a chance next week to take a closer look.

@mbosecke mbosecke changed the title Is it a bug? Bean reflection doesn't check parent class Aug 7, 2015
mbosecke added a commit that referenced this issue Aug 31, 2015
@mbosecke
Added a test for #84
f445c3c
@mbosecke
Copy link
Collaborator

Sorry for the delay on this one. I wrote a simple test case and I think our original assumption is wrong, it DOES currently find parent fields and methods. I'm using class.getMethod which is documented to recursively look at parent classes.

Are you able to create a small test case that I can use to reproduce the error?

@decebals
Copy link
Author

decebals commented Sep 1, 2015

I cannot reproduce the issue in a quick start. Indeed if I try something simple everything is OK but in my real application I used a generic-dao with multiple inheritance and the problem persist. I will close this issue and if I find something useful I will reopen it.

@decebals decebals closed this as completed Sep 1, 2015
@decebals
Copy link
Author

decebals commented Sep 1, 2015

I created a quick start here. Maybe someone take a look and find where is the problem. Thanks!

@decebals decebals reopened this Sep 1, 2015
@mbosecke
Copy link
Collaborator

mbosecke commented Sep 2, 2015

I just made a commit that I believe fixes your issue. This commit changes the way Pebble looks for methods; it's much more relaxed when comparing argument types. If you get a chance to confirm the fix, that'd be great.

@decebals
Copy link
Author

decebals commented Sep 2, 2015

I confirm that your commit resolves the issue.

@mbosecke mbosecke closed this as completed Sep 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mbosecke @jcarvalho @decebals @UshakovVasilii