Split ValueSerializer into separate Serializer and Deserializer classes.

base/json/json_value_serializer_unittest.cc

 // Copyright (c) 2012 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 <string>
 
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/json/json_reader.h"
@@ skipping 68 matching lines @@
   ASSERT_TRUE(root.get() && root->IsType(Value::TYPE_LIST));
   ListValue* list = static_cast<ListValue*>(root.get());
   ASSERT_EQ(1U, list->GetSize());
   Value* elt = NULL;
   ASSERT_TRUE(list->Get(0, &elt));
   int value = 0;
   ASSERT_TRUE(elt && elt->GetAsInteger(&value));
   ASSERT_EQ(1, value);
 }
 
 // Test proper JSON [de]serialization from string is working.

[rvargas (doing something else), 2015/02/25 03:38:24]

nit: deserialization

 TEST(JSONValueSerializerTest, ReadProperJSONFromString) {

[rvargas (doing something else), 2015/02/25 03:38:24]

nit: Deserializer. (& others)

[prashant.hiremath, 2015/02/25 06:01:37]

Hi Ricardo, 
Thanks for the review.
I'll change the test category name to JSONValueDeserializerTest, which is
explicitly testing deserialization.
But there are some tests which are testing both serialization and
deserialization like "Roundtrip". what should we name for those test categories
? 

[Matt Giuca, 2015/02/25 06:11:40]

I think the test name should probably match the filename. So if we were to
rename half of these tests to JSONValueDeserializerTest, we probably should move
them to json_value_deserializer_unittest.cc. That seems like too much churn,
especially for this already-big CL. I would just leave it as-is (which solves
the dilemma of which test class to put Roundtrip in).

[rvargas (doing something else), 2015/02/25 19:00:15]

While I agree that in general the test name matches the name of the file, I see
that as a consequence of the guideline of having a class Foo live on foo.h.

On the other hand, I see no big issue in having both classes live in the same
header (as they are very closely related)... which also means that tests live
together in the same file.

I think it is more important to match the (main) class that is under test. (see
time_unittest.cc for an example). As for Roundtrip, leaving it under
serialization sounds fine to me.

[Matt Giuca, 2015/02/25 23:20:49]

Acknowledged.

   // Try to deserialize it through the serializer.
-  JSONStringValueSerializer str_deserializer(kProperJSON);
+  JSONStringValueDeserializer str_deserializer(kProperJSON);
 
   int error_code = 0;
   std::string error_message;
-  scoped_ptr<Value> value(
-      str_deserializer.Deserialize(&error_code, &error_message));
-  ASSERT_TRUE(value.get());
-  ASSERT_EQ(0, error_code);
-  ASSERT_TRUE(error_message.empty());
-  // Verify if the same JSON is still there.
-  CheckJSONIsStillTheSame(*value);
-}
-
-// Test proper JSON deserialization from a string pointer is working.
-TEST(JSONValueSerializerTest, ReadProperJSONFromStringPointer) {
-  // Try to deserialize a string pointer through the serializer. (This exercises
-  // a separate code path to passing a StringPiece.)
-  std::string proper_json(kProperJSON);
-  JSONStringValueSerializer str_deserializer(&proper_json);
-
-  int error_code = 0;
-  std::string error_message;
   scoped_ptr<Value> value(
       str_deserializer.Deserialize(&error_code, &error_message));
   ASSERT_TRUE(value.get());
   ASSERT_EQ(0, error_code);
   ASSERT_TRUE(error_message.empty());
   // Verify if the same JSON is still there.
   CheckJSONIsStillTheSame(*value);
 }
 
 // Test proper JSON deserialization from a StringPiece substring.
 TEST(JSONValueSerializerTest, ReadProperJSONFromStringPiece) {
   // Create a StringPiece for the substring of kProperJSONPadded that matches
   // kProperJSON.
   base::StringPiece proper_json(kProperJSONPadded);
   proper_json = proper_json.substr(5, proper_json.length() - 10);
-  JSONStringValueSerializer str_deserializer(proper_json);
+  JSONStringValueDeserializer str_deserializer(proper_json);
 
   int error_code = 0;
   std::string error_message;
   scoped_ptr<Value> value(
       str_deserializer.Deserialize(&error_code, &error_message));
   ASSERT_TRUE(value.get());
   ASSERT_EQ(0, error_code);
   ASSERT_TRUE(error_message.empty());
   // Verify if the same JSON is still there.
   CheckJSONIsStillTheSame(*value);
 }
 
 // Test that trialing commas are only properly deserialized from string when
 // the proper flag for that is set.
 TEST(JSONValueSerializerTest, ReadJSONWithTrailingCommasFromString) {
   // Try to deserialize it through the serializer.
-  JSONStringValueSerializer str_deserializer(kProperJSONWithCommas);
+  JSONStringValueDeserializer str_deserializer(kProperJSONWithCommas);
 
   int error_code = 0;
   std::string error_message;
   scoped_ptr<Value> value(
       str_deserializer.Deserialize(&error_code, &error_message));
   ASSERT_FALSE(value.get());
   ASSERT_NE(0, error_code);
   ASSERT_FALSE(error_message.empty());
   // Now the flag is set and it must pass.
   str_deserializer.set_allow_trailing_comma(true);
   value.reset(str_deserializer.Deserialize(&error_code, &error_message));
   ASSERT_TRUE(value.get());
   ASSERT_EQ(JSONReader::JSON_TRAILING_COMMA, error_code);
   // Verify if the same JSON is still there.
   CheckJSONIsStillTheSame(*value);
 }
 
 // Test proper JSON [de]serialization from file is working.
 TEST(JSONValueSerializerTest, ReadProperJSONFromFile) {
   ScopedTempDir tempdir;
   ASSERT_TRUE(tempdir.CreateUniqueTempDir());
   // Write it down in the file.
   FilePath temp_file(tempdir.path().AppendASCII("test.json"));
   ASSERT_EQ(static_cast<int>(strlen(kProperJSON)),
             WriteFile(temp_file, kProperJSON, strlen(kProperJSON)));
 
   // Try to deserialize it through the serializer.
-  JSONFileValueSerializer file_deserializer(temp_file);
+  JSONFileValueDeserializer file_deserializer(temp_file);
 
   int error_code = 0;
   std::string error_message;
   scoped_ptr<Value> value(
       file_deserializer.Deserialize(&error_code, &error_message));
   ASSERT_TRUE(value.get());
   ASSERT_EQ(0, error_code);
   ASSERT_TRUE(error_message.empty());
   // Verify if the same JSON is still there.
   CheckJSONIsStillTheSame(*value);
 }
 
 // Test that trialing commas are only properly deserialized from file when
 // the proper flag for that is set.
 TEST(JSONValueSerializerTest, ReadJSONWithCommasFromFile) {
   ScopedTempDir tempdir;
   ASSERT_TRUE(tempdir.CreateUniqueTempDir());
   // Write it down in the file.
   FilePath temp_file(tempdir.path().AppendASCII("test.json"));
   ASSERT_EQ(static_cast<int>(strlen(kProperJSONWithCommas)),
             WriteFile(temp_file, kProperJSONWithCommas,
                       strlen(kProperJSONWithCommas)));
 
   // Try to deserialize it through the serializer.
-  JSONFileValueSerializer file_deserializer(temp_file);
+  JSONFileValueDeserializer file_deserializer(temp_file);
   // This must fail without the proper flag.
   int error_code = 0;
   std::string error_message;
   scoped_ptr<Value> value(
       file_deserializer.Deserialize(&error_code, &error_message));
   ASSERT_FALSE(value.get());
   ASSERT_NE(0, error_code);
   ASSERT_FALSE(error_message.empty());
   // Now the flag is set and it must pass.
   file_deserializer.set_allow_trailing_comma(true);
   value.reset(file_deserializer.Deserialize(&error_code, &error_message));
   ASSERT_TRUE(value.get());
   ASSERT_EQ(JSONReader::JSON_TRAILING_COMMA, error_code);
   // Verify if the same JSON is still there.
   CheckJSONIsStillTheSame(*value);
 }
 
 TEST(JSONValueSerializerTest, Roundtrip) {
   static const char kOriginalSerialization[] =
     "{\"bool\":true,\"double\":3.14,\"int\":42,\"list\":[1,2],\"null\":null}";
-  JSONStringValueSerializer serializer(kOriginalSerialization);
-  scoped_ptr<Value> root(serializer.Deserialize(NULL, NULL));
+  JSONStringValueDeserializer deserializer(kOriginalSerialization);
+  scoped_ptr<Value> root(deserializer.Deserialize(NULL, NULL));
   ASSERT_TRUE(root.get());
   ASSERT_TRUE(root->IsType(Value::TYPE_DICTIONARY));
 
   DictionaryValue* root_dict = static_cast<DictionaryValue*>(root.get());
 
   Value* null_value = NULL;
   ASSERT_TRUE(root_dict->Get("null", &null_value));
   ASSERT_TRUE(null_value);
   ASSERT_TRUE(null_value->IsType(Value::TYPE_NULL));
 
   bool bool_value = false;
   ASSERT_TRUE(root_dict->GetBoolean("bool", &bool_value));
   ASSERT_TRUE(bool_value);
 
   int int_value = 0;
   ASSERT_TRUE(root_dict->GetInteger("int", &int_value));
   ASSERT_EQ(42, int_value);
 
   double double_value = 0.0;
   ASSERT_TRUE(root_dict->GetDouble("double", &double_value));
   ASSERT_DOUBLE_EQ(3.14, double_value);
 
-  // We shouldn't be able to write using this serializer, since it was
-  // initialized with a const string.
-  ASSERT_FALSE(serializer.Serialize(*root_dict));
-
   std::string test_serialization;
   JSONStringValueSerializer mutable_serializer(&test_serialization);
   ASSERT_TRUE(mutable_serializer.Serialize(*root_dict));
   ASSERT_EQ(kOriginalSerialization, test_serialization);
 
   mutable_serializer.set_pretty_print(true);
   ASSERT_TRUE(mutable_serializer.Serialize(*root_dict));
   // JSON output uses a different newline style on Windows than on other
   // platforms.
 #if defined(OS_WIN)
@@ skipping 66 matching lines @@
   root.SetString("web", test);
 
   static const char kExpected[] = "{\"web\":\"\xE7\xBD\x91\xE9\xA1\xB5\"}";
 
   std::string actual;
   JSONStringValueSerializer serializer(&actual);
   ASSERT_TRUE(serializer.Serialize(root));
   ASSERT_EQ(kExpected, actual);
 
   // escaped ascii text -> json
-  JSONStringValueSerializer deserializer(kExpected);
+  JSONStringValueDeserializer deserializer(kExpected);
   scoped_ptr<Value> deserial_root(deserializer.Deserialize(NULL, NULL));
   ASSERT_TRUE(deserial_root.get());
   DictionaryValue* dict_root =
       static_cast<DictionaryValue*>(deserial_root.get());
   string16 web_value;
   ASSERT_TRUE(dict_root->GetString("web", &web_value));
   ASSERT_EQ(test, web_value);
 }
 
 TEST(JSONValueSerializerTest, HexStrings) {
   // hex string json -> escaped ascii text
   DictionaryValue root;
   string16 test(WideToUTF16(L"\x01\x02"));
   root.SetString("test", test);
 
   static const char kExpected[] = "{\"test\":\"\\u0001\\u0002\"}";
 
   std::string actual;
   JSONStringValueSerializer serializer(&actual);
   ASSERT_TRUE(serializer.Serialize(root));
   ASSERT_EQ(kExpected, actual);
 
   // escaped ascii text -> json
-  JSONStringValueSerializer deserializer(kExpected);
+  JSONStringValueDeserializer deserializer(kExpected);
   scoped_ptr<Value> deserial_root(deserializer.Deserialize(NULL, NULL));
   ASSERT_TRUE(deserial_root.get());
   DictionaryValue* dict_root =
       static_cast<DictionaryValue*>(deserial_root.get());
   string16 test_value;
   ASSERT_TRUE(dict_root->GetString("test", &test_value));
   ASSERT_EQ(test, test_value);
 
   // Test converting escaped regular chars
   static const char kEscapedChars[] = "{\"test\":\"\\u0067\\u006f\"}";
-  JSONStringValueSerializer deserializer2(kEscapedChars);
+  JSONStringValueDeserializer deserializer2(kEscapedChars);
   deserial_root.reset(deserializer2.Deserialize(NULL, NULL));
   ASSERT_TRUE(deserial_root.get());
   dict_root = static_cast<DictionaryValue*>(deserial_root.get());
   ASSERT_TRUE(dict_root->GetString("test", &test_value));
   ASSERT_EQ(ASCIIToUTF16("go"), test_value);
 }
 
 TEST(JSONValueSerializerTest, AllowTrailingComma) {
   scoped_ptr<Value> root;
   scoped_ptr<Value> root_expected;
   static const char kTestWithCommas[] = "{\"key\": [true,],}";
   static const char kTestNoCommas[] = "{\"key\": [true]}";
 
-  JSONStringValueSerializer serializer(kTestWithCommas);
-  serializer.set_allow_trailing_comma(true);
-  JSONStringValueSerializer serializer_expected(kTestNoCommas);
-  root.reset(serializer.Deserialize(NULL, NULL));
+  JSONStringValueDeserializer deserializer(kTestWithCommas);
+  deserializer.set_allow_trailing_comma(true);
+  JSONStringValueDeserializer deserializer_expected(kTestNoCommas);
+  root.reset(deserializer.Deserialize(NULL, NULL));
   ASSERT_TRUE(root.get());
-  root_expected.reset(serializer_expected.Deserialize(NULL, NULL));
+  root_expected.reset(deserializer_expected.Deserialize(NULL, NULL));
   ASSERT_TRUE(root_expected.get());
   ASSERT_TRUE(root->Equals(root_expected.get()));
 }
 
 TEST(JSONValueSerializerTest, JSONReaderComments) {
   ValidateJsonList("[ // 2, 3, ignore me ] \n1 ]");
   ValidateJsonList("[ /* 2, \n3, ignore me ]*/ \n1 ]");
   ValidateJsonList("//header\n[ // 2, \n// 3, \n1 ]// footer");
   ValidateJsonList("/*\n[ // 2, \n// 3, \n1 ]*/[1]");
   ValidateJsonList("[ 1 /* one */ ] /* end */");
@@ skipping 29 matching lines @@
 };
 
 TEST_F(JSONFileValueSerializerTest, Roundtrip) {
   base::FilePath original_file_path;
   ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &original_file_path));
   original_file_path =
       original_file_path.Append(FILE_PATH_LITERAL("serializer_test.json"));
 
   ASSERT_TRUE(PathExists(original_file_path));
 
