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

Fix Query/BasicQuery getFieldsObject() inconsistency #345

Closed
wants to merge 1 commit into from

Conversation

jdigweed
Copy link

@jdigweed jdigweed commented Mar 3, 2016

Previously, Query.getFieldsObject() and BasicQuery.getFieldsObject()
would behave differently in the case of fields().include(). For
example:
Query query = new Query(jsonStr);
query.fields().include("name");
DBObject fieldsObject = query.getFieldsObject();
// fieldsObject contains name, based on the fieldspec

However,
Query query = new BasicQuery(jsonStr);
query.fields().include("name");
DBObject fieldsObject = query.getFieldsObject();
// fieldsObject is null, because BasicQuery was not created
// with a fieldsObject in the constructor

This fix changes BasicQuery to use the parent's getFieldsObject() in
the event that the BasicQuery was created without an explicit
fieldsObject in the constructor. In the case when an explicit
not-null FieldsObject was provided, it will defer to that
FieldsObject over using the parent class's getFieldsObject().

I ran into this while using BasicQuery but trying to use an
enumerated list of fields. I was confused why fields().include()
was working for Query but not BasicQuery. After stepping through
it in the debugger, I filed DATAMONGO-1387.

Issue: DATAMONGO-1387

My contributor number is: 165520160303021604

Previously, Query.getFieldsObject() and BasicQuery.getFieldsObject()
would behave differently in the case of fields().include(). For
example:
 Query query = new Query(jsonStr);
 query.fields().include("name");
 DBObject fieldsObject = query.getFieldsObject();
 // fieldsObject contains name, based on the fieldspec

However,
 Query query = new BasicQuery(jsonStr);
 query.fields().include("name");
 DBObject fieldsObject = query.getFieldsObject();
 // fieldsObject is null, because BasicQuery was not created
 // with a fieldsObject in the constructor

This fix changes BasicQuery to use the parent's getFieldsObject() in
the event that the BasicQuery was created without an explicit
fieldsObject in the constructor.

Issue: DATAMONGO-1387
christophstrobl pushed a commit that referenced this pull request Mar 16, 2016
We changed BasicQuery to consider its parent getFieldsObject() when not given an explicit fields DBObject.

Original Pull Request: #345
CLA: 165520160303021604 (John Willemin)
christophstrobl pushed a commit that referenced this pull request Mar 17, 2016
We changed BasicQuery to consider its parent getFieldsObject() when not given an explicit fields DBObject.

Original Pull Request: #345
CLA: 165520160303021604 (John Willemin)
christophstrobl added a commit that referenced this pull request Mar 17, 2016
Added a few more tests and append values if present on Query.

Original Pull Request: #345
@christophstrobl
Copy link
Member

thanks @jdigweed . merged via 119692c.

christophstrobl pushed a commit that referenced this pull request Mar 18, 2016
We changed BasicQuery to consider its parent getFieldsObject() when not given an explicit fields DBObject.

Original Pull Request: #345
CLA: 165520160303021604 (John Willemin)
christophstrobl added a commit that referenced this pull request Mar 18, 2016
Added a few more tests and append values if present on Query.

Original Pull Request: #345
christophstrobl pushed a commit that referenced this pull request Mar 18, 2016
We changed BasicQuery to consider its parent getFieldsObject() when not given an explicit fields DBObject.

Original Pull Request: #345
CLA: 165520160303021604 (John Willemin)
christophstrobl added a commit that referenced this pull request Mar 18, 2016
Added a few more tests and append values if present on Query.

Original Pull Request: #345
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.

2 participants