personalization: EnumParam: implement as value_map, not enum.IntEnum

Because of (1) and (2) below, enum.IntEnum is the wrong tool for
EnumParam. Simplify and fix both problems by using a dict as value map,
with capability for unlimited label string, and unlimited type of value.

In short:

(1) EnumParam's enum labels do not live in the python namespace; they are
    chosen to make good UI labels and CSV values.

(2) enum values should support any type that the given PES attribute
    needs (int, bool, bytes, ...), not only int.

In practice:

(1)
The first purpose of enum.IntEnum is to use the names of an enum in the
python namespace like "MY_LABEL". This means that the enum labels must
be valid python identifiers.

By using enum.IntEnum, we make it impossible to use enum labels like
"SUCI-on" (dash not allowed in python identifier) or "True" (keyword not
allowed as python identifier) or "2" (numeric constant cannot be a
python identifier). IOW, these labels are no longer supported to appear
in a drop-down select box in a web UI.

(2)
The second purpose of enum.IntEnum is to ensure that all values have a
checked type, i.e. int.

By using enum.IntEnum, we can only write int values directly to a PES
attribute. In practice, besides int, some use cases need bool or bytes
etc., i.e. EnumParam should be capable of storing *any* type as value,
as dictated by what needs to be put into the ProfileElementSequence.

History: this patch is the original version of EnumParam, which was
modified during CR to use enum.IntEnum.

Future: new ConfigurableParameters coming up for CR would like to
introduce non-python-identifier enum labels, and non-int values:
- MncLen (labels "2" and "3")
- EuiccMandatoryServiceParam (values True and False)
- EfUstServiceParam like SuciInUsim (labels "SUCI-in-UE" and
  "SUCI-in-USIM")

Change-Id: I690ceccf0ec7ef7067bcaa5cec1303cdaf0f78a4
Jenkins: skip-card-test

pySim/esim/saip/personalization.py

@@ -420,68 +420,70 @@ class BinaryParam(ConfigurableParameter):
 
 
 class EnumParam(ConfigurableParameter):
-    """ConfigurableParameter for named integer enumeration values.
+    """ConfigurableParameter for named value enumerations.
 
-    Subclasses must define a nested enum.IntEnum named 'Values' listing all valid names and their
+    Subclasses define an own value_map, and implement their own apply_val() and get_values_from_pes().
-    integer codes.  apply_val() and get_values_from_pes() are not implemented here and this must
+    """
-    be inherited from another mixin."""
+    value_map = {
-
+            # For example:
-    class Values(enum.IntEnum):
+            #'Meaningful label for value 23': 0x23,
-        pass  # subclasses override this
+            # Where 0x23 is a valid value to use for apply_val(), of any valid type.
+        }
+    _value_map_reverse = None
 
     @classmethod
-    def validate_val(cls, val) -> int:
+    def validate_val(cls, val):
-        if isinstance(val, int):
+        orig_val = val
-            try:
+        enum_val = None
-                return int(cls.Values(val))
+        if isinstance(val, str):
-            except ValueError:
+            enum_name = val
-                pass
+            enum_val = cls.map_name_to_val(enum_name)
-        elif isinstance(val, str):
-            member = cls.map_name_to_val(val, strict=False)
-            if member is not None:
-                return member
 
-        valid = ', '.join(m.name for m in cls.Values)
+        # if the str is not one of the known value_map.keys(), is it maybe one of value_map.keys()?
-        raise ValueError(f"{cls.get_name()}: invalid argument: {val!r}. Valid arguments are: {valid}")
+        if enum_val is None and val in cls.value_map.values():
+            enum_val = val
+
+        if enum_val not in cls.value_map.values():
+            raise ValueError(f"{cls.get_name()}: invalid argument: {orig_val!r}. Valid arguments are:"
+                             f" {', '.join(cls.value_map.keys())}")
+
+        return enum_val
 
     @classmethod
-    def map_name_to_val(cls, name: str, strict=True) -> int:
+    def map_name_to_val(cls, name:str, strict=True):
-        """Return the integer value for a given enum member name. Performs an exact match first,
+        val = cls.value_map.get(name)
-        then falls back to fuzzy matching (case-insensitive, punctuation-insensitive)."""
+        if val is not None:
-        try:
+            return val
-            return int(cls.Values[name])
-        except KeyError:
-            pass
 