-  JSONFileValueSerializer deserializer(original_file_path);
+  JSONFileValueDeserializer deserializer(original_file_path);
   scoped_ptr<Value> root;
   root.reset(deserializer.Deserialize(NULL, NULL));
 
   ASSERT_TRUE(root.get());
   ASSERT_TRUE(root->IsType(Value::TYPE_DICTIONARY));
 
   DictionaryValue* root_dict = static_cast<DictionaryValue*>(root.get());
 
   Value* null_value = NULL;
   ASSERT_TRUE(root_dict->Get("null", &null_value));
@@ skipping 27 matching lines @@
 }
 
 TEST_F(JSONFileValueSerializerTest, RoundtripNested) {
   base::FilePath original_file_path;
   ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &original_file_path));
   original_file_path = original_file_path.Append(
       FILE_PATH_LITERAL("serializer_nested_test.json"));
 
   ASSERT_TRUE(PathExists(original_file_path));
 
-  JSONFileValueSerializer deserializer(original_file_path);
+  JSONFileValueDeserializer deserializer(original_file_path);
   scoped_ptr<Value> root;
   root.reset(deserializer.Deserialize(NULL, NULL));
   ASSERT_TRUE(root.get());
 
   // Now try writing.
   base::FilePath written_file_path = temp_dir_.path().Append(
       FILE_PATH_LITERAL("test_output.json"));
 
   ASSERT_FALSE(PathExists(written_file_path));
   JSONFileValueSerializer serializer(written_file_path);
   ASSERT_TRUE(serializer.Serialize(*root));
   ASSERT_TRUE(PathExists(written_file_path));
 
   // Now compare file contents.
   EXPECT_TRUE(TextContentsEqual(original_file_path, written_file_path));
   EXPECT_TRUE(base::DeleteFile(written_file_path, false));
 }
 
 TEST_F(JSONFileValueSerializerTest, NoWhitespace) {
   base::FilePath source_file_path;
   ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &source_file_path));
   source_file_path = source_file_path.Append(
       FILE_PATH_LITERAL("serializer_test_nowhitespace.json"));
   ASSERT_TRUE(PathExists(source_file_path));
-  JSONFileValueSerializer serializer(source_file_path);
+  JSONFileValueDeserializer deserializer(source_file_path);
   scoped_ptr<Value> root;
-  root.reset(serializer.Deserialize(NULL, NULL));
+  root.reset(deserializer.Deserialize(NULL, NULL));
   ASSERT_TRUE(root.get());
 }
 
 }  // namespace
 
 }  // namespace base