Add Shill to ONC translation of IPConfig.

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 48 matching lines @@
   scoped_ptr<base::DictionaryValue> CreateTranslatedONCObject();
 
  private:
   void TranslateEthernet();
   void TranslateOpenVPN();
   void TranslateIPsec();
   void TranslateVPN();
   void TranslateWiFiWithState();
   void TranslateCellularWithState();
   void TranslateNetworkWithState();
+  void TranslateIPConfig();
 
   // Creates an ONC object from |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,
                                    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|.
@@ skipping 45 matching lines @@
   } else if (onc_signature_ == &kVPNSignature) {
     TranslateVPN();
   } else if (onc_signature_ == &kOpenVPNSignature) {
     TranslateOpenVPN();
   } else if (onc_signature_ == &kIPsecSignature) {
     TranslateIPsec();
   } else if (onc_signature_ == &kWiFiWithStateSignature) {
     TranslateWiFiWithState();
   } else if (onc_signature_ == &kCellularWithStateSignature) {
     TranslateCellularWithState();
+  } else if (onc_signature_ == &kIPConfigSignature) {
+    TranslateIPConfig();
   } else {
     CopyPropertiesAccordingToSignature();
   }
   return onc_object_.Pass();
 }
 
 void ShillToONCTranslator::TranslateEthernet() {
   std::string shill_network_type;
   shill_dictionary_->GetStringWithoutPathExpansion(shill::kTypeProperty,
                                                    &shill_network_type);
@@ skipping 103 matching lines @@
   CopyPropertiesAccordingToSignature();
   const base::DictionaryValue* dictionary = NULL;
   if (shill_dictionary_->GetDictionaryWithoutPathExpansion(
         shill::kServingOperatorProperty, &dictionary)) {
     TranslateAndAddNestedObject(::onc::cellular::kServingOperator, *dictionary);
   }
   if (shill_dictionary_->GetDictionaryWithoutPathExpansion(
         shill::kCellularApnProperty, &dictionary)) {
     TranslateAndAddNestedObject(::onc::cellular::kAPN, *dictionary);
   }
-  const base::ListValue* list = NULL;
+  const base::ListValue* shill_apns = NULL;
   if (shill_dictionary_->GetListWithoutPathExpansion(
-          shill::kCellularApnListProperty, &list)) {
-    TranslateAndAddListOfObjects(::onc::cellular::kAPNList, *list);
+          shill::kCellularApnListProperty, &shill_apns)) {
+    TranslateAndAddListOfObjects(::onc::cellular::kAPNList, *shill_apns);
   }
 }
 
 void ShillToONCTranslator::TranslateNetworkWithState() {
   CopyPropertiesAccordingToSignature();
 
   std::string shill_network_type;
   shill_dictionary_->GetStringWithoutPathExpansion(shill::kTypeProperty,
                                                    &shill_network_type);
   std::string onc_network_type = ::onc::network_type::kEthernet;
@@ skipping 21 matching lines @@
                                                        &state)) {
     std::string onc_state = ::onc::connection_state::kNotConnected;
     if (NetworkState::StateIsConnected(state)) {
       onc_state = ::onc::connection_state::kConnected;
     } else if (NetworkState::StateIsConnecting(state)) {
       onc_state = ::onc::connection_state::kConnecting;
     }
     onc_object_->SetStringWithoutPathExpansion(
         ::onc::network_config::kConnectionState, onc_state);
   }