-        clean = cls.clean_name_str(name)
+        clean_name = cls.clean_name_str(name)
-        for member in cls.Values:
+        for k, v in cls.value_map.items():
-            if cls.clean_name_str(member.name) == clean:
+            if clean_name == cls.clean_name_str(k):
-                return int(member)
+                return v
 
         if strict:
-            valid = ', '.join(m.name for m in cls.Values)
+            raise ValueError(f"Problem in {cls.get_name()}: {name!r} is not a known value."
-            raise ValueError(f"{cls.get_name()}: {name!r} is not a known value. Known values are: {valid}")
+                    f" Known values are: {cls.value_map.keys()!r}")
         return None
 
     @classmethod
     def map_val_to_name(cls, val, strict=False) -> str:
-        """Return the enum member name for a given integer value."""
+        if cls._value_map_reverse is None:
-        try:
+            cls._value_map_reverse = dict((v, k) for k, v in cls.value_map.items())
-            return cls.Values(val).name
+
-        except ValueError:
+        name = cls._value_map_reverse.get(val)
-            if strict:
+        if name:
-                raise ValueError(f"{cls.get_name()}: {val!r} ({type(val).__name__}) is not a known value.")
+            return name
-            return None
+        if strict:
+            raise ValueError(f"Problem in {cls.get_name()}: {val!r} ({type(val)}) is not a known value."
+                    f" Known values are: {cls.value_map.values()!r}")
+        return None
 
     @classmethod
-    def name_normalize(cls, name: str) -> str:
+    def name_normalize(cls, name:str) -> str:
-        """Map a (possibly fuzzy) name to its canonical enum member name."""
+        return cls.map_val_to_name(cls.map_name_to_val(name))
-        return cls.Values(cls.map_name_to_val(name)).name
 
     @classmethod
-    def clean_name_str(cls, val: str) -> str:
+    def clean_name_str(cls, val):
-        """Strip punctuation and case for fuzzy name comparison.
-        Treats hyphens and underscores as equivalent (both removed)."""
         return re.sub('[^0-9A-Za-z]', '', val).lower()
 
 
@@ -663,69 +665,57 @@ class SmspTpScAddr(ConfigurableParameter):
 
 
 class MncLen(EnumParam):
-    """MNC length.  Sets only the MNC length field in EF.AD (Administrative Data).
+    """MNC length. Must be either 2 or 3. Sets only the MNC length field in EF-AD (Administrative Data)."""
-    Accepted values: integer 2 or 3, digit strings '2' or '3', or enum names 'MNC2'/'MNC3'.
-    """
     name = 'MNC-LEN'
-    example_input = '2'
+    value_map = { '2': 2, '3': 3 }
     default_source = param_source.ConstantSource
-
+    example_input = '2'
-    class Values(enum.IntEnum):
-        MNC2 = 2
-        MNC3 = 3
 
     @classmethod
-    def validate_val(cls, val):
+    def apply_val(cls, pes: ProfileElementSequence, val):
-        if isinstance(val, str) and val.isdigit():
+        """val must be an int: either 2 or 3"""
-            val = int(val)
-        return super().validate_val(val)
-
-    @classmethod
-    def _get_f_ad(cls, pe: ProfileElement):
-        if not hasattr(pe, 'files'):
-            return None
-        f_ad = pe.files.get('ef-ad', None)
-        if f_ad and f_ad.body:
-            return f_ad
-        return None
-
-    @classmethod
-    def _decode_f_ad(cls, f_ad):
-        try:
-            ef_ad_dec = EF_AD().decode_bin(f_ad.body)
-        except StreamError:
-            return None
-        if 'mnc_len' not in ef_ad_dec:
-            return None
-        return ef_ad_dec
-
-    @classmethod
-    def apply_val(cls, pes: ProfileElementSequence, val: int):
         for pe in pes.get_pes_for_type('usim'):
-            f_ad = cls._get_f_ad(pe)
+            if not hasattr(pe, 'files'):
-            if f_ad is None:
+                continue
+            f_ad = pe.files.get('ef-ad')
+            if not f_ad:
                 continue
             # decode existing values
-            ef_ad_dec = cls._decode_f_ad(f_ad)
+            if not f_ad.body:
-            if ef_ad_dec is None:
+                continue
+            try:
+                ef_ad = EF_AD()
+                ef_ad_dec = ef_ad.decode_bin(f_ad.body)
+            except StreamError:
+                continue
+            if 'mnc_len' not in ef_ad_dec:
                 continue
             # change mnc_len
             ef_ad_dec['mnc_len'] = val
             # re-encode into the File body
