katharsis-framework
katharsis-framework copied to clipboard
DefaultQuerySpecDeserializer is used instead of deserializer in custom ObjectMapper
We have our own ObjectMapper with serializer/deserializer attached and it worked like a charm before we updated to katharsis v2.8.2 - QuerySpec. ObjectMapper is used for serialization and it serializes correctly, but it's not used for deserialization. Instead DefaultQuerySpecDeserializer tries to parse fields which causes exception below.
io.katharsis.utils.parser.ParserException: Cannot parse to org.joda.time.DateTime : 2017-01-18 at io.katharsis.utils.parser.TypeParser.parse(TypeParser.java:61) ~[katharsis-core-2.8.2.jar:?] at io.katharsis.queryspec.DefaultQuerySpecDeserializer.deserializeFilter(DefaultQuerySpecDeserializer.java:224) ~[katharsis-core-2.8.2.jar:?] at io.katharsis.queryspec.DefaultQuerySpecDeserializer.deserialize(DefaultQuerySpecDeserializer.java:129) ~[katharsis-core-2.8.2.jar:?] at io.katharsis.queryspec.internal.QuerySpecAdapterBuilder.build(QuerySpecAdapterBuilder.java:29) ~[katharsis-core-2.8.2.jar:?] at io.katharsis.dispatcher.RequestDispatcher.dispatchRequest(RequestDispatcher.java:75) [katharsis-core-2.8.2.jar:?] at io.katharsis.spring.KatharsisFilterV2.dispatchRequest(KatharsisFilterV2.java:128) [katharsis-spring-2.8.2.jar:?] at io.katharsis.spring.KatharsisFilterV2.invoke(KatharsisFilterV2.java:102) [katharsis-spring-2.8.2.jar:?] at io.katharsis.spring.KatharsisFilterV2.doFilter(KatharsisFilterV2.java:87) [katharsis-spring-2.8.2.jar:?] at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:192) [tomcat-embed-core-8.5.6.jar:8.5.6]
Our object mapper:
@Component
public class CustomObjectMapper extends ObjectMapper {
public CustomObjectMapper() {
SimpleModule module = new SimpleModule();
module.addSerializer(DateTime.class, new DateTimeSerializer());
module.addDeserializer(DateTime.class, new DateTimeDeserializer());
this.registerModule(module);
}
}
@remmeier - you have been the most involved in the serializer. Perhaps you can take a look.
I am also running into this (or at least it's very similar).
I am not using Joda Time but instead just standard java.util.Date objects. Unless I format the date string to work with the (deprecated) Date constructor that takes a string argument, katharsis is unable to parse it.
Can you create a branch/sample of the issue?
On Sun, Jan 29, 2017, 9:56 PM Dustin Stanley [email protected] wrote:
I am also running into this (or at least it's very similar).
I am not using Joda Time but instead just standard java.util.Date objects. Unless I format the date string to work with the (deprecated) Date constructor that takes a string argument, katharsis is unable to parse it.
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/katharsis-project/katharsis-framework/issues/300#issuecomment-275985559, or mute the thread https://github.com/notifications/unsubscribe-auth/ABaI0Ewnhq9sYy5Fo53jY2fsqNE0jWd9ks5rXXuXgaJpZM4LpC4s .
refactoring is mostly complete, so this can be addressed. Should be a easy PR for TypeParser to also handle the date/time classes by looking for a static parse method.
It doesn't seem like looking for a static parse method would be enough, at least in the case of the Date class, where the constructor(string) internally calls Date's static parse(string) and I'm back to square 1 :-)
I was thinking that maybe TypeParser should be extended to look for any registered Jackson serializers/deserializers and use those if any are registered for the current type.
why that? so far it does not call any parsing. The TypeParser and Jackson are/should not be related, they are used in different contexts.
At some point TypeParser should also be extensible, but I think here it should not be necessary?
I suggested Jackson because we can use it now to parse dates with a specific format when processing request/response bodies. I was assuming that if your API used a specific date format then you could have the format defined in one place and then use that format when processing the request/response bodies as well as the query string params. It's just a suggestion though, I'm not intimately familiar with the framework and not sure how good of a solution that would be.
If the TypeParser is not extensible, does that mean then that the query string params for certain types (like Dates) need to adhere to a specific format that is not configurable? Or am I missing something?
Not sure about the Jackson. Maybe yes, TypeParser could make use of it. Maybe at first trough a feature flag in KatharsisProperties to have the possibility to enable/disable it. PR would be welcomed.
A second thing would the the possibility to allowo further customizations of the TypeParser by allowing to register a Parser for a given type. I could imagine there are use cases where the TypeParser and Jackson parser could different.
Potentially there are multiple instances TypeParser floating around in the Katharsis sources, that may need to be cleanup.
I just upgrade from queryParms to querySpec. I really want disable type parser feature because our application have complex parsing work.
e.g.; java.util.Date type, in some filter, we need ignore day. 2009-01-21 equals 2009-01.
if we disable type parser, we also may disable “in memory query” at the some time.
BTW, if typeParse is configurable both in Type level and Attribute Level, I think that most case will be resolved.
e.g.; java.util.Date type, in some filter, we need ignore day. 2009-01-21 equals 2009-01.
that could be resolved with a custom "FilterOperator" (like eqYearMonth instead of eq). And the FilterOperator could specify what input type it expects. In most cases it will the type of the attribute. But not in this case here.
a flag to disable it would be useful as well I guess. But it probably is more to ease migration. In the long run the parsing should be able to takeover quite some work.
=> can you add a separate issue since none of this changes affects the TypeParser?
Can we close this issue?
Well, the submitter says there is a breaking change between 2.8.1 and 2.8.2 - An alternative has been proposed. No discussion on a resolution. The submitter can offer if the suggestion works for him or not
I see there is an addParser method for this (in version 3.0 at least). Is there an easy way to access that and add a parser for other types (such as Date, or LocalDateTime) ?
I was able to figure out the above. A new TypeParser can be created for alternative data types (such as Dates) using the following in your Feature class
feature.getBoot().getModuleRegistry().getTypeParser().addParser(Class<?>, TypeParser())
. You just need to create a class that implements StringParser<T>