Skip to content
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

`FilteringGeneratorDelegate` bug when filtering arrays (in 2.10.1) #582

Closed
alarribeau opened this issue Nov 29, 2019 · 4 comments
Closed

`FilteringGeneratorDelegate` bug when filtering arrays (in 2.10.1) #582

alarribeau opened this issue Nov 29, 2019 · 4 comments
Labels
Milestone

Comments

@alarribeau
Copy link

@alarribeau alarribeau commented Nov 29, 2019

Since 2.10.1, it is not possible to filter out object arrays anymore.

The regression has been introduced by this (jackson-databind) commit which changed the array serialization implementation (more specifically the writeStartArray methods used):
FasterXML/jackson-databind@ca1867e

So it seems that the root cause is that the FilteringGeneratorDelegate (jackson-core) should override all JsonGenerator::writeStartArray methods.

Here is a maven project with 1 test class that reproduces the bug and offers a workaround :
filterGeneratorBug.zip

Test class :

package test;

import java.io.IOException;
import java.io.StringWriter;

import org.junit.Assert;
import org.junit.Test;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.filter.FilteringGeneratorDelegate;
import com.fasterxml.jackson.core.filter.JsonPointerBasedFilter;
import com.fasterxml.jackson.core.filter.TokenFilter;
import com.fasterxml.jackson.databind.ObjectMapper;

public class ArrayFilteringBugTest {
	
	private static final ArrayWrapper ARRAY_WRAPPER = new ArrayWrapper(new String[] { "foo", "bar" });
	
	static class ArrayWrapper {
		@JsonProperty
		private final String[] stringArray;
		
		ArrayWrapper(String[] stringArray) {
			this.stringArray = stringArray;
		}
	}
	
	@Test
	public void arrayFilterOut_failing() throws IOException {
		StringWriter output = new StringWriter();
		JsonGenerator jg = new JsonFactory().createGenerator(output);
		
		FilteringGeneratorDelegate filteringGeneratorDelegate = new FilteringGeneratorDelegate(jg, new JsonPointerBasedFilter("/noMatch"), true, true);
		
		new ObjectMapper().writeValue(filteringGeneratorDelegate, ARRAY_WRAPPER);
		
		Assert.assertEquals("", output.toString());
	}
	
	@Test
	public void arrayFilterOut_workaroundFix() throws IOException {
		
		StringWriter output = new StringWriter();
		JsonGenerator jg = new JsonFactory().createGenerator(output);
		
		FilteringGeneratorDelegate filteringGeneratorDelegate = new FixedFilteringGeneratorDelegate(jg, new JsonPointerBasedFilter("/noMatch"), true, true);
		
		new ObjectMapper().writeValue(filteringGeneratorDelegate, ARRAY_WRAPPER);
		
		Assert.assertEquals("", output.toString());
	}
	
	static class FixedFilteringGeneratorDelegate extends FilteringGeneratorDelegate {
		
		public FixedFilteringGeneratorDelegate(JsonGenerator d, TokenFilter f, boolean includePath, boolean allowMultipleMatches) {
			super(d, f, includePath, allowMultipleMatches);
		}
		
//		!! This method may also need overriding when called by other binders !!
//		@Override
//		public void writeStartArray(Object forValue) throws IOException;
		
		
		@Override
		public void writeStartArray(Object forValue, int size) throws IOException
		{
			// First things first: whole-sale skipping easy
			if (_itemFilter == null) {
				_filterContext = _filterContext.createChildArrayContext(null, false);
				return;
			}
			if (_itemFilter == TokenFilter.INCLUDE_ALL) { // include the whole sub-tree?
				_filterContext = _filterContext.createChildArrayContext(_itemFilter, true);
				delegate.writeStartArray(forValue, size);
				return;
			}
			// Ok; regular checking state then
			_itemFilter = _filterContext.checkValue(_itemFilter);
			if (_itemFilter == null) {
				_filterContext = _filterContext.createChildArrayContext(null, false);
				return;
			}
			if (_itemFilter != TokenFilter.INCLUDE_ALL) {
				_itemFilter = _itemFilter.filterStartArray();
			}
			if (_itemFilter == TokenFilter.INCLUDE_ALL) {
				_checkParentPath();
				_filterContext = _filterContext.createChildArrayContext(_itemFilter, true);
				delegate.writeStartArray(forValue, size);
			} else {
				_filterContext = _filterContext.createChildArrayContext(_itemFilter, false);
			}
		}
	}
	
}
@cowtowncoder

This comment has been minimized.

Copy link
Member

@cowtowncoder cowtowncoder commented Dec 1, 2019

First of all: thank you for submitting this -- and I think you are right about analysis.

Second of all: rats. It is very unfortunate regression; I wish I had realized the issue with the fix.

One good thing is that at least it should get in 2.10.2 fix; I'll put this at the top of my work queue.

@cowtowncoder

This comment has been minimized.

Copy link
Member

@cowtowncoder cowtowncoder commented Dec 2, 2019

I'll need to change a test little bit since core package can not depend on jackson-databind.

@cowtowncoder cowtowncoder added 2.10 active and removed active labels Dec 2, 2019
@cowtowncoder cowtowncoder added this to the 2.10.2 milestone Dec 3, 2019
@cowtowncoder cowtowncoder changed the title 2.10.1 FilteringGeneratorDelegate bug when filtering arrays `FilteringGeneratorDelegate` bug when filtering arrays (in 2.10.1) Dec 3, 2019
@alarribeau

This comment has been minimized.

Copy link
Author

@alarribeau alarribeau commented Dec 3, 2019

First of all: thank you for submitting this -- and I think you are right about analysis.
You're welcome :)

Thanks for the quick fix !

@cowtowncoder

This comment has been minimized.

Copy link
Member

@cowtowncoder cowtowncoder commented Dec 3, 2019

Thank you reporting this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.