From 8663d9c8ebab5e0d1feb68b8225b801b99374ec9 Mon Sep 17 00:00:00 2001 From: David van Moolenbroek Date: Tue, 29 Oct 2024 09:56:18 +0000 Subject: [PATCH] Fix support for display hints being applied While recent commit git-84be3ff resolved an issue with stacked textual conventions, it broke support for display hints, by changing the method resolution order (MRO) of types based on textual conventions. The orginal MRO was necessary for pysnmp to apply display hints. This commit replaces the previous solution with a new one. The new fix preserves the original MRO and also avoids subclassing the TextualConvention class multiple times for any type, thereby resolving both the original issue and the one newly introduced by git-84be3ff. This new fix does (necessarily) add to "recompilation creep": in order to know whether or not to subclass TextualConvention for a new type, information needs to be carried over between MIBs about whether imported types are simple types or textual conventions. That means that if one MIB is changed (e.g. by changing a type from simple to textual convention or vice versa), the MIBs importing that MIB may have to be recompiled as well. pysmi is not yet smart enough to know about that, and may therefore skip recompiling the importing MIBs, leading to a possible desynchronization. That same issue was already present for handling of DEFVALs as well. New tests are added to ensure that display hint information is indeed not only available, but also applied. Please note that this commit extends the pysmi JSON output format. --- pysmi/codegen/intermediate.py | 27 +++++++ pysmi/codegen/symtable.py | 10 ++- .../templates/pysnmp/mib-definitions.j2 | 6 +- tests/test_imports_smiv2_pysnmp.py | 57 +++++++++++++- tests/test_typedeclaration_smiv2_pysnmp.py | 77 ++++++++++++++++++- 5 files changed, 166 insertions(+), 11 deletions(-) diff --git a/pysmi/codegen/intermediate.py b/pysmi/codegen/intermediate.py index 0dfccc8..e150e05 100644 --- a/pysmi/codegen/intermediate.py +++ b/pysmi/codegen/intermediate.py @@ -228,6 +228,20 @@ def get_base_type(self, symName, module): return baseSymType, symSubtype + def is_type_derived_from_tc(self, symName, module): + """Is the given type derived from a Textual-Convention declaration? + + Given that deriving simple types from textual conventions is currently + not supported altogether, this method tests only the immediate parent. + + Any problems are ignored: if the given type is not definitely derived + from a textual convention, this method returns False. + """ + if module not in self.symbolTable or symName not in self.symbolTable[module]: + return False + + return self.symbolTable[module][symName]["isTC"] + # Clause generation functions # noinspection PyUnusedLocal @@ -564,6 +578,19 @@ def gen_type_declaration(self, data): outDict.update(attrs) self.reg_sym(pysmiName, outDict) + # Establish if this type is derived from a Textual-Convention + # declaration, as needed for pysnmp code generation. + typeType = outDict["type"]["type"] + if ( + typeType in self.symbolTable[self.moduleName[0]] + or typeType in self._importMap + ): + module = self._importMap.get(typeType, self.moduleName[0]) + isDerivedFromTC = self.is_type_derived_from_tc(typeType, module) + else: + isDerivedFromTC = False + outDict["type"]["tcbase"] = isDerivedFromTC + return outDict # noinspection PyUnusedLocal diff --git a/pysmi/codegen/symtable.py b/pysmi/codegen/symtable.py index ac816bf..ea1f799 100644 --- a/pysmi/codegen/symtable.py +++ b/pysmi/codegen/symtable.py @@ -351,12 +351,14 @@ def gen_type_declaration(self, data, classmode=False): pysmiName = self.trans_opers(origName) if declaration: - parentType, attrs = declaration + parentType, attrs, isTC = declaration + if parentType: # skipping SEQUENCE case symProps = { "type": "TypeDeclaration", - "syntax": declaration, # (type, module), subtype + "syntax": (parentType, attrs), # (type, module), subtype "origName": origName, + "isTC": isTC, } self.reg_sym(pysmiName, symProps, [declaration[0][0]]) @@ -555,13 +557,15 @@ def gen_simple_syntax(self, data, classmode=False): def gen_type_declaration_rhs(self, data, classmode=False): if len(data) == 1: parentType, attrs = data[0] # just syntax + isTC = False else: # Textual convention display, status, description, reference, syntax = data parentType, attrs = syntax + isTC = True - return parentType, attrs + return parentType, attrs, isTC # noinspection PyUnusedLocal,PyUnusedLocal,PyMethodMayBeStatic def gen_units(self, data, classmode=False): diff --git a/pysmi/codegen/templates/pysnmp/mib-definitions.j2 b/pysmi/codegen/templates/pysnmp/mib-definitions.j2 index 5902a7a..466ddc6 100644 --- a/pysmi/codegen/templates/pysnmp/mib-definitions.j2 +++ b/pysmi/codegen/templates/pysnmp/mib-definitions.j2 @@ -229,7 +229,11 @@ class {{ symbol }}({{ definition['type']['type'] }}): {% for symbol, definition in mib.items() if definition['class'] == 'textualconvention' %} -class {{ symbol }}({{ definition['type']['type'] }}, TextualConvention): + {% if definition['type']['tcbase'] %} +class {{ symbol }}({{ definition['type']['type'] }}): + {% else %} +class {{ symbol }}(TextualConvention, {{ definition['type']['type'] }}): + {% endif %} status = "{{ definition.get('status', 'current') }}" {% if 'displayhint' in definition %} displayHint = {{ definition['displayhint']|pythonstr }} diff --git a/tests/test_imports_smiv2_pysnmp.py b/tests/test_imports_smiv2_pysnmp.py index 1105d69..a3aae81 100644 --- a/tests/test_imports_smiv2_pysnmp.py +++ b/tests/test_imports_smiv2_pysnmp.py @@ -134,7 +134,10 @@ class ImportTypeTestCase(unittest.TestCase): FROM SNMPv2-SMI TEXTUAL-CONVENTION FROM SNMPv2-TC - ImportedType1, Imported-Type-2, True + ImportedType1, + Imported-Type-2, + True, + ImportedType3 FROM IMPORTED-MIB; testObject1 OBJECT-TYPE @@ -142,6 +145,7 @@ class ImportTypeTestCase(unittest.TestCase): MAX-ACCESS read-only STATUS current DESCRIPTION "Test object" + DEFVAL { '01020304'H } ::= { 1 3 } Test-Type-2 ::= TEXTUAL-CONVENTION @@ -155,6 +159,7 @@ class ImportTypeTestCase(unittest.TestCase): MAX-ACCESS read-only STATUS current DESCRIPTION "Test object" + DEFVAL { 'aabbccdd'H } ::= { 1 4 } False ::= TEXTUAL-CONVENTION @@ -177,6 +182,20 @@ class ImportTypeTestCase(unittest.TestCase): DESCRIPTION "Test object" ::= { 1 6 } + TestType3 ::= TEXTUAL-CONVENTION + DISPLAY-HINT "2d:" + STATUS current + DESCRIPTION "Test TC" + SYNTAX ImportedType3 + + testObject3 OBJECT-TYPE + SYNTAX TestType3 + MAX-ACCESS read-only + STATUS current + DESCRIPTION "Test object" + DEFVAL { '000100020003'H } + ::= { 1 7 } + END """ @@ -189,7 +208,7 @@ class ImportTypeTestCase(unittest.TestCase): FROM SNMPv2-TC; ImportedType1 ::= TEXTUAL-CONVENTION - DISPLAY-HINT "255a" + DISPLAY-HINT "1d:" STATUS current DESCRIPTION "Test TC with display hint" SYNTAX OCTET STRING @@ -204,6 +223,8 @@ class ImportTypeTestCase(unittest.TestCase): DESCRIPTION "Test TC" SYNTAX OCTET STRING + ImportedType3 ::= OCTET STRING + END """ @@ -230,10 +251,15 @@ def testObjectTypeLabel1(self): def testObjectTypeDisplayHint1(self): self.assertEqual( self.ctx["testObject1"].getSyntax().getDisplayHint(), - "255a", + "1d:", "bad display hint", ) + def testObjectTypePrettyValue1(self): + self.assertEqual( + self.ctx["testObject1"].getSyntax().prettyPrint(), "1:2:3:4", "bad defval" + ) + def testObjectTypeName2(self): self.assertEqual(self.ctx["test_object_2"].getName(), (1, 4), "bad value") @@ -249,6 +275,13 @@ def testObjectTypeDisplayHint2(self): "bad display hint", ) + def testObjectTypePrettyValue2(self): + self.assertEqual( + self.ctx["test_object_2"].getSyntax().prettyPrint(), + "aa:bb:cc:dd", + "bad defval", + ) + def testObjectTypeNameReservedKeyword1(self): self.assertEqual(self.ctx["_pysmi_global"].getName(), (1, 5), "bad value") @@ -275,6 +308,24 @@ def testObjectTypeDisplayHintReservedKeyword2(self): "bad display hint", ) + def testObjectTypeName3(self): + self.assertEqual(self.ctx["testObject3"].getName(), (1, 7), "bad value") + + def testObjectTypeLabel3(self): + self.assertEqual(self.ctx["testObject3"].getLabel(), "testObject3", "bad label") + + def testObjectTypeDisplayHint3(self): + self.assertEqual( + self.ctx["testObject3"].getSyntax().getDisplayHint(), + "2d:", + "bad display hint", + ) + + def testObjectTypePrettyValue3(self): + self.assertEqual( + self.ctx["testObject3"].getSyntax().prettyPrint(), "1:2:3", "bad defval" + ) + class ImportSelfTestCase(unittest.TestCase): """ diff --git a/tests/test_typedeclaration_smiv2_pysnmp.py b/tests/test_typedeclaration_smiv2_pysnmp.py index fe1cbd4..2e90e7d 100644 --- a/tests/test_typedeclaration_smiv2_pysnmp.py +++ b/tests/test_typedeclaration_smiv2_pysnmp.py @@ -343,12 +343,12 @@ class TypeDeclarationInheritanceTestCase(unittest.TestCase): """ TEST-MIB DEFINITIONS ::= BEGIN IMPORTS - Unsigned32 + OBJECT-TYPE FROM SNMPv2-SMI TEXTUAL-CONVENTION FROM SNMPv2-TC; - TestTypeUnsigned32 ::= Unsigned32 + TestTypeInteger ::= INTEGER -- -- without constraints @@ -359,14 +359,14 @@ class TypeDeclarationInheritanceTestCase(unittest.TestCase): DISPLAY-HINT "d-1" STATUS current DESCRIPTION "Test TC 1" - SYNTAX Unsigned32 + SYNTAX INTEGER -- textual convention for simple type, derived from base type TestTC-SB ::= TEXTUAL-CONVENTION DISPLAY-HINT "d-2" STATUS current DESCRIPTION "Test TC 2" - SYNTAX TestTypeUnsigned32 + SYNTAX TestTypeInteger -- textual convention for textual convention, derived from base type TestTC-TB ::= TEXTUAL-CONVENTION @@ -431,6 +431,50 @@ class TypeDeclarationInheritanceTestCase(unittest.TestCase): DESCRIPTION "Test TC 10" SYNTAX TestTC-TC (SIZE (20..23)) + -- + -- test objects (without constraints only) + -- + + testObjectB OBJECT-TYPE + SYNTAX TestTC-B + MAX-ACCESS read-only + STATUS current + DESCRIPTION "Test object" + DEFVAL { 123456 } + ::= { 1 4 1 } + + testObjectSB OBJECT-TYPE + SYNTAX TestTC-SB + MAX-ACCESS read-only + STATUS current + DESCRIPTION "Test object" + DEFVAL { 123456 } + ::= { 1 4 2 } + + testObjectTB OBJECT-TYPE + SYNTAX TestTC-TB + MAX-ACCESS read-only + STATUS current + DESCRIPTION "Test object" + DEFVAL { 123456 } + ::= { 1 4 3 } + + testObjectTSB OBJECT-TYPE + SYNTAX TestTC-TSB + MAX-ACCESS read-only + STATUS current + DESCRIPTION "Test object" + DEFVAL { 123456 } + ::= { 1 4 4 } + + testObjectTTB OBJECT-TYPE + SYNTAX TestTC-TTB + MAX-ACCESS read-only + STATUS current + DESCRIPTION "Test object" + DEFVAL { 123456 } + ::= { 1 4 5 } + END """ @@ -551,6 +595,31 @@ def testTextualConventionDisplayHintTTC(self): "bad DISPLAY-HINT", ) + def testObjectTypePrettyValueB(self): + self.assertEqual( + self.ctx["testObjectB"].getSyntax().prettyPrint(), "12345.6", "bad DEFVAL" + ) + + def testObjectTypePrettyValueSB(self): + self.assertEqual( + self.ctx["testObjectSB"].getSyntax().prettyPrint(), "1234.56", "bad DEFVAL" + ) + + def testObjectTypePrettyValueTB(self): + self.assertEqual( + self.ctx["testObjectTB"].getSyntax().prettyPrint(), "123.456", "bad DEFVAL" + ) + + def testObjectTypePrettyValueTSB(self): + self.assertEqual( + self.ctx["testObjectTSB"].getSyntax().prettyPrint(), "12.3456", "bad DEFVAL" + ) + + def testObjectTypePrettyValueTTB(self): + self.assertEqual( + self.ctx["testObjectTTB"].getSyntax().prettyPrint(), "1.23456", "bad DEFVAL" + ) + class TypeDeclarationBitsTextualConventionSyntaxTestCase(unittest.TestCase): """