From ea9ad92cc477b2c57174f75205512fef5e248cb8 Mon Sep 17 00:00:00 2001
From: Oliver Gorwits <oliver@cpan.org>
Date: Sat, 20 Apr 2019 09:07:14 +0100
Subject: [PATCH] #320 improve duplicate interfaces() fixup

1. sorts the interfaces as they are processed to make A/B testing easier
2. adds the interface index to the original when a duplicate is seen
3. there may be other de-duplication code in vendor modules, untouched
---
 lib/SNMP/Info/Layer2.pm         | 10 ++++++++--
 lib/SNMP/Info/Layer3.pm         | 10 ++++++++--
 xt/lib/Test/SNMP/Info/Layer2.pm | 26 +++++++++++++++++++++++++-
 xt/lib/Test/SNMP/Info/Layer3.pm | 26 +++++++++++++++++++++++++-
 4 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/lib/SNMP/Info/Layer2.pm b/lib/SNMP/Info/Layer2.pm
index 6c266331..917be9ac 100644
--- a/lib/SNMP/Info/Layer2.pm
+++ b/lib/SNMP/Info/Layer2.pm
@@ -145,15 +145,21 @@ sub interfaces {
     # Replace the Index with the ifDescr field.
     # Check for duplicates in ifDescr, if so uniquely identify by adding
     # ifIndex to repeated values
-    my %seen;
-    foreach my $iid ( keys %$i_descr ) {
+    my (%seen, %first_seen_as);
+    foreach my $iid ( sort keys %$i_descr ) {
         my $port = $i_descr->{$iid};
         next unless defined $port;
+
         if ( $seen{$port}++ ) {
+            # (#320) also fixup the port this is a duplicate of
+            $interfaces->{ $first_seen_as{$port} }
+              = sprintf( "%s (%d)", $port, $first_seen_as{$port} );
+
             $interfaces->{$iid} = sprintf( "%s (%d)", $port, $iid );
         }
         else {
             $interfaces->{$iid} = $port;
+            $first_seen_as{$port} = $iid;
         }
     }
     return $interfaces;
diff --git a/lib/SNMP/Info/Layer3.pm b/lib/SNMP/Info/Layer3.pm
index 4182ef12..660928aa 100644
--- a/lib/SNMP/Info/Layer3.pm
+++ b/lib/SNMP/Info/Layer3.pm
@@ -330,15 +330,21 @@ sub interfaces {
 
     # Check for duplicates in ifDescr, if so uniquely identify by adding
     # ifIndex to repeated values
-    my %seen;
-    foreach my $iid ( keys %$i_descr ) {
+    my (%seen, %first_seen_as);
+    foreach my $iid ( sort keys %$i_descr ) {
         my $port = $i_descr->{$iid};
         next unless defined $port;
+
         if ( $seen{$port}++ ) {
+            # (#320) also fixup the port this is a duplicate of
+            $interfaces->{ $first_seen_as{$port} }
+              = sprintf( "%s (%d)", $port, $first_seen_as{$port} );
+
             $interfaces->{$iid} = sprintf( "%s (%d)", $port, $iid );
         }
         else {
             $interfaces->{$iid} = $port;
+            $first_seen_as{$port} = $iid;
         }
     }
     return $interfaces;
diff --git a/xt/lib/Test/SNMP/Info/Layer2.pm b/xt/lib/Test/SNMP/Info/Layer2.pm
index 7e6bf59d..8d54e922 100644
--- a/xt/lib/Test/SNMP/Info/Layer2.pm
+++ b/xt/lib/Test/SNMP/Info/Layer2.pm
@@ -47,9 +47,33 @@ sub setup : Tests(setup) {
 
   # Start with a common cache that will serve most tests
   my $cache_data = {
-    'store' => {},
+    '_i_index' => 1,
+    '_i_description' => 1,
+    'store' => {
+      'i_index' => {1 => 1, 2 => 2, 3 => 3, 4 => 4},
+      'i_description' => {
+        1 => 'Unique Interface Name',
+        2 => 'Duplicate Interface Name',
+        3 => 'Duplicate Interface Name'
+      },
+    },
   };
   $test->{info}->cache($cache_data);
 }
 
+sub duplicates : Tests(2) {
+  my $test = shift;
+
+  my $expected_data = {
+    1 => 'Unique Interface Name',
+    2 => 'Duplicate Interface Name (2)',
+    3 => 'Duplicate Interface Name (3)',
+    4 => 4,
+  };
+
+  can_ok($test->{info}, 'interfaces');
+  cmp_deeply($test->{info}->interfaces(),
+    $expected_data, 'Call to interfaces() removes duplicates');
+}
+
 1;
diff --git a/xt/lib/Test/SNMP/Info/Layer3.pm b/xt/lib/Test/SNMP/Info/Layer3.pm
index 945c267b..9b77f74c 100644
--- a/xt/lib/Test/SNMP/Info/Layer3.pm
+++ b/xt/lib/Test/SNMP/Info/Layer3.pm
@@ -47,9 +47,33 @@ sub setup : Tests(setup) {
 
   # Start with a common cache that will serve most tests
   my $cache_data = {
-    'store' => {},
+    '_i_index' => 1,
+    '_i_description' => 1,
+    'store' => {
+      'i_index' => {1 => 1, 2 => 2, 3 => 3, 4 => 4},
+      'i_description' => {
+        1 => 'Unique Interface Name',
+        2 => 'Duplicate Interface Name',
+        3 => 'Duplicate Interface Name'
+      },
+    },
   };
   $test->{info}->cache($cache_data);
 }
 
+sub duplicates : Tests(2) {
+  my $test = shift;
+
+  my $expected_data = {
+    1 => 'Unique Interface Name',
+    2 => 'Duplicate Interface Name (2)',
+    3 => 'Duplicate Interface Name (3)',
+    4 => 4,
+  };
+
+  can_ok($test->{info}, 'interfaces');
+  cmp_deeply($test->{info}->interfaces(),
+    $expected_data, 'Call to interfaces() removes duplicates');
+}
+
 1;