Skip to content

Conversation

pkraeutli
Copy link
Contributor

I only added the if (aggregations != null) { in line 91. The diff looks big because of the indented block.

Copy link

@jpomykala jpomykala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if blocks are too big in my opinion. It should be moved to smaller methods.

@pkraeutli
Copy link
Contributor Author

I agree, will change it. I did it that way to be consistent with the if in line 88.

Copy link

@jpomykala jpomykala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks quite clean

@sreekanthsnair
Copy link

sreekanthsnair commented Mar 10, 2017

I'm also facing the same issue after upgrading form 2.0.6.RELEASE to 2.1.1, will be hot fixed to 2.1.1.RELEASE sooner else may i know which will be the safest version to upgrade?

@CosminCH
Copy link

CosminCH commented Apr 6, 2017

We are having the same problem with 2.1.1.RELEASE...

@a-poptsov
Copy link

as a workaround before serialization we are doing a trick like this:

 private <T> Page<T> fixEmptyPage(Page<T> page) { 
     AggregatedPageImpl<T> aggregatedPage = (AggregatedPageImpl<T>) page;
        Aggregations aggregations = aggregatedPage.getAggregations();
        if (aggregations == null) {
            Field field = ReflectionUtils.findField(AggregatedPageImpl.class, "aggregations");
            ReflectionUtils.makeAccessible(field);
            ReflectionUtils.setField(field, aggregatedPage, InternalAggregations.EMPTY);
            return aggregatedPage;
        }
        return page;
    }

@CosminCH
Copy link

@a-poptsov thank you for the workaround, I prefer not to do your workaround (would have to add it with todo, just more technical debt..) as I am sure the PR is going to be merged soon. Right, @evelan ?

@pkraeutli pkraeutli changed the base branch from master to 2.2.x April 11, 2017 14:55
@shkmaaz11
Copy link

Hi,
I have the 2.1.1.RELEASE. but still I am facing this issue.

The stack trace is as below

java.lang.NullPointerException: null at org.springframework.data.elasticsearch.core.FacetedPageImpl.processAggregations(FacetedPageImpl.java:89) ~[spring-data-elasticsearch-2.1.0.RELEASE.jar:na]
at org.springframework.data.elasticsearch.core.FacetedPageImpl.getFacets(FacetedPageImpl.java:68) ~[spring-data-elasticsearch-2.1.0.RELEASE.jar:na]
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.7.0_60]
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) ~[na:1.7.0_60]
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.7.0_60]
at java.lang.reflect.Method.invoke(Method.java:606) ~[na:1.7.0_60]
at com.fasterxml.jackson.databind.ser.BeanPropertyWriter.serializeAsField(BeanPropertyWriter.java:664) ~[jackson-databind-2.8.6.jar:2.8.6]
at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:690) ~[jackson-databind-2.8.6.jar:2.8.6]
at com.fasterxml.jackson.databind.ser.BeanSerializer.serialize(BeanSerializer.java:155) ~[jackson-databind-2.8.6.jar:2.8.6]

Can please suggest a work around on this issue.

@sreekanthsnair
Copy link

2.1.2 and 2.1.3 also having the same issue. may i please know, does this issue can be hotfixed to any of the 2.1.X branch ?

@maxtuzz
Copy link

maxtuzz commented May 9, 2017

I am also having the same problem. The fix above works but would be great if we didn't need it.

@CrAzE124
Copy link

+1 having similar problems.

@sreekanthsnair
Copy link

@olivergierke can you please tell us when this is going to be fixed and in which release, due to this issue we could not upgrade the entire spring data stack.

@odrotbohm
Copy link
Member

Unfortunately I can't, as the project is a community project and not run by the core Spring Data team.

@sepe81
Copy link

sepe81 commented May 28, 2017

@akonczak can you please tell us when this is going to be merged and in which release? Due to this issue we could not upgrade the entire spring data stack.

According to https://jira.spring.io/browse/DATAES-274 you are the Assignee of the issue.

@mohsinh
Copy link
Contributor

mohsinh commented May 29, 2017

Hello @pkraeutli

Can you please add at least one test in ElasticsearchTemplateFacetTests.java to cover this bug ?

Or give us request and response as json or java code when you are getting this error, so that we can add test ourself.

fix is simple and easily mergeable afterwords. 👍

@pkraeutli
Copy link
Contributor Author

I added a test @mohsinh
Sorry, didn't see your comment earlier.

@pkraeutli
Copy link
Contributor Author

@mohsinh is the test ok like that? Can this be merged?

@mohsinh
Copy link
Contributor

mohsinh commented Jul 21, 2017

merged with master.

@mohsinh mohsinh closed this Jul 21, 2017
@liuyatao
Copy link

liuyatao commented Aug 4, 2017

I am using the spring boot.The spring boot version is 1.5.6 RELEASED,the spring data elasticsearch version is 2.16, the elasticsearch version is 2.4.5 . Still have the problem:

nested exception is com.fasterxml.jackson.databind.JsonMappingException: (was java.lang.NullPointerException) (through reference chain: org.springframework.data.elasticsearch.core.aggregation.impl.AggregatedPageImpl["facets"])

@pkraeutli pkraeutli deleted the DATAES-274 branch August 7, 2017 13:33
@pkraeutli pkraeutli restored the DATAES-274 branch August 7, 2017 13:37
@sepe81
Copy link

sepe81 commented Aug 7, 2017

@liuyatao According to https://jira.spring.io/browse/DATAES-274 this has been fixed only for version 3.0 RC1 (Kay). I can approve that this is not fixed for version spring-data-elasticsearch-2.1.6 and spring-boot 1.5.6 still has the problem.

@mohsinh Could this also be cherry-picked for the 2.1.x branch (preferably 2.1.7)?

@CosminCH
Copy link

How many thumbs up are needed for a small fix to be backported?

@sreekanthsnair
Copy link

This is not backported to any 2.1.x releases...!, Should we have to create a new issue in spring Jira?

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 this pull request may close these issues.