search

twitter / finatra

Fast, testable, Scala services built on TwitterServer and Finagle
https://twitter.github.io/finatra/
Apache License 2.0
2.27k stars 410 forks source link

Update javax.servlet dependency #478

Closed kunalkapoor closed 5 years ago

kunalkapoor commented 5 years ago

The newest Finatra versions still use an extremely old version of javax.servlet's servlet-api library.

The version currently being used is: https://mvnrepository.com/artifact/javax.servlet/servlet-api/2.5 javax.servlet % servlet-api % 2.5

When using the above with an embedded Wiremock server for tests, all tests fail with the following error:

[info] <head>
[info] <meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"/>
[info] <title>Error 500 </title>
[info] </head>
[info] <body>
[info] <h2>HTTP ERROR: 500</h2>
[info] <p>Problem accessing /vmc/api/operator/sddcs. Reason:
[info] <pre>    javax.servlet.http.HttpServletRequest.isAsyncStarted()Z</pre></p>
[info] <hr /><i><small>Powered by Jetty://</small></i>
[info] </body>
[info] </html>

The wiremock dependency I'm using is:"com.github.tomakehurst" % "wiremock-standalone" % 2.15.0

I was pointed to the error from here which mentions that atleast servlet 3.0 is required for wiremock: https://stackoverflow.com/questions/43327076/wiremock-fails-with-nosuchmethoderror-httpservletresponse-getheader

This entire artifact has been moved to: https://mvnrepository.com/artifact/javax.servlet/javax.servlet-api javax.servlet % javax.servlet-api % 4.0.1

If we want to evict the previous artifact in build.sbt, it doesn't work because the artifact name has changed. For example, if build.sbt looks like below:

libraryDependencies += "com.twitter" %% "finatra-http" % 18.9.1
libraryDependencies += "javax.servlet" % "javax.servlet-api" % 4.0.1

The project downloads both dependencies (servlet-api:2.5 and javax.servlet-api:4.0.1) and (for me) ends up using the older one. It creates two separate dependency folders in ivy2 cache:

~/.ivy2/cache/javax.servlet/servlet-api (version 2.5 --> this is the one being used by my code)
~/.ivy2/cache/javax.servlet/javax.servlet-api (version 4.0.1)

I was able to ensure that the project uses the new dependency by manually excluding the artifact from finatra like below:

libraryDependencies += "com.twitter" %% "finatra-http" % 18.9.1 exclude("javax.servlet", "servlet-api")
libraryDependencies += "javax.servlet" % "javax.servlet-api" % 4.0.1

If there isn't any particular reason for an outdated servlet-api artifact, can this be updated in Finatra directly?

cacoco commented 5 years ago

@kunalkapoor would you like to put up a pull request for this? FWIW, Finatra doesn't use servlets, so I'm not 100% sure why this dependency is here. Thus it is possible that we can just remove it. Looking through the history it looks like it may have snuck in via a merge conflict or that its necessity was removed some time ago but the dependency wasn't dropped.

cacoco commented 5 years ago

Removed the unused dependency in 851009524e0a9f9d40fbc1ef97a0d16ccd93dac5