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

Problem with FilteringGeneratorDelegate wrt TokenFilter.Inclusion.INCLUDE_NON_NULL #1023

Closed
honhimW opened this issue May 16, 2023 · 6 comments
Milestone

Comments

@honhimW
Copy link

honhimW commented May 16, 2023

class Entity {
    Pojo[] pojos;
}
class Pojo {
    String id;
    String name;
}

public static void main(String[] args) {
// ...
    JsonGeneratorDelegate delegate = new FilteringGeneratorDelegate(
            generator,
            new JsonPointerBasedFilter(JsonPointer.compile("/pojos/0/name")),
            TokenFilter.Inclusion.INCLUDE_NON_NULL,
            true);
// ...
}

But error occur if TokenFilter#filterStartArray not returning the very TokenFilter.INCLUDE_ALL.

Current context not Array but Object (through reference chain: xxx.xxx.Xxxx["xx"])

I note that FilteringGeneratorDelegate#writeStartArray(Object, int) does not invoke delegate.writeStartArray(forValue, size) when returning JsonGeneratorDelegate itself.
Am i misunderstanding the usage of TokenFilter? And how can i filter properties like this?
Sincerely looking forward to your reply.

@honhimW
Copy link
Author

honhimW commented May 16, 2023

Well, I've figured it out.

  • com.fasterxml.jackson.core.filter.FilteringGeneratorDelegate#writeStartArray()
  • com.fasterxml.jackson.core.filter.FilteringGeneratorDelegate#writeStartArray(int)
  • com.fasterxml.jackson.core.filter.FilteringGeneratorDelegate#writeStartArray(java.lang.Object)
  • com.fasterxml.jackson.core.filter.FilteringGeneratorDelegate#writeStartArray(java.lang.Object, int)

Methods abbove seems not having the same action in situation_itemFilter != null && _inclusion == Inclusion.INCLUDE_NON_NULL.
Is this a bug or is it designed that way?

Demo

public class JacksonFilterTests {

    public static void main(String[] args) throws Exception {
        Entity entity = get();
        System.out.println(include(entity, "/id", "/gender", "/pojos/*/firstName", "/pojos/1/lastName"));
    }

    public static String include(Object value, String... includes) throws Exception {
        ObjectMapper mapper = new ObjectMapper();
        SegmentedStringWriter w = new SegmentedStringWriter(mapper.getFactory()._getBufferRecycler());
        JsonGenerator generator = mapper.createGenerator(w);
        generator.useDefaultPrettyPrinter();
        PointerFilteringGenerator delegate = new PointerFilteringGenerator(
            generator,
            new JsonPointersFilter(includes),
            TokenFilter.Inclusion.INCLUDE_NON_NULL,
            true);
        mapper.writeValue(delegate, value);
        return w.getAndClear();
    }

    public static class JsonPointersFilter extends TokenFilter {

        public static final String ANY_INDEX = "*";

        private final Set<JsonPointer> _includes = new HashSet<>();

        public JsonPointersFilter(String... includes) {
            Set<String> set = Set.of(includes);
            set.stream()
                .map(JsonPointer::compile)
                .forEach(_includes::add);
        }

        public JsonPointersFilter(Set<JsonPointer> includes) {
            _includes.addAll(includes);
        }

        @Override
        public TokenFilter includeElement(int index) {
            Set<JsonPointer> next = new HashSet<>();
            for (JsonPointer pointer : _includes) {
                if (StringUtils.equals(pointer.getMatchingProperty(), ANY_INDEX)) {
                    next.add(pointer.matchProperty(ANY_INDEX));
                } else {
                    JsonPointer _next = pointer.matchElement(index);
                    if (Objects.nonNull(_next)) {
                        next.add(_next);
                    }
                }
            }
            if (CollectionUtils.isNotEmpty(next)) {
                boolean b = next.stream().anyMatch(JsonPointer::matches);
                if (b) {
                    return TokenFilter.INCLUDE_ALL;
                }
                return new JsonPointersFilter(next);
            } else {
                return new JsonPointersFilter();
            }
        }

        @Override
        public TokenFilter includeProperty(String name) {
            Set<JsonPointer> set = _includes.stream()
                .map(jsonPointer -> jsonPointer.matchProperty(name))
                .filter(Objects::nonNull)
                .collect(Collectors.toSet());
            if (CollectionUtils.isNotEmpty(set)) {
                boolean b = set.stream().anyMatch(JsonPointer::matches);
                if (b) {
                    return TokenFilter.INCLUDE_ALL;
                }
                return new JsonPointersFilter(set);
            } else {
                return null;
            }
        }

