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.

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

Extracting raw type before creating BeanClass #837

Merged
merged 3 commits into from
Oct 14, 2014
Merged

Extracting raw type before creating BeanClass #837

merged 3 commits into from
Oct 14, 2014

Conversation

felipeweb
Copy link
Contributor

No description provided.

@garcia-jj
Copy link
Member

Please, provide some information about your pull request, and the motivation/goal/(or problem caused) for this. Because we need to understand the goal for each pull request to make sure this change wont break anything.

And, please, provide some test case if possible.

Thank you.

@felipeweb
Copy link
Contributor Author

Sorry @garcia-jj , i forgot i just said to @Turini
if i want get some annotation class and class have scope different of Dependent, the VRaptor go to create a proxy and I will not get the annotations of the object, because the proxy does not handle the annotations of the object, just if anproxy the object

@felipeweb
Copy link
Contributor Author

sorry, but my english is terrible

@garcia-jj
Copy link
Member

Hmm, ok, but next time you send a pull request, please, provide some description about the pull request. This is very important to allow us to understand the real problem, help and review your code, and to make sure the changes don't break anything.

Did you test your change before commit? Take a look in the line 40. You extract the proxy class, but you don't assign to anything. That means: does nothing.

I'm not sure if its the right place to do this check, because this method is used to create internal instance. I think the best choice is create a method getAnnotations/getAnnotation/isAnnotationPresent to get class annotations.

Makes sense, @Turini @lucascs @csokol?

@@ -35,6 +37,7 @@ public DefaultControllerMethod(BeanClass controller, Method method) {
}

public static ControllerMethod instanceFor(Class<?> type, Method method) {
CDIProxies.extractRawTypeIfPossible(type);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use tabs instead of spaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't test yet
I will finish it tomorow
Don't merge yet

Em segunda-feira, 13 de outubro de 2014, Otávio Garcia <
[email protected]> escreveu:

In
vraptor-core/src/main/java/br/com/caelum/vraptor/controller/DefaultControllerMethod.java:

@@ -35,6 +37,7 @@ public DefaultControllerMethod(BeanClass controller, Method method) {
}

public static ControllerMethod instanceFor(Class<?> type, Method method) {
  •    CDIProxies.extractRawTypeIfPossible(type);
    

Use tabs instead of spaces.


Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/837/files#r18797113.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you forgot to assign the result of the method call, Felipe. You
should do something like:

type = CDIProxies.extractEtcEtc(type);

Chico Sokol

On Mon, Oct 13, 2014 at 7:36 PM, Felipe Oliveira [email protected]
wrote:

In
vraptor-core/src/main/java/br/com/caelum/vraptor/controller/DefaultControllerMethod.java:

@@ -35,6 +37,7 @@ public DefaultControllerMethod(BeanClass controller, Method method) {
}

public static ControllerMethod instanceFor(Class<?> type, Method method) {
  •    CDIProxies.extractRawTypeIfPossible(type);
    

I don't test yet I will finish it tomorow Don't merge yet Em
segunda-feira, 13 de outubro de 2014, Otávio Garcia <
[email protected]> escreveu:
… <#1490ba774297fa3d_>
In
vraptor-core/src/main/java/br/com/caelum/vraptor/controller/DefaultControllerMethod.java:

@@ -35,6 +37,7 @@ public DefaultControllerMethod(BeanClass controller,
Method method) { > } > > public static ControllerMethod
instanceFor(Class<?> type, Method method) { > +
CDIProxies.extractRawTypeIfPossible(type); Use tabs instead of spaces. —
Reply to this email directly or view it on GitHub <
https://github.com/caelum/vraptor4/pull/837/files#r18797113>.


Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/837/files#r18799361.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll finish it with @felipeweb today.
After testing on a real app we'll merge here.

@Turini Turini changed the title Anproxy class Extracting raw type before creating bean class Oct 14, 2014
@Turini Turini changed the title Extracting raw type before creating bean class Extracting raw type before creating BeanClass Oct 14, 2014
@Turini
Copy link
Member

Turini commented Oct 14, 2014

With this change we avoid the need to check if BeanClass.getType() is a
CDI proxy on vraptor-brutalth and probably in other places in the future.

any downside for this change?

@garcia-jj
Copy link
Member

PR #733 have introduced a bug causing stackoverflow. So we need to test in real apps before merge.

Have you tested in guj or other app? Tonight I can do some tests in my apps.

@Turini
Copy link
Member

Turini commented Oct 14, 2014

Have you tested in guj or other app?

yes, we've tested on @felipeweb's app (real app)
also on jungle and blank-project

@@ -35,6 +37,7 @@ public DefaultControllerMethod(BeanClass controller, Method method) {
}

public static ControllerMethod instanceFor(Class<?> type, Method method) {
type = CDIProxies.extractRawTypeIfPossible(type);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix indentation (tabs instead spaces). And may you can use static import here.

@felipeweb
Copy link
Contributor Author

Done! i remove space to tabs

Turini added a commit that referenced this pull request Oct 14, 2014
Extracting raw type before creating BeanClass
@Turini Turini merged commit a5ce357 into caelum:master Oct 14, 2014
@Turini
Copy link
Member

Turini commented Oct 14, 2014

Thanks @felipeweb

@felipeweb felipeweb deleted the anproxy branch October 14, 2014 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants