Factor out logic to determine serial number from ENTITY-MIB in Layer2 and Layer3 to new method entity_derived_serial in Entity class

Create new method entity_derived_os_ver in Entity class to determine OS version in a similar manner

lib/SNMP/Info/Entity.pm

@@ -109,6 +109,57 @@ sub e_port {
     return \%e_port;
 }
 
+sub entity_derived_serial {
+    my $entity  = shift;
+
+    my $e_parent = $entity->e_parent() || {};
+    my $e_class  = $entity->e_class() || {};
+
+    # Sort keys to return a consistent result between runs
+    foreach my $iid ( sort keys %$e_parent ) {
+        my $parent = $e_parent->{$iid};
+        my $class = $e_class->{$iid} || '';
+        # Only consider serial numbers for entries without a parent, or
+        # if they are of type "chassis"
+        if ( $parent eq '0' or $class eq 'chassis') {
+            my $serial = $entity->e_serial($iid);
+            if ( $serial && $serial->{$iid} ) {
+                return $serial->{$iid};
+            }
+            else {
+                my $descr = $entity->e_descr($iid);
+                if ( $descr and $descr->{$iid} =~ /serial#?:\s*([a-z0-9]+)/i )
+                {
+                    return $1;
+                }
+            }
+        }
+    }
+    return;
+}
+
+sub entity_derived_os_ver {
+    my $entity  = shift;
+
+    my $e_parent = $entity->e_parent() || {};
+    my $e_class  = $entity->e_class() || {};
+
+    # Sort keys to return a consistent result between runs
+    foreach my $iid ( sort keys %$e_parent ) {
+        my $parent = $e_parent->{$iid};
+        my $class = $e_class->{$iid} || '';
+        # Only consider serial numbers for entries without a parent, or
+        # if they are of type "chassis"
+        if ( $parent eq '0' or $class eq 'chassis') {
+            my $os_ver = $entity->e_swver($iid);
+            if ( $os_ver && $os_ver->{$iid} ) {
+                return $os_ver->{$iid};
+            }
+        }
+    }
+    return;
+}
+
 1;
 
 __END__
@@ -173,6 +224,23 @@ none.
 These are methods that return tables of information in the form of a reference
 to a hash.
 
+=over
+
+=item $entity->entity_derived_serial()
+
+Tries to determine the device serial number from the F<ENTITY-MIB>. Only
+considers serial numbers for entries without a parent, or if they are of type
+chassis. Looks at C<entPhysicalSerialNum> and then C<entPhysicalDescr> for
+serial number.
+
+=item $entity->entity_derived_os_ver()
+
+Tries to determine the device OS version from the F<ENTITY-MIB>. Only
+considers serial numbers for entries without a parent, or if they are of type
+chassis. Looks at C<entPhysicalSoftwareRev> for the version.
+
+=back
+
 =head2 Entity Table
 
 =over

lib/SNMP/Info/Layer2.pm

@@ -118,24 +118,18 @@ sub vendor {
 sub serial {
     my $l2 = shift;
 
-    my $serial1  = $l2->serial1();
+    my $entity_serial = $l2->entity_derived_serial();
-    my $e_descr  = $l2->e_descr() || {};
+    if ( defined $entity_serial and $entity_serial !~ /^\s*$/ ){
-    my $e_serial = $l2->e_serial() || {};
+        return $entity_serial;
-
-    my $serial2 = $e_serial->{1} || undef;
-    my $chassis = $e_descr->{1}  || undef;
-
-    # precedence
-    #   serial2,chassis parse,serial1
-    return $serial2 if ( defined $serial2 and $serial2 !~ /^\s*$/ );
-
-    if ( defined $chassis and $chassis =~ /serial#?:\s*([a-z0-9]+)/i ) {
-        return $1;
     }
 
-    return $serial1 if ( defined $serial1 and $serial1 !~ /^\s*$/ );
+    my $serial1  = $l2->serial1();
+    if ( defined $serial1 and $serial1 !~ /^\s*$/ ) {
+        return $serial1;
+    }
 
     return;
+
 }
 
 sub interfaces {
@@ -262,7 +256,7 @@ Tries to discover the vendor from $l2->model() and $l2->description()
 
 =item $l2->serial()
 
-Returns serial number if available through SNMP
+Returns a serial number if found from F<ENTITY-MIB> and F<OLD-CISCO->... MIB.
 
 =back
 

lib/SNMP/Info/Layer3.pm

@@ -194,32 +194,18 @@ sub root_ip {
 sub serial {
     my $l3 = shift;
 
+    my $entity_serial = $l3->entity_derived_serial();
+    if ( defined $entity_serial and $entity_serial !~ /^\s*$/ ){
+        return $entity_serial;
+    }
+
     my $serial1 = $l3->serial1();
-    my $e_parent = $l3->e_parent() || {};
+    if ( defined $serial1 and $serial1 !~ /^\s*$/ ) {
-    my $e_class  = $l3->e_class() || {};
+        return $serial1;
-
-    foreach my $iid ( keys %$e_parent ) {
-        my $parent = $e_parent->{$iid};
-        my $class = $e_class->{$iid} || '';
-        # Only consider serial numbers for entries without a parent, or if they are of type "chassis"
-        if ( $parent eq '0' or $class eq 'chassis') {
-            my $serial = $l3->e_serial($iid);
-            if ( $serial && $serial->{$iid} ) {
-                return $serial->{$iid};
     }
-            else {
-                my $descr = $l3->e_descr($iid);
-                if ( $descr and $descr =~ /serial#?:\s*([a-z0-9]+)/i )
-                {
-                    return $1;
-                }
-            }
-        }
-    }
-
-    return $serial1 if ( defined $serial1 and $serial1 !~ /^\s*$/ );
 
     return;
+
 }
 
 # $l3->model() - the sysObjectID returns an IID to an entry in
@@ -493,8 +479,7 @@ Removes 'cisco'  from cisco devices for readability.
 
 =item $l3->serial()
 
-Tries to cull a serial number from F<ENTITY-MIB>, description, and
+Returns a serial number if found from F<ENTITY-MIB> and F<OLD-CISCO->... MIB.
-F<OLD-CISCO->... MIB.
 
 =item $l3->vendor()
 

xt/lib/Test/SNMP/Info/Entity.pm

@@ -33,23 +33,131 @@ use Test::Class::Most parent => 'My::Test::Class';
 
 use SNMP::Info::Entity;
 
-# Remove this startup override once we have full method coverage
-sub startup : Tests(startup => 1) {
-  my $test = shift;
-  $test->SUPER::startup();
-
-  $test->todo_methods(1);
-}
-
 sub setup : Tests(setup) {
   my $test = shift;
   $test->SUPER::setup;
 
   # Start with a common cache that will serve most tests
   my $cache_data = {
-    'store' => {},
+    '_entPhysicalDescr' => 1,
+    '_e_map'            => 1,
+    '_e_parent'         => 1,
+    '_e_class'          => 1,
+    'store'             => {
+      'entPhysicalDescr' => {1 => 1, 2 => 2, 3 => 3, 54 => 54, 55 => 55},
+      'e_map'            => {
+        '1019.0' => 'ifIndex.1',
+        '1031.0' => 'ifIndex.2',
+        '2019.0' => 'ifIndex.3',
+        '2031.0' => 'ifIndex.4'
+      },
+      'e_parent' => {1 => 0, 2 => 1, 3 => 2, 54 => 1, 55 => 54},
+      'e_class'  => {
+        1  => 'stack',
+        2  => 'chassis',
+        3  => 'module',
+        54 => 'chassis',
+        55 => 'module'
+      },
+    },
   };
   $test->{info}->cache($cache_data);
 }
 
+sub e_index : Tests(3) {
+  my $test = shift;
+
+  can_ok($test->{info}, 'e_index');
+
+  my $expected = {1 => 1, 2 => 2, 3 => 3, 54 => 54, 55 => 55};
+
+  cmp_deeply($test->{info}->e_index(),
+    $expected, q(Entity Physical Index using 'entPhysicalDescr'));
+
+  $test->{info}->clear_cache();
+  is($test->{info}->e_index(), undef, q(No data returns undef));
+}
+
+sub e_port : Tests(3) {
+  my $test = shift;
+
+  can_ok($test->{info}, 'e_port');
+
+  my $expected = {1019 => 1, 1031 => 2, 2019 => 3, 2031 => 4};
+
+  cmp_deeply($test->{info}->e_port(),
+    $expected, q(Entity cross reference to 'ifIndex'));
+
+  $test->{info}->clear_cache();
+  cmp_deeply($test->{info}->e_port(), {}, q(No data returns empty hash));
+}
+
+sub entity_derived_serial : Tests(4) {
+  my $test = shift;
+
+  can_ok($test->{info}, 'entity_derived_serial');
+
+  # The call to SUPER::serial() will use the entity_derived_serial method
+  # which uses a partial fetch for e_serial which ignores the cache
+  # and reloads data therefore we must use the mocked session.
+  my $data = {
+    'ENTITY-MIB::entPhysicalSerialNum' => {
+      1  => undef,
+      2  => 'AB01CDE2345678901234F00',
+      3  => undef,
+      54 => 'AB01CDE2345678901234F01',
+      55 => undef
+    },
+  };
+  $test->{info}{sess}{Data} = $data;
+
+  is($test->{info}->entity_derived_serial(),
+    'AB01CDE2345678901234F00',
+    q(Serial has expected value using 'entPhysicalSerialNum'));
+
+  # Clear previous mock session data from cache and data
+  delete $test->{info}{_e_serial};
+  delete $test->{info}{store}{e_serial};
+  $test->{info}{sess}{Data} = {};
+
+  # New mock data for partial call
+  $data = {
+    'ENTITY-MIB::entPhysicalDescr' => {
+      1  => undef,
+      2  => 'AS5350 chassis, Hw Serial#: 12345, Hw Revision: T',
+      3  => undef,
+      54 => 'AS5350 Cpu Card,  Hw Serial#: 23456, Hw Revision: T',
+      55 => undef
+    },
+  };
+  $test->{info}{sess}{Data} = $data;
+  is($test->{info}->entity_derived_serial(),
+    '12345', q(Serial has expected value using 'entPhysicalDescr'));
+
+  $test->{info}->clear_cache();
+  is($test->{info}->entity_derived_serial(),
+    undef, q(No data returns undef serial));
+}
+
+sub entity_derived_os_ver : Tests(3) {
+  my $test = shift;
+
+  can_ok($test->{info}, 'entity_derived_os_ver');
+
+  # The call to SUPER::serial() will use the entity_derived_serial method
+  # which uses a partial fetch for e_serial which ignores the cache
+  # and reloads data therefore we must use the mocked session.
+  my $data
+    = {'ENTITY-MIB::entPhysicalSoftwareRev' =>
+      {1 => undef, 2 => '5.1.2.3', 3 => undef, 54 => '6.1.2.3', 55 => undef},
+    };
+  $test->{info}{sess}{Data} = $data;
+
+  is($test->{info}->entity_derived_os_ver(),
+    '5.1.2.3', q(OS version has expected value using 'entPhysicalSoftwareRev'));
+
+  $test->{info}->clear_cache();
+  is($test->{info}->entity_derived_os_ver(), undef, q(No data returns undef));
+}
+
 1;