-            f_ad.body = EF_AD().encode_bin(ef_ad_dec)
+            f_ad.body = ef_ad.encode_bin(ef_ad_dec)
             pe.file2pe(f_ad)
 
     @classmethod
     def get_values_from_pes(cls, pes: ProfileElementSequence):
         for pe in pes.get_pes_for_type('usim'):
-            f_ad = cls._get_f_ad(pe)
+            if not hasattr(pe, 'files'):
+                continue
+            f_ad = pe.files.get('ef-ad', None)
             if f_ad is None:
                 continue
-            ef_ad_dec = cls._decode_f_ad(f_ad)
+
-            if ef_ad_dec is None:
+            try:
+                ef_ad = EF_AD()
+                ef_ad_dec = ef_ad.decode_bin(f_ad.body)
+            except StreamError:
                 continue
-            mnc_len = ef_ad_dec.get('mnc_len')
+
-            yield { cls.name: str(mnc_len) }
+            mnc_len = ef_ad_dec.get('mnc_len', None)
+            if mnc_len is None:
+                continue
+
+            yield { cls.name: cls.map_val_to_name(int(mnc_len)) }
 
 
 class SdKey(BinaryParam):
@@ -1099,17 +1089,17 @@ class AlgorithmID(EnumParam, AlgoConfig):
     """use validate_val() from EnumParam, and apply_val() from AlgoConfig.
     In get_values_from_pes(), return enum value names, not raw values."""
     name = "Algorithm"
+    # as in pySim/esim/asn1/saip/PE_Definitions-3.3.1.asn
+    value_map = {
+            "Milenage" : 1,
+            "TUAK" : 2,
+            "usim-test" : 3,
+        }
     algo_config_key = 'algorithmID'
     example_input = "Milenage"
     default_source = param_source.ConstantSource
 
-    # as in pySim/esim/asn1/saip/PE_Definitions-3.3.1.asn
+    # EnumParam.validate_val() returns the int values from value_map
-    class Values(enum.IntEnum):
-        Milenage  = 1
-        TUAK      = 2
-        usim_test = 3  # input 'usim-test' also accepted via fuzzy matching
-
-    # EnumParam.validate_val() returns the int values from Values
 
     @classmethod
     def get_values_from_pes(cls, pes: ProfileElementSequence):

tests/unittests/test_configurable_parameters.py

