Skip to content

Commit

Permalink
Initial step for #32. Although just now realized it's wrong... will n…
Browse files Browse the repository at this point in the history
…eed to rework
  • Loading branch information
cowtowncoder committed Oct 10, 2013
1 parent fe806fc commit bafb6fb
Show file tree
Hide file tree
Showing 15 changed files with 204 additions and 96 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public abstract class ProviderBase<
* (never try to serialize instances of these types).
*/
public final static Class<?>[] DEFAULT_UNWRITABLES = new Class<?>[] {
InputStream.class, // as per [Issue#19]
InputStream.class, // as per [Issue#19]
OutputStream.class, Writer.class,
StreamingOutput.class, Response.class
};
Expand Down Expand Up @@ -112,7 +112,7 @@ public abstract class ProviderBase<
/**
* Feature flags set.
*
* @since 2.3.0
* @since 2.3
*/
protected int _jaxRSFeatures;

Expand All @@ -125,6 +125,20 @@ public abstract class ProviderBase<
* View to use for writing if none defined for the end point.
*/
protected Class<?> _defaultWriteView;

/**
* Object used for handling possible {@link ObjectReader} injection.
*
* @since 2.3
*/
protected ObjectReaderInjector _readerInjector;

/**
* Object used for handling possible {@link ObjectWriter} injection.
*
* @since 2.3
*/
protected ObjectWriterInjector _writerInjector;

/*
/**********************************************************
Expand Down Expand Up @@ -176,6 +190,8 @@ public abstract class ProviderBase<

protected ProviderBase(MAPPER_CONFIG mconfig) {
_mapperConfig = mconfig;
_readerInjector = new ObjectReaderInjector();
_writerInjector = new ObjectWriterInjector();
}

/**
Expand Down Expand Up @@ -446,11 +462,43 @@ protected boolean hasMatchingMediaTypeForWriting(MediaType mediaType) {

protected abstract MAPPER _locateMapperViaProvider(Class<?> type, MediaType mediaType);

protected abstract EP_CONFIG _configForReading(MAPPER mapper,
Annotation[] annotations, Class<?> defaultView);
protected EP_CONFIG _configForReading(MAPPER mapper,
Annotation[] annotations, Class<?> defaultView)
{
ObjectReader r = _readerInjector.getAndClear();
if (r == null) {
if (defaultView != null) {
r = mapper.readerWithView(defaultView);
} else {
r = mapper.reader();
}
} else {
r = r.withView(defaultView);
}
return _configForReading(r, annotations);
}

protected EP_CONFIG _configForWriting(MAPPER mapper,
Annotation[] annotations, Class<?> defaultView)
{
ObjectWriter w = _writerInjector.getAndClear();
if (w == null) {
if (defaultView != null) {
w = mapper.writerWithView(defaultView);
} else {
w = mapper.writer();
}
} else {
w = w.withView(defaultView);
}
return _configForWriting(w, annotations);
}

protected abstract EP_CONFIG _configForReading(ObjectReader reader,

This comment has been minimized.

Copy link
@tbroyer

tbroyer Jan 30, 2014

You just broke Resteasy: https://github.com/resteasy/Resteasy/blob/e30b28d6cdd2cdfaaa90e94c471ae88d6b813bbe/jaxrs/providers/jackson2/src/main/java/org/jboss/resteasy/plugins/providers/jackson/ResteasyJackson2Provider.java#L107

I'm using Jongo which uses Jackson 2.3, and Resteasy only works with 2.2 because of this breaking change.

Now maybe Resteasy's provider is not needed anymore (except for their @NoJackson annotation, but I don't use it).

This comment has been minimized.

Copy link
@cowtowncoder

cowtowncoder Jan 30, 2014

Author Member

The only way I know of breakages is by reporting by users, since I do not currently use Resteasy. So this is not intentional breakage, and intention is definitely not to exclude it (in fact, I have hoped to use Resteasy for some new projects).

I will create a bug report for this problem, try to fix it for 2.3.2.

This comment has been minimized.

Copy link
@tbroyer

tbroyer Jan 30, 2014

Well, unfortunately, as soon as you change non-private non-package-private API, you're possibly breaking someone and should keep but @Deprecate the old API. That said, given the name of the method (prefixed with an underscore), maybe Resteasy shouldn't have used it to begin with…

Note: it's finally not really a problem for me as I can just use JacksonJsonProvider instead of ResteasyJackson2Provider. Maybe Resteasy should update to Jackson 2.3 and remove their workarounds, but adding back _configForWriting and _configForReading taking MAPPER as argument (and deprecating them) would allow for a migration period (for those like me who want to use Jackson 2.3 with Resteasy); Ideally, that'd mean of course calling those old methods instead of the new ones, but if that's not possible, given that Resteasy only works around an old bug of Jackson 2.2 that's already fixed in 2.3 (AFAICT) and the issue is that it calls them rather than overrides them, it wouldn't be an issue for Jackson to call the new methods and only provide the old ones as "bridges" to the new ones, without calling them itself.

Thanks for the feedback anyway.

This comment has been minimized.

Copy link
@cowtowncoder

cowtowncoder Jan 30, 2014

Author Member

Yes, I agree in that keeping API compatible is an important goal. In this case I was not aware of sub-classing, but it is not inconceivable in retrospect.

What I hope to achieve in future is bit of regression test suite, to use existing and known extensions to avoid easily preventable breakages.

I will also add an issue for project to make this particular problem solved for next patch release (2.3.2).
And yes, I was thinking along same lines as you regarding fix to use.

Thank you again for reporting this.

Annotation[] annotations);

protected abstract EP_CONFIG _configForWriting(MAPPER mapper,
Annotation[] annotations, Class<?> defaultView);
protected abstract EP_CONFIG _configForWriting(ObjectWriter writer,
Annotation[] annotations);

/*
/**********************************************************
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
import com.fasterxml.jackson.annotation.JacksonAnnotationsInside;
import com.fasterxml.jackson.annotation.JsonRootName;
import com.fasterxml.jackson.annotation.JsonView;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;

import com.fasterxml.jackson.databind.*;

import com.fasterxml.jackson.jaxrs.annotation.JacksonFeatures;

/**
Expand Down Expand Up @@ -84,72 +84,43 @@ protected void addAnnotation(Class<? extends Annotation> type,
}
}

/**
* @deprecated Since 2.3
*/
@Deprecated
protected THIS initReader(ObjectMapper mapper) {
return initReader(mapper, null);
}

