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

@Index disableValidation is not used when finding the MappedField #764

Closed
agiannone opened this issue May 6, 2015 · 4 comments
Closed
Labels
Milestone

Comments

@agiannone
Copy link

Prior to the use of the new @Index annotations the disableValidation flag would avoid validating that the MappedField could be found in the class. More importantly it would avoid validating that the field could be found in an Embedded object. This is essential when using Interfaces as the interface will not contain the field.

disableValidation should also disable checking that the MappedField can be found.

The issue is in DatastoreImpl on line 385 - 388:

MappedField mf = findField(namePath, mc, value);
if (mf == null) {
throw new MappingException(format("Unknown field '%s' for index: %s", value, mc.getClazz().getName()));
}

This check should be skipped entirely if the options have disableValidation.

@evanchooly
Copy link
Member

If you can put together a test case and a pull request I can get this in to
rc0. I was hoping to cut that release yesterday but things got in the way.
On May 6, 2015 2:20 PM, "Alessandro Giannone" [email protected]
wrote:

Prior to the use of the new @Index https://github.com/Index annotations
the disableValidation flag would avoid validating that the MappedField
could be found in the class. More importantly it would avoid validating
that the field could be found in an Embedded object. This is essential when
using Interfaces as the interface will not contain the field.

disableValidation should also disable checking that the MappedField can be
found.

The issue is in DatastoreImpl on line 385 - 388:

MappedField mf = findField(namePath, mc, value);
if (mf == null) {
throw new MappingException(format("Unknown field '%s' for index: %s",
value, mc.getClazz().getName()));
}

This check should be skipped entirely if the options have
disableValidation.


Reply to this email directly or view it on GitHub
#764.

evanchooly pushed a commit that referenced this issue May 7, 2015
@evanchooly evanchooly added this to the 1.0.0-rc0 milestone May 7, 2015
@evanchooly evanchooly added the bug label May 7, 2015
@evanchooly
Copy link
Member

@agiannone I just pushed a change that I think will fix your issue. Unfortunately, the test coverage around this is nonexistent but I'll see what I can do to fix that over the next few months. If you could try a snapshot build (or build locally) and let me know that'd be great. I'm going to hopefully push rc0 tomorrow.

@agiannone
Copy link
Author

@evanchooly Thanks, just ran a local build and the issue appears to be fixed. When is the next planned release? (just want to update the Maven property when possible)

@agiannone
Copy link
Author

@evanchooly I just ran our Integration Tests and spotted an issue with the fix. The indexes are not indexed correctly when validation is disabled, however the namePath is used to build the key for the index and the namePath will be empty if the findField() method is not called. This means we end up with an empty key which is indexed and causes save() calls to fail (as it saves the index as _1).

To fix this the following should work:

if (!"$**".equals(value) && !options.disableValidation()) {
                List<String> namePath = new ArrayList<String>();
                if ( findField(namePath, mc, value) == null) {
                    throw new MappingException(format("Unknown field '%s' for index: %s", value, mc.getClazz().getName()));
                } else {
                    StringBuilder sb = new StringBuilder();
                    for (String s : namePath) {
                        if (sb.length() != 0) {
                            sb.append(".");
                        }
                        sb.append(s);
                    }
                    key = sb.toString();
                }
            }

This way the key becomes the actual field.value() value and everything works.

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

No branches or pull requests

2 participants