        @Override
        public TokenFilter filterStartObject() {
            if (CollectionUtils.isEmpty(_includes)) {
                return null;
            }
            return super.filterStartObject();
        }

    }

    public static class PointerFilteringGenerator extends FilteringGeneratorDelegate {

        public PointerFilteringGenerator(JsonGenerator d, TokenFilter f, TokenFilter.Inclusion inclusion, boolean allowMultipleMatches) {
            super(d, f, inclusion, allowMultipleMatches);
        }

        @Override
        public void writeStartArray(Object forValue) throws IOException
        {
            if (_itemFilter == null) {
                _filterContext = _filterContext.createChildArrayContext(null, false);
                return;
            }
            if (_itemFilter == TokenFilter.INCLUDE_ALL) {
                _filterContext = _filterContext.createChildArrayContext(_itemFilter, true);
                delegate.writeStartArray(forValue);
                return;
            }
            _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);
            } else if (_itemFilter != null && _inclusion == TokenFilter.Inclusion.INCLUDE_NON_NULL) {
                _checkParentPath(false);
                _filterContext = _filterContext.createChildArrayContext(_itemFilter, true);
                delegate.writeStartArray(forValue);
            } else {
                _filterContext = _filterContext.createChildArrayContext(_itemFilter, false);
            }
        }

        @Override
        public void writeStartArray(Object forValue, int size) throws IOException
        {
            if (_itemFilter == null) {
                _filterContext = _filterContext.createChildArrayContext(null, false);
                return;
            }
            if (_itemFilter == TokenFilter.INCLUDE_ALL) {
                _filterContext = _filterContext.createChildArrayContext(_itemFilter, true);
                delegate.writeStartArray(forValue, size);
                return;
            }
            _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 if (_itemFilter != null && _inclusion == TokenFilter.Inclusion.INCLUDE_NON_NULL) {
                _checkParentPath(false);
                _filterContext = _filterContext.createChildArrayContext(_itemFilter, true);
                delegate.writeStartArray(forValue, size);
            } else {
                _filterContext = _filterContext.createChildArrayContext(_itemFilter, false);
            }
        }
    }

    private static Entity get() {
        Entity entity = new Entity();
        entity.setId("1");
        entity.setAge(18);
        entity.setGender(true);
        Pojo sub = new Pojo();
        sub.setFirstName("hello");
        sub.setLastName("world");
        entity.setSub(sub);
        entity.setPojos(new Pojo[]{sub, sub});
        return entity;
    }

    @Data
    public static class Entity implements Serializable {
        private String id;
        private Integer age;
        private Boolean gender;
        private Pojo sub;
        private Pojo[] pojos;
    }
    @Data
    public static class Pojo implements Serializable {
        private String firstName;
        private String lastName;
    }

}

@cowtowncoder
Copy link
Member

@honhimW I think you may have found a bug: looking at variants of writeStartArray(), first two (no-args, int) differ from last 2 -- and I think this must be due to an earlier fix that was incomplete, basically missing this:

    } else if (_itemFilter != null && _inclusion == Inclusion.INCLUDE_NON_NULL) {

part of handling.

@cowtowncoder cowtowncoder changed the title How to use TokenFilter(e.g. JsonPointerBasedFilter) to specifices path containing array? Problem with FilteringGeneratorDelegate wrt TokenFilter.Inclusion.INCLUDE_NON_NULL May 16, 2023
cowtowncoder added a commit that referenced this issue May 16, 2023
@cowtowncoder cowtowncoder added this to the 2.10 milestone May 16, 2023
@cowtowncoder cowtowncoder modified the milestones: 2.10, 2.15.1 May 16, 2023
@cowtowncoder
Copy link
Member

@honhimW I fixed this in 2.15 branch, but there's no test for it yet. Would be great if you could build locally 2.15.1-SNAPSHOT and verify, or (if possible) maybe provide a compact test case?

@honhimW
Copy link
Author

honhimW commented May 17, 2023

Thanks for your reply, but i think it's hard to test without databind-module.🤔

@pjfanning
Copy link
Member

Thanks for your reply, but i think it's hard to test without databind-module.🤔

we could put a test in jackson-databind project

@cowtowncoder
Copy link
Member

@honhimW Yes, adding tests in jackson-databind where necessary is fine.

Also: I released 2.15.1 yesterday so testing should be easier now.

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

Successfully merging a pull request may close this issue.

3 participants