Use GetManagedProperties in InternetOptionsHandler

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 97 matching lines @@
       const OncValueSignature* value_signature);
 
   // Applies function CopyProperty to each field of |onc_signature_| and its
   // base signatures.
   void CopyPropertiesAccordingToSignature();
 
   // If |shill_property_name| is defined in |field_signature|, copies this
   // entry from |shill_dictionary_| to |onc_object_| if it exists.
   void CopyProperty(const OncFieldSignature* field_signature);
 
+  // If |shill_property_name| exists in |shill_dictionary_|, copy it to
+  // |dictionary|.

[pneubeck (no reviews), 2014/09/03 15:14:29]

nit: mention that it copies to the key |onc_property_name| (called
onc_field_name in all other comments)

[stevenjb, 2014/09/03 21:28:06]

Done, and renamed -> onc_field_name for consistency.

+  void CopyPropertyToDictionary(const std::string& shill_property_name,
+                                const std::string& onc_property_name,
+                                base::DictionaryValue* dictionary);
+
   // If existent, translates the entry at |shill_property_name| in
   // |shill_dictionary_| using |table|. It is an error if no matching table
   // entry is found. Writes the result as entry at |onc_field_name| in
   // |onc_object_|.
   void TranslateWithTableAndSet(const std::string& shill_property_name,
                                 const StringTranslationEntry table[],
                                 const std::string& onc_field_name);
 
   // Returns the name of the Shill service provided in |shill_dictionary_|
   // for debugging.
@@ skipping 242 matching lines @@
   // Translate nested Cellular, WiFi, etc. properties.
   if (!onc_network_type.empty()) {
     onc_object_->SetStringWithoutPathExpansion(::onc::network_config::kType,
                                                onc_network_type);
     TranslateAndAddNestedObject(onc_network_type);
   }
 
   // Since Name is a read only field in Shill unless it's a VPN, it is copied
   // here, but not when going the other direction (if it's not a VPN).
   std::string name;
