Permalink
Browse files

Added required vs optional validation for partial updates.

Changed validation path formats to match PathSpec.

RB=521169
BUG=
G=si-dev
R=mtagle,erli
A=mtagle,erli
  • Loading branch information...
1 parent 715e203 commit d9b67aaaa574d12b627ded5d80732824b7a32daf @soojungha soojungha committed Jul 10, 2015
Showing with 357 additions and 412 deletions.
  1. +3 −1 CHANGELOG
  2. +10 −6 data-transform/src/main/java/com/linkedin/data/transform/patch/Patch.java
  3. +1 −0 data/build.gradle
  4. +1 −24 data/src/main/java/com/linkedin/data/element/DataElementUtil.java
  5. +64 −0 data/src/main/java/com/linkedin/data/schema/DataSchemaUtil.java
  6. +1 −5 data/src/main/java/com/linkedin/data/schema/validation/ValidateDataAgainstSchema.java
  7. +14 −11 data/src/main/java/com/linkedin/data/schema/validation/ValidationOptions.java
  8. +13 −14 ...ion/TestValidationUtil.java → data/src/test/java/com/linkedin/data/schema/TestDataSchemaUtil.java
  9. +1 −1 ...common/validation → data/src/test/resources/pegasus/com/linkedin/data/schema}/ValidationDemo.pdsc
  10. +0 −1 restli-common/build.gradle
  11. +199 −141 restli-common/src/main/java/com/linkedin/restli/common/validation/RestLiDataValidator.java
  12. +0 −109 restli-common/src/main/java/com/linkedin/restli/common/validation/ValidationUtil.java
  13. +2 −2 ...-api/src/main/idl/com.linkedin.restli.examples.greetings.client.autoValidationDemos.restspec.json
  14. +2 −2 ...test-api/src/main/idl/com.linkedin.restli.examples.greetings.client.validationDemos.restspec.json
  15. +2 −2 ...src/main/snapshot/com.linkedin.restli.examples.greetings.client.autoValidationDemos.snapshot.json
  16. +2 −2 ...api/src/main/snapshot/com.linkedin.restli.examples.greetings.client.validationDemos.snapshot.json
  17. +2 −2 .../src/main/java/com/linkedin/restli/examples/greetings/server/AutomaticValidationDemoResource.java
  18. +2 −2 ...st-server/src/main/java/com/linkedin/restli/examples/greetings/server/ValidationDemoResource.java
  19. +27 −23 restli-int-test/src/test/java/com/linkedin/restli/examples/TestRestLiValidation.java
  20. +1 −2 restli-server/src/main/java/com/linkedin/restli/internal/server/model/RestLiAnnotationReader.java
  21. +3 −3 restli-server/src/test/java/com/linkedin/restli/server/invalid/InvalidResources.java
  22. +3 −3 restli-server/src/test/java/com/linkedin/restli/server/test/TestRestLiResourceModels.java
  23. +4 −56 restli-tools/src/main/java/com/linkedin/restli/tools/compatibility/ResourceCompatibilityChecker.java
