-
Notifications
You must be signed in to change notification settings - Fork 40
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
Upgrade Jackson 1.x to Jackson 2.x #22
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,13 +9,10 @@ | |
import com.socrata.model.search.SearchClause; | ||
import com.sun.jersey.api.client.ClientResponse; | ||
import com.sun.jersey.api.client.GenericType; | ||
import org.codehaus.jackson.JsonParseException; | ||
import org.codehaus.jackson.map.JsonMappingException; | ||
|
||
import javax.ws.rs.core.MediaType; | ||
import javax.ws.rs.core.UriBuilder; | ||
import java.io.File; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.net.URI; | ||
|
||
|
@@ -355,16 +352,9 @@ public ClientResponse issueRequest() throws LongRunningQueryException, SodaError | |
|
||
try { | ||
final ClientResponse response = requester.issueRequest(); | ||
return mapper.readValue(response.getEntity(InputStream.class), AssetResponse.class); | ||
//return response.getEntity(AssetResponse.class); | ||
return response.getEntity(AssetResponse.class); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. response.getEntity doesn't work with creating assets /api/assets. Keep the original mapper.readValue and error handlers. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I cannot find any documentation for the "assets" resource of the Import API. Does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. jackson looks for json content-type in getEntity. But the actual content type returned from the server is text/plain despite that it is json. Using readValue from input stream get around this problem. |
||
} catch (LongRunningQueryException e) { | ||
return getHttpLowLevel().getAsyncResults(e.location, e.timeToRetry, getHttpLowLevel().getMaxRetries(), AssetResponse.class, requester); | ||
} catch (JsonMappingException e) { | ||
throw new SodaError("Illegal response from the service."); | ||
} catch (JsonParseException e) { | ||
throw new SodaError("Invalid JSON returned from the service."); | ||
} catch (IOException e) { | ||
throw new SodaError("Error communicating with service."); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor suggestion: since ObjectMapper is stateless, you can maintain a single ObjectMapper object as singleton across the code base.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not critical. Can push it off to next release
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, but since this isn't a new field, I was considering this to be out of scope; however, I did add a new constructor signature (line 188) so that the application may provide the library with its singleton ObjectMapper