-  shill_dictionary_->GetStringWithoutPathExpansion(shill::kNameProperty,
-                                                   &name);
+  shill_dictionary_->GetStringWithoutPathExpansion(shill::kNameProperty, &name);
   onc_object_->SetStringWithoutPathExpansion(::onc::network_config::kName,
                                              name);
 
   // Limit ONC state to "NotConnected", "Connected", or "Connecting".
   std::string state;
   if (shill_dictionary_->GetStringWithoutPathExpansion(shill::kStateProperty,
                                                        &state)) {
     std::string onc_state = ::onc::connection_state::kNotConnected;
     if (NetworkState::StateIsConnected(state)) {
       onc_state = ::onc::connection_state::kConnected;
@@ skipping 19 matching lines @@
 
   // Shill's Service has an IPConfig property (note the singular), not an
   // IPConfigs property. However, we require the caller of the translation to
   // patch the Shill dictionary before passing it to the translator.
   const base::ListValue* shill_ipconfigs = NULL;
   if (shill_dictionary_->GetListWithoutPathExpansion(shill::kIPConfigsProperty,
                                                      &shill_ipconfigs)) {
     TranslateAndAddListOfObjects(::onc::network_config::kIPConfigs,
                                  *shill_ipconfigs);
   }
+
+  // Convert StaticIP and SavedIP properties to dictionary format.
+  scoped_ptr<base::DictionaryValue> static_ip(new base::DictionaryValue);

[pneubeck (no reviews), 2014/09/03 15:14:28]

I think if you can save most of this code by supporting both translation
directions by adding the right signature and entries to the translation tables:

Add two new signatures that point to the base IPConfig signature (these
signatures inherit all members from kIPConfigSignature):

const OncValueSignature kStaticIPConfigSignature = {
 base::Value::TYPE_DICTIONARY, NULL, NULL, &kIPConfigSignature };

const OncValueSignature kSavedIPConfigSignature = {
 base::Value::TYPE_DICTIONARY, NULL, NULL, &kIPConfigSignature };

... network_configuration_fields[] = {
   ...
   { ::onc::network_config::kSavedIPConfig, &kSavedIPConfigSignature},
   { ::onc::network_config::kStaticIPConfig, &kStaticIPConfigSignature},
   ...
};

And for each signature add a translation table:

const FieldTranslationEntry static_ipconfig_fields[] = {
  { ::onc::ipconfig::kIPAddress, shill::kStaticIPAddressProperty},
  { ::onc::ipconfig::kGateway, shill::kStaticIPGatewayProperty},
  { ::onc::ipconfig::kRoutingPrefix, shill::kStaticIPPrefixlenProperty},
  { ::onc::ipconfig::kNameServers, shill::kStaticIPNameServersProperty}
};

const FieldTranslationEntry saved_ipconfig_fields[] = {
  ...
};

const OncValueTranslationEntry onc_value_translation_table[] = {
  ...
  { &kSavedIPConfigSignature, saved_ipconfig_fields },
  { &kStaticIPConfigSignature, static_ipconfig_fields },
  ...
};


To trigger the translation of these nested objects, add here the calls
 TranslateAndAddNestedObject(::onc::network_config::kSavedIPConfig)
 TranslateAndAddNestedObject(::onc::network_config::kStaticIPConfig)

The new function CopyPropertyToDictionary will not be necessary anymore and you
get the other direction of translation at the same time.

[stevenjb, 2014/09/03 21:28:06]

I'll... try doing this in a separate CL. I think I agree that sounds better, but
it all still kind of makes my head hurt.

[pneubeck (no reviews), 2014/09/04 14:54:13]

Thanks

+  CopyPropertyToDictionary(shill::kStaticIPAddressProperty,
+                           ::onc::ipconfig::kIPAddress,
+                           static_ip.get());
+  CopyPropertyToDictionary(shill::kStaticIPGatewayProperty,
+                           ::onc::ipconfig::kGateway,
+                           static_ip.get());
+  CopyPropertyToDictionary(shill::kStaticIPNameServersProperty,
+                           ::onc::ipconfig::kNameServers,
+                           static_ip.get());
+  CopyPropertyToDictionary(shill::kStaticIPPrefixlenProperty,
+                           ::onc::ipconfig::kRoutingPrefix,
+                           static_ip.get());
+  if (!static_ip->empty()) {
+    onc_object_->SetWithoutPathExpansion(::onc::network_config::kStaticIPConfig,
+                                         static_ip.release());
+  }
+
+  scoped_ptr<base::DictionaryValue> saved_ip(new base::DictionaryValue);
+  CopyPropertyToDictionary(shill::kSavedIPAddressProperty,
+                           ::onc::ipconfig::kIPAddress,
+                           saved_ip.get());
+  CopyPropertyToDictionary(shill::kSavedIPGatewayProperty,
+                           ::onc::ipconfig::kGateway,
+                           saved_ip.get());
+  CopyPropertyToDictionary(shill::kSavedIPNameServersProperty,
+                           ::onc::ipconfig::kNameServers,
+                           saved_ip.get());
+  CopyPropertyToDictionary(shill::kSavedIPPrefixlenProperty,
+                           ::onc::ipconfig::kRoutingPrefix,
+                           saved_ip.get());
+  if (!saved_ip->empty()) {
+    onc_object_->SetWithoutPathExpansion(::onc::network_config::kSavedIPConfig,
+                                         saved_ip.release());
+  }
 }
 
 void ShillToONCTranslator::TranslateIPConfig() {
   CopyPropertiesAccordingToSignature();
   std::string shill_ip_method;
   shill_dictionary_->GetStringWithoutPathExpansion(shill::kMethodProperty,
                                                    &shill_ip_method);
   std::string type;
   if (shill_ip_method == shill::kTypeIPv4 ||
       shill_ip_method == shill::kTypeDHCP) {
@@ skipping 113 matching lines @@
                << "' requires type "
                << field_signature->value_signature->onc_type
                << ": " << GetName();
     return;
   }
 
  onc_object_->SetWithoutPathExpansion(field_signature->onc_field_name,
                                       shill_value->DeepCopy());
 }
 
+void ShillToONCTranslator::CopyPropertyToDictionary(
+    const std::string& shill_property_name,
+    const std::string& onc_property_name,
+    base::DictionaryValue* dictionary) {
+  const base::Value* shill_value;
+  if (!shill_dictionary_->GetWithoutPathExpansion(shill_property_name,
+                                                  &shill_value)) {
+    return;
+  }
+  dictionary->SetWithoutPathExpansion(onc_property_name,
+                                      shill_value->DeepCopy());
+}
+
 void ShillToONCTranslator::TranslateWithTableAndSet(
     const std::string& shill_property_name,
     const StringTranslationEntry table[],
     const std::string& onc_field_name) {
   std::string shill_value;
   if (!shill_dictionary_->GetStringWithoutPathExpansion(shill_property_name,
                                                         &shill_value)) {
     return;
   }
   std::string onc_value;
@@ skipping 18 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