View
@@ -1,6 +1,8 @@
2.10.6
-----
-
+(RB=521169)
+Added required vs optional validation for partial updates.
+Changed validation path formats to match PathSpec.
2.10.5
-----
@@ -55,17 +55,17 @@
private IdentityHashMap<DataMap, Boolean> _hasDeletesOnly =
new IdentityHashMap<DataMap, Boolean>();
- // On $delete operations, log the path as an info message in the interpreter context
- private final boolean _logDeletes;
+ // On $set and $delete operations, log the path as an info message in the interpreter context
+ private final boolean _logOperations;
public Patch()
{
this(false);
}
- public Patch(boolean logDeletes)
+ public Patch(boolean logOperations)
{
- _logDeletes = logDeletes;
+ _logOperations = logOperations;
}
/**
@@ -135,7 +135,7 @@ else if (opChild.getClass() == DataMap.class)
// and if patch's branch contains only deletes operations, then it is
// not necessary to create nodes on the data object and process that branch,
// unless we have to log all delete operations
- if (!hasDeletesOnly(opChildDataMap) || _logDeletes)
+ if (!hasDeletesOnly(opChildDataMap) || _logOperations)
{
// if patch does data manipulations other than deletes, then we need to
@@ -198,7 +198,7 @@ private boolean executeDeleteCommand(Object deleteCommand, Object data, Map<Stri
{
usedFields.put(key.toString(), DELETE_COMMAND);
dataDataMap.remove(key);
- if (_logDeletes)
+ if (_logOperations)
{
instrCtx.addInfoMessage(key.toString());
}
@@ -238,6 +238,10 @@ private boolean executeSetCommand(Object setCommand, Object data, Map<String, St
{
usedFields.put(key.toString(), SET_COMMAND);
dataDataMap.put(key, entry.getValue());
+ if (_logOperations)
+ {
+ instrCtx.addInfoMessage(key);
+ }
}
}
}
View
@@ -1,5 +1,6 @@
dependencies {
compile externalDependency.jacksonCore
+ testCompile externalDependency.commonsIo
testCompile externalDependency.testng
}
@@ -195,6 +195,7 @@ else if (currentElement.getValue().getClass() == DataList.class && component.get
DataSchema schema = currentElement.getSchema();
if (schema != null)
{
+ schema = schema.getDereferencedDataSchema();
switch (schema.getType())
{
case ARRAY:
@@ -254,28 +255,4 @@ else if (currentElement.getValue().getClass() == DataList.class && component.get
}
return list;
}
-
- /**
- * Similar to {@link DataElement#pathAsString}, but without map keys or array indices.
- *
- * @param dataElement data element
- * @return path without keys as a string
- */
- public static String pathWithoutKeysAsString(DataElement dataElement)
- {
- DataElement element = dataElement;
- StringBuilder builder = new StringBuilder();
- while (element.getParent() != null)
- {
- DataSchema.Type parentType = element.getParent().getSchema().getType();
- if (parentType == DataSchema.Type.ARRAY || parentType == DataSchema.Type.MAP)
- {
- element = element.getParent();
- continue;
- }
- builder = new StringBuilder().append(DataElement.SEPARATOR).append(element.getName()).append(builder);
- element = element.getParent();
- }
- return builder.toString();
- }
}
@@ -17,6 +17,8 @@
package com.linkedin.data.schema;
import com.linkedin.data.ByteString;
+import com.linkedin.data.element.DataElement;
+
import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.Map;
@@ -51,6 +53,68 @@ public static PrimitiveDataSchema dataSchemaTypeToPrimitiveDataSchema(DataSchema
return _TYPE_STRING_TO_COMPLEX_DATA_SCHEMA_TYPE_MAP.get(type);
}
+ /**
+ * Determines whether a path is valid according to the given data schema.
+ * The path must be a field of a record, and not an enum constant, a member of a union, etc.
+ * Wild card (*) for array indices and map keys are accepted.
+ */
+ public static boolean containsPath(DataSchema schema, String path)
+ {
+ return getField(schema, path) != null;
+ }
+
+ /**
+ * Same as {@link #getField(DataSchema, Object[])}, but with the path as string.
+ */
+ public static RecordDataSchema.Field getField(DataSchema schema, String path)
+ {
+ // Discard the initial / character if present
+ if (path.length() > 0 && path.charAt(0) == DataElement.SEPARATOR)
+ {
+ path = path.substring(1);
+ }
+ return getField(schema, path.split(DataElement.SEPARATOR.toString()));
+ }
+
+ /**
+ * Finds the {@link RecordDataSchema.Field} corresponding to the specified path by
+ * following the path starting from the provided {@link DataSchema} as the root.
+ *
+ * @return field object, or null if the field is not found
+ */
+ public static RecordDataSchema.Field getField(DataSchema schema, Object[] path)
+ {
+ RecordDataSchema.Field field = null;
+ DataSchema dataSchema = schema;
+ for (int i = 0; i < path.length; i++)
+ {
+ dataSchema = dataSchema.getDereferencedDataSchema();
+ switch (dataSchema.getType())
+ {
+ case MAP:
+ dataSchema = ((MapDataSchema) dataSchema).getValues();
+ break;
+ case ARRAY:
+ dataSchema = ((ArrayDataSchema) dataSchema).getItems();
+ break;
+ case RECORD:
+ field = ((RecordDataSchema) dataSchema).getField(path[i].toString());
+ if (i == path.length - 1) return field;
+ if (field == null) return null;
+ dataSchema = field.getType();
+ break;
+ case UNION:
+ dataSchema = ((UnionDataSchema) dataSchema).getType(path[i].toString());
+ if (dataSchema == null) return null;
+ break;
+ default:
+ // Parent schema cannot be a primitive
+ return null;
+ }
+ }
+ return null;
+ }
+
private DataSchemaUtil() {}
static final Map<String, PrimitiveDataSchema> _TYPE_STRING_TO_PRIMITIVE_DATA_SCHEMA_MAP;
@@ -294,11 +294,7 @@ private boolean isFieldOptional(RecordDataSchema.Field field, DataElement elemen
{
return true;
}
- StringBuilder sb = new StringBuilder();
- sb.append(DataElementUtil.pathWithoutKeysAsString(element));
- sb.append(DataElement.SEPARATOR);
- sb.append(field.getName());
- return _options.getOptionalFields().contains(sb.substring(1));
+ return _options.getTreatOptional().evaluate(new SimpleDataElement(null, field.getName(), field.getType(), element));
}
protected Object validateRecord(DataElement element, RecordDataSchema schema, Object object)
@@ -17,12 +17,13 @@
package com.linkedin.data.schema.validation;
+import com.linkedin.data.it.Predicate;
+import com.linkedin.data.it.Predicates;
import com.linkedin.util.ArgumentUtil;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
-import java.util.Set;
/**
@@ -171,24 +172,25 @@ public Object getValidatorParameter(String key)
}
/**
- * Set optional fields.
- * The fields corresponding to the paths will be treated as optional, even if they are required.
+ * Option to treat certain required fields as optional.
+ * A required field whose corresponding data element satisfies the given {@link Predicate}
+ * will be treated as optional.
*
- * @param optionalFields
+ * @param treatOptional
*/
- public void setOptionalFields(Set<String> optionalFields)
+ public void setTreatOptional(Predicate treatOptional)
{
- _optionalFields = Collections.unmodifiableSet(optionalFields);
+ _treatOptional = treatOptional;
}
/**
- * Return which paths should be treated as optional.
+ * Return the predicate for treating certain fields as optional.
*
- * @return paths for optional fields
+ * @return predicate
*/
- public Set<String> getOptionalFields()
+ public Predicate getTreatOptional()
{
- return _optionalFields;
+ return _treatOptional;
}
/**
@@ -251,7 +253,8 @@ public String toString()
private RequiredMode _requiredMode;
private boolean _avroUnionMode = false;
private Map<String,Object> _validatorParameters = NO_VALIDATOR_PARAMETERS;
- private Set<String> _optionalFields = Collections.emptySet();
+ // Treat required fields as optional if the corresponding data element satisfies this predicate
+ private Predicate _treatOptional = Predicates.alwaysFalse();
private static final Map<String,Object> NO_VALIDATOR_PARAMETERS = Collections.emptyMap();
}
@@ -14,10 +14,9 @@
limitations under the License.
*/
-package com.linkedin.restli.common.validation;
+package com.linkedin.data.schema;
-import com.linkedin.data.schema.DataSchema;
import com.linkedin.data.template.DataTemplateUtil;
import org.apache.commons.io.IOUtils;
import org.testng.Assert;
@@ -28,11 +27,11 @@
import java.io.InputStream;
/**
- * Unit tests for ValidationUtil.
+ * Unit tests for DataSchemaUtil.
*
* @author Soojung Ha
*/
-public class TestValidationUtil
+public class TestDataSchemaUtil
{
@DataProvider
public Object[][] pathData()
@@ -43,31 +42,31 @@
{"stringB", true},
{"intB", true},
{"UnionFieldWithInlineRecord", true},
- {"UnionFieldWithInlineRecord/com.linkedin.restli.common.validation.myRecord/foo1", true},
+ {"UnionFieldWithInlineRecord/com.linkedin.data.schema.myRecord/foo1", true},
{"ArrayWithInlineRecord", true},
- {"ArrayWithInlineRecord/bar1", true},
- {"ArrayWithInlineRecord/bar2", true},
+ {"ArrayWithInlineRecord/*/bar1", true},
+ {"ArrayWithInlineRecord/*/bar2", true},
{"MapWithTyperefs", true},
- {"MapWithTyperefs/id", true},
+ {"MapWithTyperefs/*/id", true},
{"validationDemoNext/stringB", true},
{"validationDemoNext/UnionFieldWithInlineRecord", true},
{"validationDemoNext/validationDemoNext/validationDemoNext", true},
// nonexistent field
{"stringA1", false},
{"stringA/abc", false},
- {"ArrayWithInlineRecord/bar3", false},
+ {"ArrayWithInlineRecord/*/bar3", false},
// valid path but not a field of a record
- {"UnionFieldWithInlineRecord/com.linkedin.restli.common.validation.myRecord", false},
- {"UnionFieldWithInlineRecord/com.linkedin.restli.common.validation.myEnum", false},
- {"UnionFieldWithInlineRecord/com.linkedin.restli.common.validation.myEnum/FOOFOO", false}
+ {"UnionFieldWithInlineRecord/com.linkedin.data.schema.myRecord", false},
+ {"UnionFieldWithInlineRecord/com.linkedin.data.schema.myEnum", false},
+ {"UnionFieldWithInlineRecord/com.linkedin.data.schema.myEnum/FOOFOO", false}
};
}
@Test(dataProvider = "pathData")
public void testContainsPath(String path, boolean expected) throws IOException
{
DataSchema validationDemoSchema = pdscToDataSchema(DATA_SCHEMA_PATH);
- Assert.assertEquals(ValidationUtil.containsPath(validationDemoSchema, path), expected);
+ Assert.assertEquals(DataSchemaUtil.containsPath(validationDemoSchema, path), expected);
}
private DataSchema pdscToDataSchema(String path) throws IOException
@@ -76,5 +75,5 @@ private DataSchema pdscToDataSchema(String path) throws IOException
return DataTemplateUtil.parseSchema(IOUtils.toString(pdscStream));
}
- private static final String DATA_SCHEMA_PATH = "pegasus/com/linkedin/restli/common/validation/ValidationDemo.pdsc";
+ private static final String DATA_SCHEMA_PATH = "pegasus/com/linkedin/data/schema/ValidationDemo.pdsc";
}
@@ -1,7 +1,7 @@
{
"type" : "record",
"name" : "ValidationDemo",
- "namespace" : "com.linkedin.restli.common.validation",
+ "namespace" : "com.linkedin.data.schema",
"fields" : [
{
"name": "stringA",
@@ -8,7 +8,6 @@ dependencies {
testCompile project(path: ':data', configuration: 'testArtifacts')
testCompile project(path: ':generator-test', configuration: 'testArtifacts')
testCompile project(':li-jersey-uri')
- testCompile externalDependency.commonsIo
testCompile externalDependency.testng
}
Oops, something went wrong.

0 comments on commit d9b67aa

Please sign in to comment.