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

Add support for zip protocol to ClassPathUtils #1235

Closed
ukchucktown opened this issue Mar 4, 2015 · 9 comments
Closed

Add support for zip protocol to ClassPathUtils #1235

ukchucktown opened this issue Mar 4, 2015 · 9 comments
Milestone

Comments

@ukchucktown
Copy link

@ukchucktown ukchucktown commented Mar 4, 2015

When calling classLoader.getResources(packagePath) WebLogic returns "zip" as the protocol and the current code in scanPackage only handles "jar" as the protocol and will not call the scanJar method resulting in an illegal argument exception.

@timowest
Copy link
Member

@timowest timowest commented Mar 10, 2015

Maybe this library should be considered as a replacement to the ClassPathUtils usage http://code.google.com/p/reflections/

@Shredder121
Copy link
Member

@Shredder121 Shredder121 commented Mar 10, 2015

We already have it in our test classpath, and I personally use it in a few of our projects.
I am very fond of the way it is implemented, and the things you can do with it, but the question is 'do we want to impose this to the users of Querydsl'.

@timowest
Copy link
Member

@timowest timowest commented Mar 10, 2015

ClassPathUtils is mostly used from the codegen modules and since those are not used at runtime, the extra dependency there shouldn't hurt that much.

@Shredder121
Copy link
Member

@Shredder121 Shredder121 commented Mar 10, 2015

It isn't exactly the dependency that is the problem. I think the imported classpath is not quite right.
I think that at least javassist should be excluded, since it is probably 'included' by mistake?
Apart from that, indeed, the extra dependency shouldn't hurt too much.

@bcubk
Copy link
Contributor

@bcubk bcubk commented Apr 6, 2015

I'm currently working on this bug/task for contribution and I excluded the javassist dependency in the pom file. But I get following Exception:

java.lang.NoClassDefFoundError: javassist/bytecode/ClassFile
    at org.reflections.adapters.JavassistAdapter.getOfCreateClassObject(JavassistAdapter.java:100)
    at org.reflections.adapters.JavassistAdapter.getOfCreateClassObject(JavassistAdapter.java:24)
    at org.reflections.scanners.AbstractScanner.scan(AbstractScanner.java:30)
    at org.reflections.Reflections.scan(Reflections.java:243)
    at org.reflections.Reflections.scan(Reflections.java:203)
    at org.reflections.Reflections.<init>(Reflections.java:128)
    at com.querydsl.core.util.ClassPathUtils.scanPackage(ClassPathUtils.java:49)
    at com.querydsl.core.util.ClassPathUtils.scanPackage(ClassPathUtils.java:40)

It can't be a mistake.

@timowest
Copy link
Member

@timowest timowest commented Apr 6, 2015

What do the reflections docs say on the javassist dependency? For which
functionality is it needed?
On 6 Apr 2015 17:04, "Baris Cubukcuoglu" notifications@github.com wrote:

I'm currently working on this bug/task for contribution and I excluded the
javassist dependency in the pom file. But I get following Exception:

java.lang.NoClassDefFoundError: javassist/bytecode/ClassFile
at org.reflections.adapters.JavassistAdapter.getOfCreateClassObject(JavassistAdapter.java:100)
at org.reflections.adapters.JavassistAdapter.getOfCreateClassObject(JavassistAdapter.java:24)
at org.reflections.scanners.AbstractScanner.scan(AbstractScanner.java:30)
at org.reflections.Reflections.scan(Reflections.java:243)
at org.reflections.Reflections.scan(Reflections.java:203)
at org.reflections.Reflections.(Reflections.java:128)
at com.querydsl.core.util.ClassPathUtils.scanPackage(ClassPathUtils.java:49)
at com.querydsl.core.util.ClassPathUtils.scanPackage(ClassPathUtils.java:40)

It can't be a mistake.


Reply to this email directly or view it on GitHub
#1235 (comment).

@Shredder121
Copy link
Member

@Shredder121 Shredder121 commented Apr 6, 2015

I believe it is to read the class' metadata without loading it via java.lang.reflect means.
So I suppose it was not a mistake indeed, but it is a little counter intuitive.

-------- Original message --------
From: Timo Westkämper notifications@github.com
Date: 06/04/2015 18:09 (GMT+01:00)
To: querydsl/querydsl querydsl@noreply.github.com
Cc: Ruben Dijkstra ruben_dijkstra123@hotmail.com
Subject: Re: [querydsl] Add support for zip protocol to ClassPathUtils (#1235)

What do the reflections docs say on the javassist dependency? For which
functionality is it needed?
On 6 Apr 2015 17:04, "Baris Cubukcuoglu" notifications@github.com wrote:

I'm currently working on this bug/task for contribution and I excluded the
javassist dependency in the pom file. But I get following Exception:

java.lang.NoClassDefFoundError: javassist/bytecode/ClassFile
at org.reflections.adapters.JavassistAdapter.getOfCreateClassObject(JavassistAdapter.java:100)
at org.reflections.adapters.JavassistAdapter.getOfCreateClassObject(JavassistAdapter.java:24)
at org.reflections.scanners.AbstractScanner.scan(AbstractScanner.java:30)
at org.reflections.Reflections.scan(Reflections.java:243)
at org.reflections.Reflections.scan(Reflections.java:203)
at org.reflections.Reflections.(Reflections.java:128)
at com.querydsl.core.util.ClassPathUtils.scanPackage(ClassPathUtils.java:49)
at com.querydsl.core.util.ClassPathUtils.scanPackage(ClassPathUtils.java:40)

It can't be a mistake.


Reply to this email directly or view it on GitHub
#1235 (comment).


Reply to this email directly or view it on GitHub:
#1235 (comment)

@bcubk
Copy link
Contributor

@bcubk bcubk commented Apr 6, 2015

Yes, that's right. I'm going to examaine the docs in detail.

@bcubk
Copy link
Contributor

@bcubk bcubk commented Apr 6, 2015

Ah, I got it. The ConfigurationBuilder provides a setter to override the default "MetadataAdapter" to fetch the metadata of the classes. The default adapter uses Javassist. I switched it to the "JavaReflectionAdapter" and my test case passes through. I will provide a PR the next days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.