+
+  const base::DictionaryValue* shill_ipconfig = NULL;
+  if (shill_dictionary_->GetDictionaryWithoutPathExpansion(
+          shill::kIPConfigProperty, &shill_ipconfig)) {
+    // ONC has a list of IPConfigs and Shill only a single IPConfig object.
+    // Create a list with a single entry.

[stevenjb, 2014/04/23 19:43:42]

So, I think we should fudge this. In Shill, Service.IPConfig is an Object, but
Device.IPConfigs is an array{object}. In order to support ipv6 we will need an
array, so I think Chrome should always use "IPConfigs" and if we fetch these
properties from Service we will put them into a single entry array (ListValue),
but eventually we will need to always fetch them from Device.

[pneubeck (no reviews), 2014/04/25 10:07:34]

Done.
You'll be able to use this translation
 - if an IPConfigS is embedded in a Service
 - or on a standalone IPConfig dictionary (just use the exposed
kIPConfigSignature)
 - but not yet for a stand-alone array of IPConfig objects.

For the latter, we could add a TranslateShillArrayToONCArray function. You can
also repeatedly translate the individual IPConfig dictionaries.

An IPConfig (without 's') _field_ embedded within a Service will not be handled
by the translator.

+    ShillToONCTranslator nested_translator(*shill_ipconfig, kIPConfigSignature);
+    scoped_ptr<base::DictionaryValue> onc_ipconfig =
+        nested_translator.CreateTranslatedONCObject();
+    // If the nested object couldn't be parsed, simply omit it.
+    if (!onc_ipconfig->empty()) {

[armansito, 2014/04/23 18:46:36]

Early return here would be better, i.e if (empty) return;

[pneubeck (no reviews), 2014/04/25 10:07:34]

I kept the pattern in this function, which doesn't returns in order to translate
as much as possible, not early returning on errors.

I'd rather like to keep it this way.
(I adapted TranslateIPConfig)

+      scoped_ptr<base::ListValue> onc_ipconfigs(new base::ListValue());
+      onc_ipconfigs->Append(onc_ipconfig.release());
+      onc_object_->SetWithoutPathExpansion(::onc::network_config::kIPConfigs,
+                                           onc_ipconfigs.release());
+    }
+  }
+}
+
+void ShillToONCTranslator::TranslateIPConfig() {
+  CopyPropertiesAccordingToSignature();
+  std::string ip_address;
+  if (shill_dictionary_->GetStringWithoutPathExpansion(shill::kAddressProperty,
+                                                        &ip_address)) {

[armansito, 2014/04/23 18:46:36]

nit: align "&ip_address" with "shill::kAddressProperty" (looks unaligned in my
browser).

[pneubeck (no reviews), 2014/04/25 10:07:34]

Done.

+    std::string type = ::onc::ipconfig::kIPv4;
+    if (ip_address.find(':') != std::string::npos) {

[stevenjb, 2014/04/23 19:43:42]

Hmm, I guess wwe can use this for identifying ipv6, but I think we're better off
tracking 'Method'.

[pneubeck (no reviews), 2014/04/25 10:07:34]

Missed that one. Thanks.

+      // ip_address is an IPv6 address and not IPv4.
+      type = ::onc::ipconfig::kIPv6;
+    }
+    onc_object_->SetStringWithoutPathExpansion(::onc::ipconfig::kType, type);
+  }
 }
 
 void ShillToONCTranslator::TranslateAndAddNestedObject(
     const std::string& onc_field_name) {
   TranslateAndAddNestedObject(onc_field_name, *shill_dictionary_);
 }
 
 void ShillToONCTranslator::TranslateAndAddNestedObject(
     const std::string& onc_field_name,
     const base::DictionaryValue& dictionary) {
@@ skipping 26 matching lines @@
   for (base::ListValue::const_iterator it = list.begin();
        it != list.end(); ++it) {
     const base::DictionaryValue* shill_value = NULL;
     if (!(*it)->GetAsDictionary(&shill_value))
       continue;
     ShillToONCTranslator nested_translator(
         *shill_value,
         *field_signature->value_signature->onc_array_entry_signature);
     scoped_ptr<base::DictionaryValue> nested_object =
         nested_translator.CreateTranslatedONCObject();
+    // If the nested object couldn't be parsed, simply omit it.
     if (nested_object->empty())
-      // The nested object couldn't be parsed, so simply omit it.
       continue;
     result->Append(nested_object.release());
   }
+  // If there are no entries in the list, there is no need to expose this field.
   if (result->empty())
-    // There are no entries in the list, so there is no need to expose this
-    // field.
     return;
   onc_object_->SetWithoutPathExpansion(onc_field_name, result.release());
 }
 
 void ShillToONCTranslator::CopyPropertiesAccordingToSignature() {
   CopyPropertiesAccordingToSignature(onc_signature_);
 }
 
 void ShillToONCTranslator::CopyPropertiesAccordingToSignature(
     const OncValueSignature* value_signature) {
@@ skipping 56 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