Remove naming-convention from import and add tests

src/azure-cli/azure/cli/command_modules/appconfig/_kv_helpers.py

@@ -28,6 +28,8 @@
 logger = get_logger(__name__)
 FEATURE_FLAG_PREFIX = ".appconfig.featureflag/"
 FEATURE_FLAG_CONTENT_TYPE = "application/vnd.microsoft.appconfig.ff+json;charset=utf-8"
+FEATURE_MANAGEMENT_KEYWORDS = ["FeatureManagement", "featureManagement", "feature_management", "feature-management"]
+ENABLED_FOR_KEYWORDS = ["EnabledFor", "enabledFor", "enabled_for", "enabled-for"]
 
 
 class FeatureManagementReservedKeywords(object):
@@ -41,25 +43,25 @@ class FeatureManagementReservedKeywords(object):
     '''
 
     def pascal(self):
-        self.featuremanagement = "FeatureManagement"
-        self.enabledfor = "EnabledFor"
+        self.featuremanagement = FEATURE_MANAGEMENT_KEYWORDS[0]
+        self.enabledfor = ENABLED_FOR_KEYWORDS[0]
 
     def camel(self):
-        self.featuremanagement = "featureManagement"
-        self.enabledfor = "enabledFor"
+        self.featuremanagement = FEATURE_MANAGEMENT_KEYWORDS[1]
+        self.enabledfor = ENABLED_FOR_KEYWORDS[1]
 
     def underscore(self):
-        self.featuremanagement = "feature_management"
-        self.enabledfor = "enabled_for"
+        self.featuremanagement = FEATURE_MANAGEMENT_KEYWORDS[2]
+        self.enabledfor = ENABLED_FOR_KEYWORDS[2]
 
     def hyphen(self):
-        self.featuremanagement = "feature-management"
-        self.enabledfor = "enabled-for"
+        self.featuremanagement = FEATURE_MANAGEMENT_KEYWORDS[3]
+        self.enabledfor = ENABLED_FOR_KEYWORDS[3]
 
     def __init__(self,
                  naming_convention):
-        self.featuremanagement = "FeatureManagement"
-        self.enabledfor = "EnabledFor"
+        self.featuremanagement = FEATURE_MANAGEMENT_KEYWORDS[0]
+        self.enabledfor = ENABLED_FOR_KEYWORDS[0]
 
         if naming_convention != 'pascal':
             select_keywords = getattr(self, naming_convention, self.pascal)
@@ -90,28 +92,35 @@ def __compare_kvs_for_restore(restore_kvs, current_kvs):
 # File <-> List of KeyValue object
 
 
-def __read_kv_from_file(file_path, format_, feature_reserved_keywords, separator=None, prefix_to_add="", depth=None):
+def __read_kv_from_file(file_path, format_, separator=None, prefix_to_add="", depth=None):
     config_data = {}
     try:
         with io.open(file_path, 'r', encoding=__check_file_encoding(file_path)) as config_file:
             if format_ == 'json':
                 config_data = json.load(config_file)
-                if feature_reserved_keywords.featuremanagement in config_data:
-                    del config_data[feature_reserved_keywords.featuremanagement]
+                for feature_management_keyword in FEATURE_MANAGEMENT_KEYWORDS:
+                    # delete all feature management sections in any name format.
+                    # If users have not skipped features, and there are multiple
+                    # feature sections, we will error out while reading features.
+                    if feature_management_keyword in config_data:
+                        del config_data[feature_management_keyword]
 
             elif format_ == 'yaml':
                 for yaml_data in list(yaml.safe_load_all(config_file)):
                     config_data.update(yaml_data)
-                if feature_reserved_keywords.featuremanagement in config_data:
-                    del config_data[feature_reserved_keywords.featuremanagement]
+                for feature_management_keyword in FEATURE_MANAGEMENT_KEYWORDS:
+                    # delete all feature management sections in any name format.
+                    # If users have not skipped features, and there are multiple
+                    # feature sections, we will error out while reading features.
+                    if feature_management_keyword in config_data:
+                        del config_data[feature_management_keyword]
 
             elif format_ == 'properties':
                 config_data = javaproperties.load(config_file)
                 logger.warning("Importing feature flags from a properties file is not supported. If properties file contains feature flags, they will be imported as regular key-values.")
 
     except ValueError:
-        raise CLIError(
-            'The input is not a well formatted %s file.' % (format_))
+        raise CLIError('The input is not a well formatted %s file.' % (format_))
     except OSError:
         raise CLIError('File is not available.')
     flattened_data = {}
@@ -133,9 +142,12 @@ def __read_kv_from_file(file_path, format_, feature_reserved_keywords, separator
     return key_values
 
 
-def __read_features_from_file(file_path, format_, feature_reserved_keywords):
+def __read_features_from_file(file_path, format_):
     config_data = {}
     features_dict = {}
+    # Default is PascalCase, but it will always be overwritten as long as there is a feature section in file
+    enabled_for_keyword = ENABLED_FOR_KEYWORDS[0]
+
     if format_ == 'properties':
         logger.warning("Importing feature flags from a properties file is not supported. If properties file contains feature flags, they will be imported as regular key-values.")
         return features_dict
@@ -144,14 +156,23 @@ def __read_features_from_file(file_path, format_, feature_reserved_keywords):
         with io.open(file_path, 'r', encoding=__check_file_encoding(file_path)) as config_file:
             if format_ == 'json':
                 config_data = json.load(config_file)
-                if feature_reserved_keywords.featuremanagement in config_data:
-                    features_dict = config_data[feature_reserved_keywords.featuremanagement]
 
             elif format_ == 'yaml':
                 for yaml_data in list(yaml.safe_load_all(config_file)):
                     config_data.update(yaml_data)
-                if feature_reserved_keywords.featuremanagement in config_data:
-                    features_dict = config_data[feature_reserved_keywords.featuremanagement]
+
+        found_feature_section = False
+        for index, feature_management_keyword in enumerate(FEATURE_MANAGEMENT_KEYWORDS):
+            # find the first occurence of feature management section in file.
+            # Enforce the same naming convention for 'EnabledFor' keyword
+            # If there are multiple feature sections, we will error out here.
+            if feature_management_keyword in config_data:
+                if not found_feature_section:
+                    features_dict = config_data[feature_management_keyword]
+                    enabled_for_keyword = ENABLED_FOR_KEYWORDS[index]
+                    found_feature_section = True
+                else:
+                    raise CLIError('Unable to proceed because file contains multiple sections corresponding to "Feature Management".')
 
     except ValueError:
         raise CLIError(
@@ -160,14 +181,14 @@ def __read_features_from_file(file_path, format_, feature_reserved_keywords):
         raise CLIError('File is not available.')
 
     # features_dict contains all features that need to be converted to KeyValue format now
-    return __convert_feature_dict_to_keyvalue_list(features_dict, feature_reserved_keywords)
+    return __convert_feature_dict_to_keyvalue_list(features_dict, enabled_for_keyword)
 
 
-def __write_kv_and_features_to_file(file_path, key_values=None, features=None, format_=None, separator=None, skip_features=False, feature_reserved_keywords='pascal'):
+def __write_kv_and_features_to_file(file_path, key_values=None, features=None, format_=None, separator=None, skip_features=False, naming_convention='pascal'):
     try:
         exported_keyvalues = __export_keyvalues(key_values, format_, separator, None)
         if features and not skip_features:
-            exported_features = __export_features(features, feature_reserved_keywords)
+            exported_features = __export_features(features, naming_convention)
             exported_keyvalues.update(exported_features)
 
         with open(file_path, 'w') as fp:
@@ -584,7 +605,8 @@ def __export_keyvalues(fetched_items, format_, separator, prefix=None):
         raise CLIError("Fail to export key-values." + str(exception))
 
 
-def __export_features(retrieved_features, feature_reserved_keywords):
+def __export_features(retrieved_features, naming_convention):
+    feature_reserved_keywords = FeatureManagementReservedKeywords(naming_convention)
     exported_dict = {}
     exported_dict[feature_reserved_keywords.featuremanagement] = {}
     client_filters = []
@@ -621,7 +643,7 @@ def __export_features(retrieved_features, feature_reserved_keywords):
         raise CLIError("Failed to export feature flags. " + str(exception))
 
 
-def __convert_feature_dict_to_keyvalue_list(features_dict, feature_reserved_keywords):
+def __convert_feature_dict_to_keyvalue_list(features_dict, enabled_for_keyword):
     # pylint: disable=too-many-nested-blocks
     key_values = []
     default_conditions = {'client_filters': []}
@@ -635,9 +657,9 @@ def __convert_feature_dict_to_keyvalue_list(features_dict, feature_reserved_keyw
                 # This may be a conditional feature
                 feature_flag_value.enabled = False
                 try:
-                    feature_flag_value.conditions = {'client_filters': v[feature_reserved_keywords.enabledfor]}
+                    feature_flag_value.conditions = {'client_filters': v[enabled_for_keyword]}
                 except KeyError:
-                    raise CLIError("Feature '{0}' must contain '{1}' definition or have a true/false value. \n".format(str(k), feature_reserved_keywords.enabledfor))
+                    raise CLIError("Feature '{0}' must contain '{1}' definition or have a true/false value. \n".format(str(k), enabled_for_keyword))
 
                 if feature_flag_value.conditions["client_filters"]:
                     feature_flag_value.enabled = True

src/azure-cli/azure/cli/command_modules/appconfig/_params.py

@@ -87,7 +87,6 @@ def load_arguments(self, _):
         c.argument('depth', validator=validate_import_depth, help="Depth for flattening the json or yaml file to key-value pairs. Flatten to the deepest level by default. Not applicable for property files or feature flags.")
         # bypass cli allowed values limition
         c.argument('separator', validator=validate_separator, help="Delimiter for flattening the json or yaml file to key-value pairs. Required for importing hierarchical structure. Separator will be ignored for property files and feature flags. Supported values: '.', ',', ';', '-', '_', '__', '/', ':' ")
-        c.argument('naming_convention', arg_type=get_enum_type(['pascal', 'camel', 'underscore', 'hyphen']), help='Naming convention being used for "Feature Management" section of file. Default is pascal. Example: pascal = FeatureManagement, camel = featureManagement, underscore = feature_management, hyphen = feature-management.')
 
     with self.argument_context('appconfig kv import', arg_group='AppConfig') as c:
         c.argument('src_name', help='The name of the source App Configuration.')

src/azure-cli/azure/cli/command_modules/appconfig/keyvalue.py

@@ -20,7 +20,7 @@
                                ModifyKeyValueOptions,
                                QueryKeyValueCollectionOptions,
                                QueryKeyValueOptions)
-from ._kv_helpers import (FeatureManagementReservedKeywords, __compare_kvs_for_restore, __read_kv_from_file, __read_features_from_file,
+from ._kv_helpers import (__compare_kvs_for_restore, __read_kv_from_file, __read_features_from_file,
                           __write_kv_and_features_to_file, __read_kv_from_config_store,
                           __write_kv_and_features_to_config_store, __discard_features_from_retrieved_kv, __read_kv_from_app_service,
                           __write_kv_to_app_service, __serialize_kv_list_to_comparable_json_object, __serialize_features_from_kv_list_to_comparable_json_object,
@@ -52,31 +52,22 @@ def import_config(cmd,
                   src_label=None,
                   # from-appservice parameters
                   appservice_account=None,
-                  skip_features=False,
-                  naming_convention=None):
+                  skip_features=False):
     # pylint: disable=too-many-locals
     src_features = []
     dest_features = []
     dest_kvs = []
     source = source.lower()
     format_ = format_.lower() if format_ else None
 
-    if not naming_convention:
-        logger.warning("--naming-convention was not provided. Reserved keywords for feature management section will default to PascalCase.")
-        naming_convention = 'pascal'
-    else:
-        naming_convention = naming_convention.lower()
-
-    feature_reserved_keywords = FeatureManagementReservedKeywords(naming_convention)
-
     # fetch key values from source
     if source == 'file':
         src_kvs = __read_kv_from_file(
-            file_path=path, format_=format_, feature_reserved_keywords=feature_reserved_keywords, separator=separator, prefix_to_add=prefix, depth=depth)
+            file_path=path, format_=format_, separator=separator, prefix_to_add=prefix, depth=depth)
 
         if not skip_features:
             # src_features is a list of KeyValue objects
-            src_features = __read_features_from_file(file_path=path, format_=format_, feature_reserved_keywords=feature_reserved_keywords)
+            src_features = __read_features_from_file(file_path=path, format_=format_)
 
     elif source == 'appconfig':
         src_kvs = __read_kv_from_config_store(cmd, name=src_name, connection_string=src_connection_string,
@@ -159,13 +150,6 @@ def export_config(cmd,
     dest_kvs = []
     destination = destination.lower()
     format_ = format_.lower() if format_ else None
-    if not naming_convention:
-        logger.warning("--naming-convention was not provided. Reserved keywords for feature management section will default to PascalCase.")
-        naming_convention = 'pascal'
-    else:
-        naming_convention = naming_convention.lower()
-
-    feature_reserved_keywords = FeatureManagementReservedKeywords(naming_convention)
 
     # fetch key values from user's configstore
     src_kvs = __read_kv_from_config_store(
@@ -176,7 +160,24 @@ def export_config(cmd,
 
     if not skip_features:
         # Get all Feature flags with matching label
-        if destination in ('file', 'appconfig'):
+        if destination == 'file':
+            if format_ == 'properties':
+                skip_features = True
+            else:
+                if not naming_convention:
+                    logger.warning("--naming-convention was not provided. Reserved keywords for feature management section will default to PascalCase.")
+                    naming_convention = 'pascal'
+                else:
+                    naming_convention = naming_convention.lower()
+
+                # src_features is a list of FeatureFlag objects
+                src_features = list_feature(cmd,
+                                            feature='*',
+                                            label=QueryKeyValueCollectionOptions.empty_label if label is None else label,
+                                            name=name,
+                                            connection_string=connection_string,
+                                            all_=True)
+        elif destination == 'appconfig':
             # src_features is a list of FeatureFlag objects
             src_features = list_feature(cmd,
                                         feature='*',
@@ -225,7 +226,7 @@ def export_config(cmd,
     if destination == 'file':
         __write_kv_and_features_to_file(file_path=path, key_values=src_kvs, features=src_features,
                                         format_=format_, separator=separator, skip_features=skip_features,
-                                        feature_reserved_keywords=feature_reserved_keywords)
+                                        naming_convention=naming_convention)
     elif destination == 'appconfig':
         __write_kv_and_features_to_config_store(cmd, key_values=src_kvs, features=src_features, name=dest_name,
                                                 connection_string=dest_connection_string, label=dest_label)

src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/export_as_kv.properties

@@ -0,0 +1,11 @@
+Color=Red
+Region=West US
+feature-management.FalseFeature=false
+feature-management.FeatureSample.enabled-for[0].Name=Filter1
+feature-management.FeatureSample.enabled-for[0].Parameters.paramforfilter1=value1
+feature-management.FeatureSample.enabled-for[1].Name=Filter2
+feature-management.FeatureSample.enabled-for[2].Name=Filter@3
+feature-management.FeatureSample.enabled-for[3].Name=filter.4
+feature-management.FeatureSample.enabled-for[3].Parameters.EmptyValue=
+feature-management.FeatureSample.enabled-for[3].Parameters.dotInFilter.Param=?
+feature-management.TrueFeature=true

src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/export_features.json

@@ -1,11 +1,11 @@
 {
   "Color": "Red",
   "Region": "West US",
-  "FeatureManagement": {
+  "feature_management": {
     "Beta": false,
     "Percentage": true,
     "Timestamp": {
-      "EnabledFor": [
+      "enabled_for": [
         {
           "Name": "Local Tests",
           "Parameters": {

src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/export_features.properties

@@ -0,0 +1,12 @@
+#Fri Dec 20 14:14:34 Pacific Standard Time 2019
+Color=Red
+Region=West US
+feature-management.FalseFeature=false
+feature-management.FeatureSample.enabled-for[0].Name=Filter1
+feature-management.FeatureSample.enabled-for[0].Parameters.paramforfilter1=value1
+feature-management.FeatureSample.enabled-for[1].Name=Filter2
+feature-management.FeatureSample.enabled-for[2].Name=Filter@3
+feature-management.FeatureSample.enabled-for[3].Name=filter.4
+feature-management.FeatureSample.enabled-for[3].Parameters.EmptyValue=
+feature-management.FeatureSample.enabled-for[3].Parameters.dotInFilter.Param=?
+feature-management.TrueFeature=true

src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/export_features.yaml

@@ -0,0 +1,14 @@
+Color: Red
+Region: West US
+feature-management:
+  Beta: false
+  Timestamp:
+    enabled-for:
+    - Name: Local Tests
+      Parameters:
+        StartTime: '2019-01-01T00:00:00Z'
+        EndTime: '2019-09-01T00:00:00Z'
+    - Name: Production Tests
+      Parameters:
+        StartTime: '2019-09-02T00:00:00Z'
+        EndTime: '2019-11-01T00:00:00Z'

src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/export_features_hyphen.yaml

@@ -0,0 +1,14 @@
+Color: Red
+Region: West US
+feature-management:
+  Beta: false
+  Timestamp:
+    enabled-for:
+    - Name: Local Tests
+      Parameters:
+        StartTime: '2019-01-01T00:00:00Z'
+        EndTime: '2019-09-01T00:00:00Z'
+    - Name: Production Tests
+      Parameters:
+        StartTime: '2019-09-02T00:00:00Z'
+        EndTime: '2019-11-01T00:00:00Z'

src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/export_features_underscore.json

@@ -0,0 +1,26 @@
+{
+  "Color": "Red",
+  "Region": "West US",
+  "feature_management": {
+    "Beta": false,
+    "Percentage": true,
+    "Timestamp": {
+      "enabled_for": [
+        {
+          "Name": "Local Tests",
+          "Parameters": {
+            "StartTime": "2019-01-01T00:00:00Z",
+            "EndTime": "2019-09-01T00:00:00Z"
+          }
+        },
+        {
+          "Name": "Production Tests",
+          "Parameters": {
+            "StartTime": "2019-09-02T00:00:00Z",
+            "EndTime": "2019-11-01T00:00:00Z"
+          }
+        }
+      ]
+    }
+  }
+}

src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/import_features.properties

@@ -0,0 +1,11 @@
+feature-management.FeatureSample.enabled-for[0].Name=Filter1
+feature-management.FeatureSample.enabled-for[0].Parameters.paramforfilter1=value1
+feature-management.FeatureSample.enabled-for[1].Name=Filter2
+feature-management.FeatureSample.enabled-for[2].Name=Filter@3
+feature-management.FeatureSample.enabled-for[3].Name=filter.4
+feature-management.FeatureSample.enabled-for[3].Parameters.EmptyValue=
+feature-management.FeatureSample.enabled-for[3].Parameters.dotInFilter.Param=?
+feature-management.TrueFeature=true
+feature-management.FalseFeature=false
+Color=Red
+Region=West US