search

krasserm / grails-jaxrs

JAX-RS Plugin for Grails
http://code.google.com/p/grails-jaxrs/
Apache License 2.0
50 stars 48 forks source link

Remove copy-pasted MockHttpServletRequest from plugin. #24

Closed ajbrown closed 11 years ago

ajbrown commented 11 years ago

The MockHttpServletRequest being included in this plugin uses the same namespace as the MockHttpServletRequest class it was copied from, but does not implement the same interface. This causes an IncompatibleClassChangeError runtime error to occur if this version of the class is selected within another class which needs it and expects it to be of the HttpServletRequest type.

The issue above can be reproduced by installing both the JAX-RS and Mail grails plugins. The mail plugin uses a MockhttpServletRequest when rendering mail content, but fails at Runtime because the class is not an HttpServletRequest.

I grepped the code and can't find a reason that this copied class even needs to exist. It appears that all cases where it's used, the method signature is compatible with the real MockHttpServletRequest. Either way, the removed class needs to be placed in a different namespace (or the dependent code refactored) in order to prevent chaos.

davidecavestro commented 11 years ago

That copy-paste beast was added in order to prevent runtime dependencies on spring-test, and then patched to avoid depending upon servlet 3 apis and its new methods. So I guess it would be safer - as a first step - moving it to another namespace as you suggested.

davidecavestro commented 11 years ago

I've just committed a raw refactoring into the springtest_dep_removal branch. Could you please give it a try?

ajbrown commented 11 years ago

grails maven-install and grails compile both fail on the springtest_dep_removal branch after a successful grails clean with the following stacktrace:

https://gist.github.com/ajbrown/6281644

Just to be sure it wasn't an environment thing, I created a clean new project and attempted to load the plugin inline and got a similar compilation error.

davidecavestro commented 11 years ago

Sorry, I used the IDE for my previous commit and I missed adding some additional files copied from spring-test. I've just committed them and pushed to the branch.

ajbrown commented 11 years ago

Ok it's working now. I ran some tests in my development environment and wasn't able to reproduce my previous problem. Production will be the real test, but I don't want to disrupt the current running version unless a related error creeps up.

Any chance you can pull my commit in for the delete so I can get some street cred? :-)

davidecavestro commented 11 years ago

Glad to hear it works now. I'll try to pull your commit before merging :-)

davidecavestro commented 11 years ago

Just merged on trunk the whole fix, namely the _springtest_depremoval branch