search

FasterXML / jackson-annotations

Core annotations (annotations that only depend on jackson-core) for Jackson data processor
https://github.com/FasterXML/jackson
Apache License 2.0
1.03k stars 330 forks source link

API for custom Object Identifier resolution. #28

Closed pgelinas closed 10 years ago

cowtowncoder commented 10 years ago

Ok, makes sense. I think we need to talk about code organization -- ideally annotation package would not contain implementation classes. In this case it is bit tricky, wrt defaulting and such.

One possible alternate would be to move ObjectIdResolver (and base implementation) into jackson-databind, and changing type of resolver annotation to be Class<Object>. This in turn would allow resolver itself to be more strongly typed. Defaulting is bit tricky however... could use NoClass.class, and let AnnotationIntrospector handle it.

But I am not sure what would be optimal here -- I'll have to think about this bit more.

pgelinas commented 10 years ago

Sure, no problem. My goal was to provide a sensible first draft of the idea. Really, all I did was inspire myself with what was done for generator: the ObjectIdGenerator API and its default implementation are part of the annotation project, so it made sense to me to put ObjectIdResolver and default implementation (ie keep current behaviour) there.

With regards to typing: I might have been a bit lazy with ObjectIdResolver. It started off with it being generic, but I had some compilation error and generics warning here and there, so I scratched that and made it handle Object. I might do a second pass if you wish and see if I can make it generic, so that it could be more strongly typed.

cowtowncoder commented 10 years ago

Understood. I am not sure I can actually figure out better ways; and the point about ObjectIdGenerator is good -- it is also deviation from ideal.

So in all likelihood I think this is the way to go, but I will go through code once more just in case. Besides, we can still re-factor, 2.4 won't be quite ready in very near future.

cowtowncoder commented 10 years ago

Actually I think this is fine, will merge.