Use Managed properties for Preferred and Provider.

chromeos/network/onc/onc_translator_shill_to_onc.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 "chromeos/network/onc/onc_translator.h"
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/json/json_reader.h"
@@ skipping 71 matching lines @@
   // associated to |onc_field_name| and adds it to |onc_object_| at
   // |onc_field_name|.
   void TranslateAndAddNestedObject(const std::string& onc_field_name,
                                    const base::DictionaryValue& dictionary);
 
   // Creates an ONC object from |shill_dictionary_| according to the signature
   // associated to |onc_field_name| and adds it to |onc_object_| at
   // |onc_field_name|.
   void TranslateAndAddNestedObject(const std::string& onc_field_name);
 
+  // Sets |onc_field_name| in dictionary |onc_dictionary_name| in |onc_object_|
+  // to |value| if the dictionary exists.
+  void SetNestedOncValue(const std::string& onc_dictionary_name,
+                         const std::string& onc_field_name,
+                         const base::Value& value);
+
   // Translates a list of nested objects and adds the list to |onc_object_| at
   // |onc_field_name|. If there are errors while parsing individual objects or
   // if the resulting list contains no entries, the result will not be added to
   // |onc_object_|.
   void TranslateAndAddListOfObjects(const std::string& onc_field_name,
                                     const base::ListValue& list);
 
   // Applies function CopyProperty to each field of |value_signature| and its
   // base signatures.
   void CopyPropertiesAccordingToSignature(
@@ skipping 73 matching lines @@
           shill::kOpenVPNRemoteCertKUProperty, &certKU)) {
     scoped_ptr<base::ListValue> certKUs(new base::ListValue);
     certKUs->AppendString(certKU);
     onc_object_->SetWithoutPathExpansion(::onc::openvpn::kRemoteCertKU,
                                          certKUs.release());
   }
 
   for (const OncFieldSignature* field_signature = onc_signature_->fields;
        field_signature->onc_field_name != NULL; ++field_signature) {
     const std::string& onc_field_name = field_signature->onc_field_name;
-    if (onc_field_name == ::onc::vpn::kSaveCredentials ||
-        onc_field_name == ::onc::openvpn::kRemoteCertKU ||
+    if (onc_field_name == ::onc::openvpn::kRemoteCertKU ||

[pneubeck (no reviews), 2014/08/19 19:26:38]

Why?
These exceptions are for OpenVPN properties that Shill returns not as strings
but as the actual types.

[stevenjb, 2014/08/19 23:20:18]

SaveCredentials is a Service property, not a Service.OpenVPN property, so this
doesn't belong here for the Shill -> ONC translation. I fixed up the unit tests
to deal with the asymetric nature of Provider properties and added an explicit
check for the correct SaveCredentials type in TranslateVPN().

         onc_field_name == ::onc::openvpn::kServerCAPEMs) {
       CopyProperty(field_signature);
       continue;
     }
 
     std::string shill_property_name;
     const base::Value* shill_value = NULL;
     if (!field_translation_table_ ||
         !GetShillPropertyName(field_signature->onc_field_name,
                               field_translation_table_,
@@ skipping 28 matching lines @@
     }
   }
 }
 
 void ShillToONCTranslator::TranslateIPsec() {
   CopyPropertiesAccordingToSignature();
   if (shill_dictionary_->HasKey(shill::kL2tpIpsecXauthUserProperty))
     TranslateAndAddNestedObject(::onc::ipsec::kXAUTH);
 }
 
 void ShillToONCTranslator::TranslateVPN() {

[pneubeck (no reviews), 2014/08/19 19:26:38]

I'll take a more thorough look at this part tomorrow.

[stevenjb, 2014/08/19 23:20:18]

OK, hopefully the need for these changes will be more clear with the updated
tests. (I had been testing with actual Shill results up to now).

-  TranslateWithTableAndSet(
-      shill::kProviderTypeProperty, kVPNTypeTable, ::onc::vpn::kType);
   CopyPropertiesAccordingToSignature();
 
-  std::string vpn_type;
-  if (onc_object_->GetStringWithoutPathExpansion(::onc::vpn::kType,
-                                                 &vpn_type)) {
-    if (vpn_type == ::onc::vpn::kTypeL2TP_IPsec) {
-      TranslateAndAddNestedObject(::onc::vpn::kIPsec);
-      TranslateAndAddNestedObject(::onc::vpn::kL2TP);
-    } else {
-      TranslateAndAddNestedObject(vpn_type);
+  // Parse Shill Provider dictionary.
+  const base::DictionaryValue* provider = NULL;
+  if (!shill_dictionary_->GetDictionaryWithoutPathExpansion(
+          shill::kProviderProperty, &provider)) {
+    LOG(ERROR) << "Shill VPN object with no Provider dictionary";
+    return;
+  }
+  std::string shill_provider_type, onc_provider_type;
+  if (!provider->GetStringWithoutPathExpansion(
+          shill::kTypeProperty, &shill_provider_type)) {
+    LOG(ERROR) << "Shill VPN object with no Provider.Type";
+    return;
+  }
+  TranslateStringToONC(kVPNTypeTable, shill_provider_type, &onc_provider_type);
+  onc_object_->SetStringWithoutPathExpansion(
+      ::onc::vpn::kType, onc_provider_type);
+  std::string provider_host;
+  provider->GetStringWithoutPathExpansion(shill::kHostProperty, &provider_host);
+  onc_object_->SetStringWithoutPathExpansion(::onc::vpn::kHost, provider_host);
+
+  const base::Value* save_credentials = NULL;
+  shill_dictionary_->GetWithoutPathExpansion(
+      shill::kSaveCredentialsProperty, &save_credentials);
+
+  // Translate the nested dictionary.
+  if (onc_provider_type == ::onc::vpn::kTypeL2TP_IPsec) {
+    TranslateAndAddNestedObject(::onc::vpn::kIPsec, *provider);
+    TranslateAndAddNestedObject(::onc::vpn::kL2TP, *provider);
+    if (save_credentials) {
+      SetNestedOncValue(
+          ::onc::vpn::kIPsec, ::onc::vpn::kSaveCredentials, *save_credentials);
+      SetNestedOncValue(
+          ::onc::vpn::kL2TP, ::onc::vpn::kSaveCredentials, *save_credentials);
+    }
+  } else {
+    TranslateAndAddNestedObject(onc_provider_type, *provider);
+    if (save_credentials) {
+      SetNestedOncValue(
+          onc_provider_type, ::onc::vpn::kSaveCredentials, *save_credentials);
     }
   }
 }
 
 void ShillToONCTranslator::TranslateWiFiWithState() {
   TranslateWithTableAndSet(
       shill::kSecurityProperty, kWiFiSecurityTable, ::onc::wifi::kSecurity);
   std::string ssid = shill_property_util::GetSSIDFromProperties(
       *shill_dictionary_, NULL /* ignore unknown encoding */);
   if (!ssid.empty())
@@ skipping 139 matching lines @@
     const std::string& onc_field_name,
     const base::DictionaryValue& dictionary) {
   const OncFieldSignature* field_signature =
       GetFieldSignature(*onc_signature_, onc_field_name);
   DCHECK(field_signature) << "Unable to find signature for field "
                           << onc_field_name << ".";
   ShillToONCTranslator nested_translator(dictionary,
                                          *field_signature->value_signature);
   scoped_ptr<base::DictionaryValue> nested_object =
       nested_translator.CreateTranslatedONCObject();
-  if (nested_object->empty())
-    return;
+  // Note: this may create an empty nested object which is necessary in case
+  // properties are added later with SetNestedOncValue.
   onc_object_->SetWithoutPathExpansion(onc_field_name, nested_object.release());
 }
 
+void ShillToONCTranslator::SetNestedOncValue(
+    const std::string& onc_dictionary_name,
+    const std::string& onc_field_name,
+    const base::Value& value) {
+  base::DictionaryValue* nested;
+  if (!onc_object_->GetDictionaryWithoutPathExpansion(
+          onc_dictionary_name, &nested)) {
+    NOTREACHED();
+  }
+  nested->SetWithoutPathExpansion(onc_field_name, value.DeepCopy());
+}
+
 void ShillToONCTranslator::TranslateAndAddListOfObjects(
     const std::string& onc_field_name,
     const base::ListValue& list) {
   const OncFieldSignature* field_signature =
       GetFieldSignature(*onc_signature_, onc_field_name);
   if (field_signature->value_signature->onc_type != base::Value::TYPE_LIST) {
     LOG(ERROR) << "ONC Field name: '" << onc_field_name << "' has type '"
                << field_signature->value_signature->onc_type
                << "', expected: base::Value::TYPE_LIST.";
     return;
@@ skipping 86 matching lines @@
     const base::DictionaryValue& shill_dictionary,
     const OncValueSignature* onc_signature) {
   CHECK(onc_signature != NULL);
 
   ShillToONCTranslator translator(shill_dictionary, *onc_signature);
   return translator.CreateTranslatedONCObject();
 }
 
 }  // namespace onc
 }  // namespace chromeos