From d0d9e04975717a1b74204a3b15183bfbd1d99f5d Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Tue, 1 Nov 2016 12:34:10 -0400 Subject: [PATCH] make the path validation smarter to avoid and hopefully eliminate the need for disabling validation. fixes #379 fixes #1060 --- config/clirr-exclude.yml | 10 +- .../morphia/internal/MorphiaUtils.java | 48 ++++++ .../mongodb/morphia/internal/PathTarget.java | 155 ++++++++++++++++++ .../morphia/internal/package-info.java | 20 +++ .../mongodb/morphia/query/UpdateOpsImpl.java | 36 ++-- .../org/mongodb/morphia/IndexHelperTest.java | 2 +- .../org/mongodb/morphia/TestUpdateOps.java | 25 +++ .../morphia/entities/EmbeddedType.java | 3 + .../mongodb/morphia/entities/ParentType.java | 3 + .../morphia/internal/PathTargetTest.java | 120 ++++++++++++++ 10 files changed, 393 insertions(+), 29 deletions(-) create mode 100644 morphia/src/main/java/org/mongodb/morphia/internal/MorphiaUtils.java create mode 100644 morphia/src/main/java/org/mongodb/morphia/internal/PathTarget.java create mode 100644 morphia/src/main/java/org/mongodb/morphia/internal/package-info.java create mode 100644 morphia/src/test/java/org/mongodb/morphia/internal/PathTargetTest.java diff --git a/config/clirr-exclude.yml b/config/clirr-exclude.yml index a11b3ae846d..c84a6ff7254 100644 --- a/config/clirr-exclude.yml +++ b/config/clirr-exclude.yml @@ -2,12 +2,4 @@ differenceTypes: [] packages: -- net.sf.cglib.asm -- net.sf.cglib.asm.signature -- net.sf.cglib.beans -- net.sf.cglib.core -- net.sf.cglib.proxy -- net.sf.cglib.reflect -- net.sf.cglib.transform -- net.sf.cglib.transform.impl -- net.sf.cglib.util +- com.mongodb.* diff --git a/morphia/src/main/java/org/mongodb/morphia/internal/MorphiaUtils.java b/morphia/src/main/java/org/mongodb/morphia/internal/MorphiaUtils.java new file mode 100644 index 00000000000..f110a391e68 --- /dev/null +++ b/morphia/src/main/java/org/mongodb/morphia/internal/MorphiaUtils.java @@ -0,0 +1,48 @@ +/* + * Copyright 2016 MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.mongodb.morphia.internal; + +import java.util.List; + +/** + * This class provides a set of utilities for use in Morphia. All these methods should be considered internal only and should not be + * used in application code. + * + * @since 1.3 + */ +public final class MorphiaUtils { + private MorphiaUtils() { + } + + /** + * Joins strings with the given delimiter + * + * @param strings the strings to join + * @param delimiter the delimiter + * @return the joined string + */ + public static String join(final List strings, final char delimiter) { + StringBuilder builder = new StringBuilder(); + for (String element : strings) { + if (builder.length() != 0) { + builder.append(delimiter); + } + builder.append(element); + } + return builder.toString(); + } +} diff --git a/morphia/src/main/java/org/mongodb/morphia/internal/PathTarget.java b/morphia/src/main/java/org/mongodb/morphia/internal/PathTarget.java new file mode 100644 index 00000000000..ec08c025c81 --- /dev/null +++ b/morphia/src/main/java/org/mongodb/morphia/internal/PathTarget.java @@ -0,0 +1,155 @@ +/* + * Copyright 2016 MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.mongodb.morphia.internal; + +import org.mongodb.morphia.mapping.MappedClass; +import org.mongodb.morphia.mapping.MappedField; +import org.mongodb.morphia.mapping.Mapper; +import org.mongodb.morphia.query.ValidationException; + +import java.util.Iterator; +import java.util.List; + +import static java.lang.String.format; +import static java.util.Arrays.asList; +import static org.mongodb.morphia.internal.MorphiaUtils.join; + +/** + * This is an internal class and is subject to change or removal. + * + * @since 1.3 + */ +public class PathTarget { + private final List segments; + private boolean validateNames = true; + private int position; + private Mapper mapper; + private MappedClass context; + private MappedClass root; + private MappedField target; + private boolean resolved = false; + + /** + * Creates a resolution context for the given root and path. + * + * @param mapper mapper + * @param root root + * @param path path + */ + public PathTarget(final Mapper mapper, final MappedClass root, final String path) { + this.root = root; + segments = asList(path.split("\\.")); + this.mapper = mapper; + } + + /** + * Disables validation of path segments. + */ + public void disableValidation() { + resolved = false; + validateNames = false; + } + + private boolean hasNext() { + return position < segments.size(); + } + + /** + * Returns the translated path for this context. If validation is disabled, that path could be the same as the initial value. + * + * @return the translated path + */ + public String translatedPath() { + if (!resolved) { + resolve(); + } + return join(segments, '.'); + } + + /** + * Returns the MappedField found at the end of a path. May be null if the path is invalid and validation is disabled. + * + * @return the field + */ + public MappedField getTarget() { + if (!resolved) { + resolve(); + } + return target; + } + + String next() { + return segments.get(position++); + } + + private void resolve() { + context = this.root; + position = 0; + MappedField field = null; + while (hasNext()) { + String segment = next(); + + if (segment.equals("$") || segment.matches("[0-9]+")) { // array operator + segment = next(); + } + field = resolveField(segment); + + if (field != null) { + if (!field.isMap()) { + translate(field.getNameToStore()); + } else { + next(); // consume the map key segment + } + } else { + if (validateNames) { + throw new ValidationException(format("Could not resolve path '%s' against '%s'.", join(segments, '.'), + root.getClazz().getName())); + } + } + } + target = field; + resolved = true; + } + + private void translate(final String nameToStore) { + segments.set(position - 1, nameToStore); + } + + private MappedField resolveField(final String segment) { + MappedField mf = context.getMappedField(segment); + if (mf == null) { + mf = context.getMappedFieldByJavaField(segment); + } + if (mf == null) { + Iterator subTypes = mapper.getSubTypes(context).iterator(); + while (mf == null && subTypes.hasNext()) { + context = subTypes.next(); + mf = resolveField(segment); + } + } + + if (mf != null) { + context = mapper.getMappedClass(mf.getSubClass() != null ? mf.getSubClass() : mf.getConcreteType()); + } + return mf; + } + + @Override + public String toString() { + return String.format("PathTarget{root=%s, segments=%s, target=%s}", root.getClazz().getSimpleName(), segments, target); + } +} diff --git a/morphia/src/main/java/org/mongodb/morphia/internal/package-info.java b/morphia/src/main/java/org/mongodb/morphia/internal/package-info.java new file mode 100644 index 00000000000..8b2b492a73b --- /dev/null +++ b/morphia/src/main/java/org/mongodb/morphia/internal/package-info.java @@ -0,0 +1,20 @@ +/* + * Copyright (c) 2016 MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Defines internal classes for use in Morphia. Classes in this package should not be used in application code. + */ +package org.mongodb.morphia.internal; diff --git a/morphia/src/main/java/org/mongodb/morphia/query/UpdateOpsImpl.java b/morphia/src/main/java/org/mongodb/morphia/query/UpdateOpsImpl.java index 05f09a2ed7d..538903e5edc 100644 --- a/morphia/src/main/java/org/mongodb/morphia/query/UpdateOpsImpl.java +++ b/morphia/src/main/java/org/mongodb/morphia/query/UpdateOpsImpl.java @@ -3,6 +3,7 @@ import com.mongodb.BasicDBObject; import com.mongodb.DBObject; +import org.mongodb.morphia.internal.PathTarget; import org.mongodb.morphia.mapping.MappedField; import org.mongodb.morphia.mapping.Mapper; @@ -13,7 +14,6 @@ import java.util.Map; import static java.util.Collections.singletonList; -import static org.mongodb.morphia.query.QueryValidator.validateQuery; /** @@ -25,7 +25,6 @@ public class UpdateOpsImpl implements UpdateOperations { private final Class clazz; private Map> ops = new HashMap>(); private boolean validateNames = true; - private boolean validateTypes = true; private boolean isolated; /** @@ -113,12 +112,14 @@ public UpdateOperations push(final String field, final List values, final throw new QueryException("Values cannot be null or empty."); } - StringBuilder fieldName = new StringBuilder(field); - MappedField mf = validate(values, fieldName); + PathTarget pathTarget = new PathTarget(mapper, mapper.getMappedClass(clazz), field); + if (!validateNames) { + pathTarget.disableValidation(); + } - BasicDBObject dbObject = new BasicDBObject(UpdateOperator.EACH.val(), mapper.toMongoObject(mf, null, values)); + BasicDBObject dbObject = new BasicDBObject(UpdateOperator.EACH.val(), mapper.toMongoObject(pathTarget.getTarget(), null, values)); options.update(dbObject); - addOperation(UpdateOperator.PUSH, fieldName, dbObject); + addOperation(UpdateOperator.PUSH, pathTarget.translatedPath(), dbObject); return this; } @@ -143,14 +144,12 @@ public UpdateOperations dec(final String field, final Number value) { @Override public UpdateOperations disableValidation() { validateNames = false; - validateTypes = false; return this; } @Override public UpdateOperations enableValidation() { validateNames = true; - validateTypes = true; return this; } @@ -261,6 +260,7 @@ public void setOps(final DBObject ops) { /** * @return true if isolated */ + @Override public boolean isIsolated() { return isolated; } @@ -272,8 +272,11 @@ protected void add(final UpdateOperator op, final String f, final Object value, } Object val = value; - StringBuilder fieldName = new StringBuilder(f); - MappedField mf = validate(val, fieldName); + PathTarget pathTarget = new PathTarget(mapper, mapper.getMappedClass(clazz), f); + if (!validateNames) { + pathTarget.disableValidation(); + } + MappedField mf = pathTarget.getTarget(); if (convert) { if (UpdateOperator.PULL_ALL.equals(op) && value instanceof List) { @@ -288,22 +291,16 @@ protected void add(final UpdateOperator op, final String f, final Object value, val = new BasicDBObject(UpdateOperator.EACH.val(), val); } - addOperation(op, fieldName, val); + addOperation(op, pathTarget.translatedPath(), val); } - private void addOperation(final UpdateOperator op, final StringBuilder fieldName, final Object val) { + private void addOperation(final UpdateOperator op, final String fieldName, final Object val) { final String opString = op.val(); if (!ops.containsKey(opString)) { ops.put(opString, new LinkedHashMap()); } - ops.get(opString).put(fieldName.toString(), val); - } - - private MappedField validate(final Object val, final StringBuilder fieldName) { - return validateNames || validateTypes - ? validateQuery(clazz, mapper, fieldName, FilterOperator.EQUAL, val, validateNames, validateTypes) - : null; + ops.get(opString).put(fieldName, val); } protected UpdateOperations remove(final String fieldExpr, final boolean firstNotLast) { @@ -319,4 +316,5 @@ protected List toDBObjList(final MappedField mf, final List values) { return list; } + } diff --git a/morphia/src/test/java/org/mongodb/morphia/IndexHelperTest.java b/morphia/src/test/java/org/mongodb/morphia/IndexHelperTest.java index 137a3917139..a4bda325928 100644 --- a/morphia/src/test/java/org/mongodb/morphia/IndexHelperTest.java +++ b/morphia/src/test/java/org/mongodb/morphia/IndexHelperTest.java @@ -80,7 +80,7 @@ public void calculateBadKeys() { .type(IndexType.DESC)); try { indexHelper.calculateKeys(mappedClass, index); - fail("Validation should have errored on the bad key"); + fail("Validation should have failed on the bad key"); } catch (MappingException e) { // all good } diff --git a/morphia/src/test/java/org/mongodb/morphia/TestUpdateOps.java b/morphia/src/test/java/org/mongodb/morphia/TestUpdateOps.java index 4e07e99e9f4..a448242f169 100644 --- a/morphia/src/test/java/org/mongodb/morphia/TestUpdateOps.java +++ b/morphia/src/test/java/org/mongodb/morphia/TestUpdateOps.java @@ -30,6 +30,7 @@ import org.mongodb.morphia.query.TestQuery.ContainsPic; import org.mongodb.morphia.query.TestQuery.Pic; import org.mongodb.morphia.query.UpdateOperations; +import org.mongodb.morphia.query.UpdateOpsImpl; import org.mongodb.morphia.query.UpdateResults; import org.mongodb.morphia.query.ValidationException; import org.mongodb.morphia.testmodel.Circle; @@ -39,6 +40,7 @@ import java.util.Date; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; @@ -90,6 +92,29 @@ public void shouldUpdateAnArrayElement() { assertThat(getDs().find(Parent.class, "id", parentId).get().children, hasItem(new Child(childName, updatedLastName))); } + @Test + public void testDisableValidation() { + Child child1 = new Child("James", "Rigney"); + + validateClassName("children", getDs().createUpdateOperations(Parent.class) + .removeAll("children", child1), false); + + validateClassName("children", getDs().createUpdateOperations(Parent.class) + .disableValidation() + .removeAll("children", child1), false); + + validateClassName("c", getDs().createUpdateOperations(Parent.class) + .disableValidation() + .removeAll("c", child1), true); + } + + private void validateClassName(final String path, final UpdateOperations ops, final boolean expected) { + DBObject ops1 = ((UpdateOpsImpl) ops).getOps(); + Map pull = (Map) ops1.get("$pull"); + Map children = (Map) pull.get(path); + Assert.assertEquals(expected, children.containsKey("className")); + } + @Test @SuppressWarnings("deprecation") public void testAdd() throws Exception { diff --git a/morphia/src/test/java/org/mongodb/morphia/entities/EmbeddedType.java b/morphia/src/test/java/org/mongodb/morphia/entities/EmbeddedType.java index 72aaf8ee019..65c2e6786f7 100644 --- a/morphia/src/test/java/org/mongodb/morphia/entities/EmbeddedType.java +++ b/morphia/src/test/java/org/mongodb/morphia/entities/EmbeddedType.java @@ -16,6 +16,9 @@ package org.mongodb.morphia.entities; +import org.mongodb.morphia.annotations.Embedded; + +@Embedded public class EmbeddedType { private Long number; private String text; diff --git a/morphia/src/test/java/org/mongodb/morphia/entities/ParentType.java b/morphia/src/test/java/org/mongodb/morphia/entities/ParentType.java index 005ae68484d..cd72da8801d 100644 --- a/morphia/src/test/java/org/mongodb/morphia/entities/ParentType.java +++ b/morphia/src/test/java/org/mongodb/morphia/entities/ParentType.java @@ -19,12 +19,15 @@ import org.bson.types.ObjectId; import org.mongodb.morphia.annotations.Entity; import org.mongodb.morphia.annotations.Id; +import org.mongodb.morphia.annotations.Property; @Entity public class ParentType { @Id private ObjectId id; private EmbeddedType embedded; + @Property("n") + private String name; public EmbeddedType getEmbedded() { return embedded; diff --git a/morphia/src/test/java/org/mongodb/morphia/internal/PathTargetTest.java b/morphia/src/test/java/org/mongodb/morphia/internal/PathTargetTest.java new file mode 100644 index 00000000000..6a26c22cb0d --- /dev/null +++ b/morphia/src/test/java/org/mongodb/morphia/internal/PathTargetTest.java @@ -0,0 +1,120 @@ +/* + * Copyright 2016 MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.mongodb.morphia.internal; + +import org.junit.Assert; +import org.junit.Test; +import org.mongodb.morphia.TestArrayUpdates.Grade; +import org.mongodb.morphia.TestArrayUpdates.Student; +import org.mongodb.morphia.TestBase; +import org.mongodb.morphia.entities.EmbeddedSubtype; +import org.mongodb.morphia.entities.EmbeddedType; +import org.mongodb.morphia.entities.EntityWithListsAndArrays; +import org.mongodb.morphia.entities.ParentType; +import org.mongodb.morphia.mapping.EmbeddedMappingTest.AnotherNested; +import org.mongodb.morphia.mapping.EmbeddedMappingTest.Nested; +import org.mongodb.morphia.mapping.EmbeddedMappingTest.NestedImpl; +import org.mongodb.morphia.mapping.EmbeddedMappingTest.WithNested; +import org.mongodb.morphia.mapping.MappedClass; +import org.mongodb.morphia.mapping.Mapper; + +public class PathTargetTest extends TestBase { + + @Test + public void simpleResolution() { + getMorphia().map(ParentType.class, EmbeddedType.class); + Mapper mapper = getMorphia().getMapper(); + MappedClass mappedClass = mapper.getMappedClass(ParentType.class); + + PathTarget pathTarget = new PathTarget(mapper, mappedClass, "name"); + Assert.assertEquals("n", pathTarget.translatedPath()); + Assert.assertEquals(mappedClass.getMappedFieldByJavaField("name"), pathTarget.getTarget()); + + pathTarget = new PathTarget(mapper, mappedClass, "n"); + Assert.assertEquals("n", pathTarget.translatedPath()); + Assert.assertEquals(mappedClass.getMappedField("n"), pathTarget.getTarget()); + } + + @Test + public void dottedPath() { + getMorphia().map(ParentType.class, EmbeddedType.class); + Mapper mapper = getMorphia().getMapper(); + + PathTarget pathTarget = new PathTarget(mapper, mapper.getMappedClass(ParentType.class), "embedded.number"); + Assert.assertEquals("embedded.number", pathTarget.translatedPath()); + Assert.assertEquals(mapper.getMappedClass(EmbeddedType.class).getMappedFieldByJavaField("number"), pathTarget.getTarget()); + } + + @Test + public void subClasses() { + getMorphia().map(ParentType.class, EmbeddedType.class, EmbeddedSubtype.class); + Mapper mapper = getMorphia().getMapper(); + + PathTarget pathTarget = new PathTarget(mapper, mapper.getMappedClass(ParentType.class), "embedded.flag"); + Assert.assertEquals("embedded.flag", pathTarget.translatedPath()); + Assert.assertEquals(mapper.getMappedClass(EmbeddedSubtype.class).getMappedFieldByJavaField("flag"), pathTarget.getTarget()); + } + + @Test + public void arrays() { + getMorphia().map(EntityWithListsAndArrays.class, EmbeddedType.class, Student.class); + Mapper mapper = getMorphia().getMapper(); + MappedClass mappedClass = mapper.getMappedClass(EntityWithListsAndArrays.class); + + PathTarget pathTarget = new PathTarget(mapper, mappedClass, "listEmbeddedType.1.number"); + Assert.assertEquals("listEmbeddedType.1.number", pathTarget.translatedPath()); + Assert.assertEquals(mapper.getMappedClass(EmbeddedType.class).getMappedFieldByJavaField("number"), pathTarget.getTarget()); + } + + @Test + public void maps() { + getMorphia().map(Student.class); + Mapper mapper = getMorphia().getMapper(); + MappedClass mappedClass = mapper.getMappedClass(Student.class); + + final PathTarget pathTarget = new PathTarget(mapper, mappedClass, "grades.$.data.name"); + Assert.assertEquals("grades.$.data.name", pathTarget.translatedPath()); + Assert.assertEquals(mapper.getMappedClass(Grade.class).getMappedFieldByJavaField("data"), pathTarget.getTarget()); + } + + @Test + public void interfaces() { + getMorphia().map(WithNested.class, Nested.class, NestedImpl.class, AnotherNested.class); + Mapper mapper = getMorphia().getMapper(); + MappedClass mappedClass = mapper.getMappedClass(WithNested.class); + + PathTarget pathTarget = new PathTarget(mapper, mappedClass, "nested.value"); + Assert.assertEquals("nested.value", pathTarget.translatedPath()); + Assert.assertEquals(mapper.getMappedClass(AnotherNested.class).getMappedFieldByJavaField("value"), pathTarget.getTarget()); + + pathTarget = new PathTarget(mapper, mappedClass, "nested.field"); + Assert.assertEquals("nested.field", pathTarget.translatedPath()); + Assert.assertEquals(mapper.getMappedClass(NestedImpl.class).getMappedFieldByJavaField("field"), pathTarget.getTarget()); + } + + @Test + public void disableValidation() { + getMorphia().map(WithNested.class, Nested.class, NestedImpl.class, AnotherNested.class); + Mapper mapper = getMorphia().getMapper(); + MappedClass mappedClass = mapper.getMappedClass(WithNested.class); + + final PathTarget pathTarget = new PathTarget(mapper, mappedClass, "nested.field.fail"); + pathTarget.disableValidation(); + Assert.assertEquals("nested.field.fail", pathTarget.translatedPath()); + Assert.assertNull(pathTarget.getTarget()); + } +}