Reland "Reland: "Add validator for schema org entities and types in doc metadata mojo.""

This is a reland of d828e2eb

* I added the BUILD dependency to the wrong target in the
  previous reland. It needed to be on generate_schema_org_code.
  https://crrev.com/c/2097359

Original change's description:
> Reland: "Add validator for schema org entities and types in doc metadata mojo."
>
> "
> * Add a validator that removes properties that are not the right type.
> * Add double, time, and timedelta types in the document metadata mojo
>   structure.
> * Use the new mojo types in document metadata extractor.
> "
>
> This is a reland of https://crrev.com/c/2062907 which was reverted in
> https://crrev.com/c/2095670.
>
> The substantial changes are two-fold:
> (1) add build dep on base in schema_org_properties. This should fix the
>     try bot issue which was somehow skipped.
> (2) Rebase on top of https://crrev.com/c/2073337 which was submitted and
>     contains some changes to the mojo structure.
>
> Change-Id: I7f47d74e8833f76d4077028eac0dcb43790a1d26
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2097059
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Becca Hughes <beccahughes@chromium.org>
> Commit-Queue: Sam Bowen <sgbowen@google.com>
> Cr-Commit-Position: refs/heads/master@{#748945}

Change-Id: Ibc7124d5a779d663048315a2697c9c050c1c00c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2097452Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Sam Bowen <sgbowen@google.com>
Cr-Commit-Position: refs/heads/master@{#749248}

components/schema_org/BUILD.gn

@@ -7,11 +7,13 @@ source_set("unit_tests") {
   sources = [
     "extractor_unittest.cc",
     "generate_schema_org_code_unittest.cc",
+    "validator_unittest.cc",
   ]
 
   deps = [
     ":extractor",
     ":generate_schema_org_code",
+    ":schema_org",
     ":schema_org_properties",
     "//base",
     "//components/schema_org/common:improved_mojom",
@@ -58,7 +60,10 @@ static_library("schema_org_properties") {
     "$target_gen_dir/schema_org_property_names.h",
   ]
 
-  deps = [ ":generate_schema_org_code" ]
+  deps = [
+    ":generate_schema_org_code",
+    "//base",
+  ]
 }
 
 static_library("extractor") {
@@ -68,6 +73,21 @@ static_library("extractor") {
   ]
 
   deps = [
+    "//components/schema_org:generate_schema_org_code",
+    "//components/schema_org:schema_org",
+    "//components/schema_org:schema_org_properties",
+    "//components/schema_org/common:improved_mojom",
+  ]
+}
+
+static_library("schema_org") {
+  sources = [
+    "validator.cc",
+    "validator.h",
+  ]
+
+  deps = [
+    "//base",
     "//components/schema_org:generate_schema_org_code",
     "//components/schema_org:schema_org_properties",
     "//components/schema_org/common:improved_mojom",

components/schema_org/common/BUILD.gn

@@ -15,5 +15,8 @@ mojom("improved_mojom") {
   generate_java = true
   sources = [ "improved_metadata.mojom" ]
 
-  public_deps = [ "//url/mojom:url_mojom_gurl" ]
+  public_deps = [
+    "//mojo/public/mojom/base",
+    "//url/mojom:url_mojom_gurl",
+  ]
 }

components/schema_org/common/improved_metadata.mojom

@@ -10,12 +10,17 @@
 
 module schema_org.improved.mojom;
 
+import "mojo/public/mojom/base/time.mojom";
+
 // A property can have arrays of different types simultaneously. Non-array
 // values are converted to arrays of one element.
 struct Values {
   array<bool> bool_values;
   array<int64> long_values;
   array<string> string_values;
+  array<double> double_values;
+  array<mojo_base.mojom.Time> date_time_values;
+  array<mojo_base.mojom.TimeDelta> time_values;
   array<Entity> entity_values;
 };
 

components/schema_org/extractor.cc

@@ -15,6 +15,8 @@
 #include "base/values.h"
 #include "components/schema_org/common/improved_metadata.mojom.h"
 #include "components/schema_org/schema_org_entity_names.h"
+#include "components/schema_org/schema_org_property_configurations.h"
+#include "components/schema_org/validator.h"
 
 namespace schema_org {
 
@@ -54,7 +56,56 @@ bool IsSupportedType(const std::string& type) {
 
 void ExtractEntity(const base::DictionaryValue&, Entity*, int recursion_level);
 
+// Parses a string into a property value. The string may be parsed as a double,
+// date, or time, depending on the types that the property supports. If the
+// property supports text, uses the string itself.
+bool ParseStringValue(const std::string& property_type,
+                      base::StringPiece value,
+                      Values* values) {
+  value = value.substr(0, kMaxStringLength);
+
+  schema_org::property::PropertyConfiguration prop_config =
+      schema_org::property::GetPropertyConfiguration(property_type);
+  if (prop_config.text) {
+    values->string_values.push_back(value.as_string());
+    return true;
+  }
+  if (prop_config.number) {
+    double d;
+    bool parsed_double = base::StringToDouble(value, &d);
+    if (parsed_double) {
+      values->double_values.push_back(d);
+      return true;
+    }
+  }
+  if (prop_config.date_time || prop_config.date) {
+    base::Time time;
+    bool parsed_time = base::Time::FromString(value.data(), &time);
+    if (parsed_time) {
+      values->date_time_values.push_back(time);
+      return true;
+    }
+  }
+  if (prop_config.time) {
+    base::Time time_of_day;
+    base::Time start_of_day;
+    bool parsed_time = base::Time::FromString(
+        ("1970-01-01T" + value.as_string()).c_str(), &time_of_day);
+    bool parsed_day_start =
+        base::Time::FromString("1970-01-01T00:00:00", &start_of_day);
+    base::TimeDelta time = time_of_day - start_of_day;
+    // The string failed to parse as a DateTime, but did parse as a Time. Use
+    // this value instead.
+    if (parsed_time && parsed_day_start) {
+      values->time_values.push_back(time);
+      return true;
+    }
+  }
+  return false;
+}
+
 bool ParseRepeatedValue(const base::Value::ConstListView& arr,
+                        const std::string& property_type,
                         Values* values,
                         int recursion_level) {
   DCHECK(values);
@@ -63,34 +114,27 @@ bool ParseRepeatedValue(const base::Value::ConstListView& arr,
   }
 
   for (size_t j = 0; j < std::min(arr.size(), kMaxRepeatedSize); ++j) {
-    auto& listItem = arr[j];
+    auto& list_item = arr[j];
 
-    switch (listItem.type()) {
+    switch (list_item.type()) {
       case base::Value::Type::BOOLEAN: {
-        bool v;
-        listItem.GetAsBoolean(&v);
-        values->bool_values.push_back(v);
+        values->bool_values.push_back(list_item.GetBool());
       } break;
       case base::Value::Type::INTEGER: {
-        int v = listItem.GetInt();
-        values->long_values.push_back(v);
+        values->long_values.push_back(list_item.GetInt());
       } break;
       case base::Value::Type::DOUBLE: {
-        // App Indexing doesn't support double type, so just encode its decimal
-        // value as a string instead.
-        double v = listItem.GetDouble();
-        std::string s = base::NumberToString(v);
-        s = s.substr(0, kMaxStringLength);
-        values->string_values.push_back(s);
+        values->double_values.push_back(list_item.GetDouble());
       } break;
       case base::Value::Type::STRING: {
-        std::string v = listItem.GetString();
-        v = v.substr(0, kMaxStringLength);
-        values->string_values.push_back(v);
+        base::StringPiece v = list_item.GetString();
+        if (!ParseStringValue(property_type, v, values)) {
+          return false;
+        }
       } break;
       case base::Value::Type::DICTIONARY: {
         const base::DictionaryValue* dict_value = nullptr;
-        if (listItem.GetAsDictionary(&dict_value)) {
+        if (list_item.GetAsDictionary(&dict_value)) {
           auto entity = Entity::New();
           ExtractEntity(*dict_value, entity.get(), recursion_level + 1);
           values->entity_values.push_back(std::move(entity));
@@ -130,40 +174,48 @@ void ExtractEntity(const base::DictionaryValue& val,
     }
     property->values = Values::New();
 
-    if (entry.second.is_bool()) {
-      bool v;
-      val.GetBoolean(entry.first, &v);
-      property->values->bool_values.push_back(v);
-    } else if (entry.second.is_int()) {
-      int v;
-      val.GetInteger(entry.first, &v);
-      property->values->long_values.push_back(v);
-    } else if (entry.second.is_double()) {
-      double v;
-      val.GetDouble(entry.first, &v);
-      std::string s = base::NumberToString(v);
-      s = s.substr(0, kMaxStringLength);
-      property->values->string_values.push_back(s);
-    } else if (entry.second.is_string()) {
-      std::string v;
-      val.GetString(entry.first, &v);
-      v = v.substr(0, kMaxStringLength);
-      property->values->string_values.push_back(v);
-    } else if (entry.second.is_dict()) {
-      if (recursion_level + 1 >= kMaxDepth) {
-        continue;
+    switch (entry.second.type()) {
+      case base::Value::Type::BOOLEAN:
+        property->values->bool_values.push_back(entry.second.GetBool());
+        break;
+      case base::Value::Type::INTEGER:
+        property->values->long_values.push_back(entry.second.GetInt());
+        break;
+      case base::Value::Type::DOUBLE:
+        property->values->double_values.push_back(entry.second.GetDouble());
+        break;
+      case base::Value::Type::STRING: {
+        base::StringPiece v = entry.second.GetString();
+        if (!(ParseStringValue(property->name, v, property->values.get()))) {
+          continue;
+        }
+        break;
       }
-      const base::DictionaryValue* dict_value = nullptr;
-      if (!entry.second.GetAsDictionary(&dict_value)) {
-        continue;
+      case base::Value::Type::DICTIONARY: {
+        if (recursion_level + 1 >= kMaxDepth) {
+          continue;
+        }
+
+        const base::DictionaryValue* dict_value = nullptr;
+        if (!entry.second.GetAsDictionary(&dict_value)) {
+          continue;
+        }
+
+        auto nested_entity = Entity::New();
+        ExtractEntity(*dict_value, nested_entity.get(), recursion_level + 1);
+        property->values->entity_values.push_back(std::move(nested_entity));
+        break;
+      }
+      case base::Value::Type::LIST: {
+        const base::Value::ConstListView list_view = entry.second.GetList();
+        if (!ParseRepeatedValue(list_view, property->name,
+                                property->values.get(), recursion_level)) {
+          continue;
+        }
+        break;
       }
-      auto nested_entity = Entity::New();
-      ExtractEntity(*dict_value, nested_entity.get(), recursion_level + 1);
-      property->values->entity_values.push_back(std::move(nested_entity));
-    } else if (entry.second.is_list()) {
-      const auto& list_view = entry.second.GetList();
-      if (!ParseRepeatedValue(list_view, property->values.get(),
-                              recursion_level)) {
+      default: {
+        // Unsupported value type. Skip this property.
         continue;
       }
     }

components/schema_org/generate_schema_org_code_unittest.cc

@@ -14,6 +14,11 @@ TEST(GenerateSchemaOrgTest, EntityName) {
   EXPECT_STREQ(entity::kAboutPage, "AboutPage");
 }
 
+TEST(GenerateSchemaOrgTest, IsValidEntityName) {
+  EXPECT_TRUE(entity::IsValidEntityName(entity::kAboutPage));
+  EXPECT_FALSE(entity::IsValidEntityName("a made up name"));
+}
+
 TEST(GenerateSchemaOrgTest, PropertyName) {
   EXPECT_STREQ(property::kAcceptedAnswer, "acceptedAnswer");
 }

components/schema_org/templates/schema_org_entity_names.cc.tmpl

@@ -6,6 +6,9 @@
 // Do not edit.
 
 #include "components/schema_org/{{ header_file }}.h"
+#include "base/containers/flat_set.h"
+#include "base/no_destructor.h"
+#include "base/strings/string_piece.h"
 
 namespace schema_org {
 namespace entity {
@@ -14,5 +17,15 @@ namespace entity {
 const char k{{entity[0]|upper}}{{entity[1:]}}[] = "{{entity}}";
 {% endfor %}
 
+bool IsValidEntityName(const std::string& entity_name) {
+  static const base::NoDestructor<base::flat_set<base::StringPiece>>
+    kValidEntityNames(base::flat_set<base::StringPiece>({
+      {%for entity in entities %}
+      k{{entity[0]|upper}}{{entity[1:]}},
+      {% endfor %}
+  }));
+  return kValidEntityNames->find(entity_name) != kValidEntityNames->end();
+}
+
 }  // entity
 }  // schema_org

components/schema_org/templates/schema_org_entity_names.h.tmpl

@@ -17,6 +17,8 @@ namespace entity {
 extern const char k{{entity[0]|upper}}{{entity[1:]}}[];
 {% endfor %}
 
+bool IsValidEntityName(const std::string& entity_name);
+
 }  // namespace entity
 }  // namespace schema_org
 

components/schema_org/validator.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/schema_org/validator.h"
+
+#include <vector>
+
+#include "components/schema_org/common/improved_metadata.mojom.h"
+#include "components/schema_org/schema_org_entity_names.h"
+#include "components/schema_org/schema_org_property_configurations.h"
+#include "components/schema_org/schema_org_property_names.h"
+
+namespace schema_org {
+
+using improved::mojom::Entity;
+using improved::mojom::EntityPtr;
+
+// static
+bool ValidateEntity(Entity* entity) {
+  if (!entity::IsValidEntityName(entity->type)) {
+    return false;
+  }
+
+  // Cycle through properties and remove any that have the wrong type.
+  auto it = entity->properties.begin();
+  while (it != entity->properties.end()) {
+    property::PropertyConfiguration config =
+        property::GetPropertyConfiguration((*it)->name);
+
+    if (!(*it)->values->string_values.empty() && !config.text) {
+      it = entity->properties.erase(it);
+    } else if (!(*it)->values->double_values.empty() && !config.number) {
+      it = entity->properties.erase(it);
+    } else if (!(*it)->values->time_values.empty() && !config.time) {
+      it = entity->properties.erase(it);
+    } else if (!(*it)->values->date_time_values.empty() && !config.date_time &&
+               !config.date) {
+      it = entity->properties.erase(it);
+    } else if (!(*it)->values->entity_values.empty()) {
+      if (config.thing_types.empty()) {
+        // Property is not supposed to have an entity type.
+        it = entity->properties.erase(it);
+      } else {
+        // Check all the entities nested in this property. Remove any invalid
+        // ones.
+        bool has_valid_entities = false;
+        auto nested_it = (*it)->values->entity_values.begin();
+        while (nested_it != (*it)->values->entity_values.end()) {
+          auto& nested_entity = *nested_it;
+          if (!ValidateEntity(nested_entity.get())) {
+            nested_it = (*it)->values->entity_values.erase(nested_it);
+          } else {
+            has_valid_entities = true;
+            ++nested_it;
+          }
+        }
+
+        // If there were no valid entity values for this property, remove the
+        // whole property.
+        if (!has_valid_entities) {
+          it = entity->properties.erase(it);
+        } else {
+          ++it;
+        }
+      }
+    } else {
+      ++it;
+    }
+  }
+
+  return true;
+}
+
+}  // namespace schema_org

components/schema_org/validator.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_SCHEMA_ORG_VALIDATOR_H_
+#define COMPONENTS_SCHEMA_ORG_VALIDATOR_H_
+
+#include "components/schema_org/common/improved_metadata.mojom-forward.h"
+
+namespace schema_org {
+
+// Validates and cleans up the Schema.org entity in-place. Invalid properties
+// will be removed from the entity. Returns true if the entity was valid.
+bool ValidateEntity(improved::mojom::Entity* entity);
+
+}  // namespace schema_org
+
+#endif  // COMPONENTS_SCHEMA_ORG_VALIDATOR_H_

components/schema_org/validator_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "components/schema_org/common/improved_metadata.mojom.h"
+#include "components/schema_org/schema_org_entity_names.h"
+#include "components/schema_org/schema_org_property_configurations.h"
+#include "components/schema_org/schema_org_property_names.h"
+#include "components/schema_org/validator.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace schema_org {
+
+using improved::mojom::Entity;
+using improved::mojom::EntityPtr;
+using improved::mojom::Property;
+using improved::mojom::PropertyPtr;
+using improved::mojom::Values;
+
+class SchemaOrgValidatorTest : public testing::Test {};
+
+TEST_F(SchemaOrgValidatorTest, InvalidEntityType) {
+  EntityPtr entity = Entity::New();
+  entity->type = "random entity type";
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_FALSE(validated_entity);
+}
+
+TEST_F(SchemaOrgValidatorTest, ValidStringPropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kAboutPage;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kAccessMode;
+  property->values = Values::New();
+  property->values->string_values.push_back("foo");
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_EQ(1u, entity->properties.size());
+}
+
+TEST_F(SchemaOrgValidatorTest, InvalidStringPropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kAboutPage;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kAbout;
+  property->values = Values::New();
+  property->values->string_values.push_back("foo");
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_TRUE(entity->properties.empty());
+}
+
+TEST_F(SchemaOrgValidatorTest, ValidNumberPropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kSingleFamilyResidence;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kAdditionalNumberOfGuests;
+  property->values = Values::New();
+  property->values->double_values.push_back(1.0);
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_EQ(1u, entity->properties.size());
+}
+
+TEST_F(SchemaOrgValidatorTest, InvalidNumberPropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kAboutPage;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kAbout;
+  property->values = Values::New();
+  property->values->double_values.push_back(1.0);
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_TRUE(entity->properties.empty());
+}
+
+TEST_F(SchemaOrgValidatorTest, ValidDateTimePropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kLodgingBusiness;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kCheckinTime;
+  property->values = Values::New();
+  property->values->date_time_values.push_back(
+      base::Time::FromDeltaSinceWindowsEpoch(
+          base::TimeDelta::FromMilliseconds(12999772800000)));
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_EQ(1u, entity->properties.size());
+}
+
+TEST_F(SchemaOrgValidatorTest, InvalidDateTimePropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kAboutPage;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kAbout;
+  property->values = Values::New();
+  property->values->date_time_values.push_back(
+      base::Time::FromDeltaSinceWindowsEpoch(
+          base::TimeDelta::FromMilliseconds(12999772800000)));
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_TRUE(entity->properties.empty());
+}
+
+TEST_F(SchemaOrgValidatorTest, ValidTimePropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kLodgingBusiness;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kCheckinTime;
+  property->values = Values::New();
+  property->values->time_values.push_back(
+      base::TimeDelta::FromMilliseconds(12999772800000));
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_EQ(1u, entity->properties.size());
+}
+
+TEST_F(SchemaOrgValidatorTest, InvalidTimePropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kAboutPage;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kAbout;
+  property->values = Values::New();
+  property->values->time_values.push_back(
+      base::TimeDelta::FromMilliseconds(12999772800000));
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_TRUE(entity->properties.empty());
+}
+
+TEST_F(SchemaOrgValidatorTest, ValidEntityPropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kRestaurant;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kAddress;
+  property->values = Values::New();
+
+  EntityPtr value = Entity::New();
+  value->type = entity::kPostalAddress;
+  property->values->entity_values.push_back(std::move(value));
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_EQ(1u, entity->properties.size());
+}
+
+TEST_F(SchemaOrgValidatorTest, InvalidEntityPropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kAboutPage;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kAccessMode;
+  property->values = Values::New();
+
+  EntityPtr value = Entity::New();
+  value->type = entity::kPostalAddress;
+  property->values->entity_values.push_back(std::move(value));
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_TRUE(entity->properties.empty());
+}
+
+TEST_F(SchemaOrgValidatorTest, ValidRepeatedEntityPropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kRestaurant;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kAddress;
+  property->values = Values::New();
+
+  EntityPtr value1 = Entity::New();
+  value1->type = entity::kPostalAddress;
+  EntityPtr value2 = Entity::New();
+  value2->type = entity::kPostalAddress;
+
+  property->values->entity_values.push_back(std::move(value1));
+  property->values->entity_values.push_back(std::move(value2));
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_EQ(1u, entity->properties.size());
+  EXPECT_EQ(2u, entity->properties[0]->values->entity_values.size());
+}
+
+// If one value of a repeated property is invalid but the other is not,
+// validator should keep the outer property and remove only the invalid nested
+// property.
+TEST_F(SchemaOrgValidatorTest, MixedValidityRepeatedEntityPropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kRestaurant;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kAddress;
+  property->values = Values::New();
+
+  EntityPtr value1 = Entity::New();
+  value1->type = entity::kPostalAddress;
+  EntityPtr value2 = Entity::New();
+  value2->type = "bad address";
+
+  property->values->entity_values.push_back(std::move(value1));
+  property->values->entity_values.push_back(std::move(value2));
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_EQ(1u, entity->properties.size());
+  EXPECT_EQ(1u, entity->properties[0]->values->entity_values.size());
+}
+
+TEST_F(SchemaOrgValidatorTest, InvalidRepeatedEntityPropertyValue) {
+  EntityPtr entity = Entity::New();
+  entity->type = entity::kRestaurant;
+
+  PropertyPtr property = Property::New();
+  property->name = property::kAddress;
+  property->values = Values::New();
+
+  EntityPtr value1 = Entity::New();
+  value1->type = "this is not a real type";
+  EntityPtr value2 = Entity::New();
+  value2->type = "bad address type";
+
+  property->values->entity_values.push_back(std::move(value1));
+  property->values->entity_values.push_back(std::move(value2));
+
+  entity->properties.push_back(std::move(property));
+
+  bool validated_entity = ValidateEntity(entity.get());
+  EXPECT_TRUE(validated_entity);
+  EXPECT_TRUE(entity->properties.empty());
+}
+
+}  // namespace schema_org