@@ -148,7 +148,7 @@ class ConfigurableParameterTest(unittest.TestCase):
             Paramtest(param_cls=p13n.AlgorithmID,
                       val='usim-test',
                       expect_clean_val=3,
-                      expect_val='usim_test'),
+                      expect_val='usim-test'),
 
             Paramtest(param_cls=p13n.AlgorithmID,
                       val=1,
@@ -161,7 +161,7 @@ class ConfigurableParameterTest(unittest.TestCase):
             Paramtest(param_cls=p13n.AlgorithmID,
                       val=3,
                       expect_clean_val=3,
-                      expect_val='usim_test'),
+                      expect_val='usim-test'),
 
             Paramtest(param_cls=p13n.K,
                       val='01020304050607080910111213141516',
@@ -558,7 +558,7 @@ class TestEnumParam(unittest.TestCase):
     def test_validate_by_name_exact(self):
         self.assertEqual(p13n.AlgorithmID.validate_val('Milenage'), 1)
         self.assertEqual(p13n.AlgorithmID.validate_val('TUAK'), 2)
-        self.assertEqual(p13n.AlgorithmID.validate_val('usim_test'), 3)
+        self.assertEqual(p13n.AlgorithmID.validate_val('usim-test'), 3)
 
     def test_validate_by_int(self):
         self.assertEqual(p13n.AlgorithmID.validate_val(1), 1)
@@ -571,7 +571,7 @@ class TestEnumParam(unittest.TestCase):
         self.assertEqual(p13n.AlgorithmID.validate_val('tuak'), 2)
 
     def test_validate_fuzzy_hyphen_underscore(self):
-        # 'usim-test' has a hyphen; enum member is 'usim_test' — must fuzzy-match
+        # 'usim-test' has a hyphen; enum member is 'usim-test' — must fuzzy-match
         self.assertEqual(p13n.AlgorithmID.validate_val('usim-test'), 3)
 
     def test_validate_invalid_name(self):
@@ -608,7 +608,7 @@ class TestEnumParam(unittest.TestCase):
     def test_map_val_known(self):
         self.assertEqual(p13n.AlgorithmID.map_val_to_name(1), 'Milenage')
         self.assertEqual(p13n.AlgorithmID.map_val_to_name(2), 'TUAK')
-        self.assertEqual(p13n.AlgorithmID.map_val_to_name(3), 'usim_test')
+        self.assertEqual(p13n.AlgorithmID.map_val_to_name(3), 'usim-test')
 
     def test_map_val_unknown_nonstrict(self):
         self.assertIsNone(p13n.AlgorithmID.map_val_to_name(99))
@@ -622,7 +622,9 @@ class TestEnumParam(unittest.TestCase):
     def test_name_normalize(self):
         self.assertEqual(p13n.AlgorithmID.name_normalize('Milenage'), 'Milenage')
         self.assertEqual(p13n.AlgorithmID.name_normalize('milenage'), 'Milenage')
-        self.assertEqual(p13n.AlgorithmID.name_normalize('usim-test'), 'usim_test')
+        self.assertEqual(p13n.AlgorithmID.name_normalize('usimtest'), 'usim-test')
+        self.assertEqual(p13n.AlgorithmID.name_normalize('usim_test'), 'usim-test')
+        self.assertEqual(p13n.AlgorithmID.name_normalize('USIM Test'), 'usim-test')
 
     # --- clean_name_str ---
 

tests/unittests/xo/test_configurable_parameters

@@ -77,7 +77,7 @@ ok: TS48v5_SAIP2.1A_NoBERTLV.der AlgorithmID(val=	'TUAK':str)
 
 ok: TS48v5_SAIP2.1A_NoBERTLV.der AlgorithmID(val=	'usim-test':str)
 	clean_val=	3:int
-	read_back_val=	{'Algorithm': 'usim_test'}:{str}
+	read_back_val=	{'Algorithm': 'usim-test'}:{str}
 
 ok: TS48v5_SAIP2.1A_NoBERTLV.der AlgorithmID(val=	1:int)
 	clean_val=	1:int
@@ -89,7 +89,7 @@ ok: TS48v5_SAIP2.1A_NoBERTLV.der AlgorithmID(val=	2:int)
 
 ok: TS48v5_SAIP2.1A_NoBERTLV.der AlgorithmID(val=	3:int)
 	clean_val=	3:int
-	read_back_val=	{'Algorithm': 'usim_test'}:{str}
+	read_back_val=	{'Algorithm': 'usim-test'}:{str}
 
 ok: TS48v5_SAIP2.1A_NoBERTLV.der K(val=	'01020304050607080910111213141516':str)
 	clean_val=	b'\x01\x02\x03\x04\x05\x06\x07\x08\t\x10\x11\x12\x13\x14\x15\x16':bytes
@@ -777,7 +777,7 @@ ok: TS48v5_SAIP2.3_BERTLV_SUCI.der AlgorithmID(val=	'TUAK':str)
 
 ok: TS48v5_SAIP2.3_BERTLV_SUCI.der AlgorithmID(val=	'usim-test':str)
 	clean_val=	3:int
-	read_back_val=	{'Algorithm': 'usim_test'}:{str}
+	read_back_val=	{'Algorithm': 'usim-test'}:{str}
 
 ok: TS48v5_SAIP2.3_BERTLV_SUCI.der AlgorithmID(val=	1:int)
 	clean_val=	1:int
@@ -789,7 +789,7 @@ ok: TS48v5_SAIP2.3_BERTLV_SUCI.der AlgorithmID(val=	2:int)
 
 ok: TS48v5_SAIP2.3_BERTLV_SUCI.der AlgorithmID(val=	3:int)
 	clean_val=	3:int
-	read_back_val=	{'Algorithm': 'usim_test'}:{str}
+	read_back_val=	{'Algorithm': 'usim-test'}:{str}
 
 ok: TS48v5_SAIP2.3_BERTLV_SUCI.der K(val=	'01020304050607080910111213141516':str)
 	clean_val=	b'\x01\x02\x03\x04\x05\x06\x07\x08\t\x10\x11\x12\x13\x14\x15\x16':bytes