WINDUP-952 Multi-application support for Spring bean report #860
Conversation
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
|
@windupgithubbot1 retest |
1 similar comment
|
@windupgithubbot1 retest |
| if (application.equals(model.getApplication())) | ||
| { | ||
| models.add(model); | ||
| } |
There was a problem hiding this comment.
It would be better to filter at the DB query level. Imagine getting 1 000 000 spring beans from 1000 apps. That means 1000 x running through 1 000 000 beans filtering out a fraction of them. Not that I expect 1mil beans, but we should pay attention to performance.
There was a problem hiding this comment.
@OndraZizka +1 - I changed this while merging.
There was a problem hiding this comment.
Makes sense. I was not sure how to implement this properly. Anyway, we should probably review all other rules as well. For instance, CreateJPAReportRuleProvider gets and filters all the JPA entities...
There was a problem hiding this comment.
Yes, those could likely be improved as well. I did go through them to find any cases where we weren't filtering by app (hopefully I found them all).
Note that this PR does not contain a test. Actually, I'm not quite sure how to write such a test yet. But I will try to do my best if it's required ;-)