@SuppressWarnings("unchecked")
protected THIS initReader(ObjectMapper mapper, Class<?> defaultView)
protected THIS initReader(ObjectReader reader)
{
// first common config
Class<?> view = _activeView;
if (view == null) {
view = defaultView;
}
if (view != null) {
_reader = mapper.readerWithView(view);
} else {
_reader = mapper.reader();
if (_activeView != null) {
reader = reader.withView(_activeView);
}

if (_rootName != null) {
_reader = _reader.withRootName(_rootName);
reader = reader.withRootName(_rootName);
}
// Then deser features
if (_deserEnable != null) {
_reader = _reader.withFeatures(_deserEnable);
reader = reader.withFeatures(_deserEnable);
}
if (_deserDisable != null) {
_reader = _reader.withoutFeatures(_deserDisable);
reader = reader.withoutFeatures(_deserDisable);
}
_reader = reader;
return (THIS) this;
}

/**
* @deprecated Since 2.3
*/
@Deprecated
protected THIS initWriter(ObjectMapper mapper) {
return initWriter(mapper, null);
}

@SuppressWarnings("unchecked")
protected THIS initWriter(ObjectMapper mapper, Class<?> defaultView)
protected THIS initWriter(ObjectWriter writer)
{
// first common config
Class<?> view = _activeView;
if (view == null) {
view = defaultView;
}
if (view != null) {
_writer = mapper.writerWithView(view);
} else {
_writer = mapper.writer();
if (_activeView != null) {
writer = writer.withView(_activeView);
}
if (_rootName != null) {
_writer = _writer.withRootName(_rootName);
writer = writer.withRootName(_rootName);
}
// Then features
if (_serEnable != null) {
_writer = _writer.withFeatures(_serEnable);
writer = writer.withFeatures(_serEnable);
}
if (_serDisable != null) {
_writer = _writer.withoutFeatures(_serDisable);
writer = writer.withoutFeatures(_serDisable);
}
_writer = writer;
return (THIS) this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public enum JaxRSFeature implements ConfigFeature
/* Other
/******************************************************
*/

;

private final boolean _defaultState;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package com.fasterxml.jackson.jaxrs.cfg;

import java.util.concurrent.atomic.AtomicBoolean;

import com.fasterxml.jackson.databind.*;

/**
* Based on ideas from [Issue#32], this class allows "overriding" of {@link ObjectReader}
* that JAX-RS Resource will use; usually this is done from a Servlet or JAX-RS filter
* before execution reaches resource.
*
* @author apemberton@github, Tatu Saloranta
*
* @since 2.3
*/
public class ObjectReaderInjector
{
protected static final ThreadLocal<ObjectReader> _threadLocal = new ThreadLocal<ObjectReader>();

/**
* Simple marker used to optimize out {@link ThreadLocal} access in cases
* where this feature is not being used
*/
protected final AtomicBoolean _hasBeenSet = new AtomicBoolean(false);

public ObjectReaderInjector() { }

public void set(ObjectReader r) {
_hasBeenSet.set(true);
_threadLocal.set(r);
}

public ObjectReader get() {
return _hasBeenSet.get() ? _threadLocal.get() : null;
}

public ObjectReader getAndClear() {
ObjectReader r = get();
if (r != null) {
_threadLocal.remove();
}
return r;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package com.fasterxml.jackson.jaxrs.cfg;

import java.util.concurrent.atomic.AtomicBoolean;

import com.fasterxml.jackson.databind.*;

/**
* Based on ideas from [Issue#32], this class allows "overriding" of {@link ObjectWriter}
* that JAX-RS Resource will use; usually this is done from a Servlet or JAX-RS filter
* before execution reaches resource.
*
* @author apemberton@github, Tatu Saloranta
*
* @since 2.3
*/
public class ObjectWriterInjector
{
protected static final ThreadLocal<ObjectWriter> _threadLocal = new ThreadLocal<ObjectWriter>();

/**
* Simple marker used to optimize out {@link ThreadLocal} access in cases
* where this feature is not being used
*/
protected final AtomicBoolean _hasBeenSet = new AtomicBoolean(false);

public ObjectWriterInjector() { }

public void set(ObjectWriter r) {
_hasBeenSet.set(true);
_threadLocal.set(r);
}

public ObjectWriter get() {
return _hasBeenSet.get() ? _threadLocal.get() : null;
}

public ObjectWriter getAndClear() {
ObjectWriter w = get();
if (w != null) {
_threadLocal.remove();
}
return w;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import com.fasterxml.jackson.databind.BeanProperty;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.PropertyMetadata;
import com.fasterxml.jackson.databind.PropertyName;
import com.fasterxml.jackson.databind.introspect.AnnotationMap;

/**
Expand All @@ -17,7 +19,7 @@
public class EndpointAsBeanProperty
extends BeanProperty.Std
{
public final static String ENDPOINT_NAME = "JAX-RS/endpoint";
public final static PropertyName ENDPOINT_NAME = new PropertyName("JAX-RS/endpoint");

private final static AnnotationMap NO_ANNOTATIONS = new AnnotationMap();

Expand All @@ -27,8 +29,7 @@ public EndpointAsBeanProperty(JavaType type, Annotation[] annotations)
{
// TODO: find and pass wrapper; isRequired marker?
super(ENDPOINT_NAME, type, /*PropertyName wrapperName*/ null,
null, null,
/* isRequired */ false);
null, null, PropertyMetadata.STD_OPTIONAL);
boolean hasAnn = (annotations != null && annotations.length > 0);
if (hasAnn) {
_annotations = new AnnotationMap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,15 +217,15 @@ protected ObjectMapper _locateMapperViaProvider(Class<?> type, MediaType mediaTy
}

@Override
protected JsonEndpointConfig _configForReading(ObjectMapper mapper,
Annotation[] annotations, Class<?> defaultView) {
return JsonEndpointConfig.forReading(mapper, annotations, defaultView);
protected JsonEndpointConfig _configForReading(ObjectReader reader,
Annotation[] annotations) {
return JsonEndpointConfig.forReading(reader, annotations);
}

@Override
protected JsonEndpointConfig _configForWriting(ObjectMapper mapper,
Annotation[] annotations, Class<?> defaultView) {
return JsonEndpointConfig.forWriting(mapper, annotations, defaultView,
protected JsonEndpointConfig _configForWriting(ObjectWriter writer,
Annotation[] annotations) {
return JsonEndpointConfig.forWriting(writer, annotations,
_jsonpFunctionName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ public class JsonEndpointConfig

protected JsonEndpointConfig() { }

public static JsonEndpointConfig forReading(ObjectMapper mapper,
Annotation[] annotations, Class<?> defaultView)
public static JsonEndpointConfig forReading(ObjectReader reader,
Annotation[] annotations)
{
return new JsonEndpointConfig()
.add(annotations, false)
.initReader(mapper, defaultView);
.initReader(reader);
}

public static JsonEndpointConfig forWriting(ObjectMapper mapper,
Annotation[] annotations, Class<?> defaultView,
public static JsonEndpointConfig forWriting(ObjectWriter writer,
Annotation[] annotations,
String defaultJsonpMethod)
{
JsonEndpointConfig config = new JsonEndpointConfig();
Expand All @@ -46,7 +46,7 @@ public static JsonEndpointConfig forWriting(ObjectMapper mapper,
}
return config
.add(annotations, true)
.initWriter(mapper, defaultView)
.initWriter(writer)
;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* Annotation that can be used enable and/or disable various
* features for <code>ObjectReader</code>s and <code>ObjectWriter</code>s.
*
* @deprecated Since 2.2, use shared {@link com.fasterxml.jackson.jaxrs.annotation.JacksonAnnotation} instead
* @deprecated Since 2.2, use shared {@link com.fasterxml.jackson.jaxrs.annotation.JacksonFeatures} instead
*/
@Target({ElementType.ANNOTATION_TYPE, ElementType.METHOD })
@Retention(RetentionPolicy.RUNTIME)
Expand Down
Loading

0 comments on commit bafb6fb

Please sign in to comment.