From 6bd6c9de8c9b259a68947482a3cd3baa093ed08b Mon Sep 17 00:00:00 2001
From: Oliver Gorwits <oliver@cpan.org>
Date: Fri, 27 Dec 2013 16:25:41 +0000
Subject: [PATCH] Keep a hint to SNMP community if new style snmp_auth config
 is in use

---
 Netdisco/Changes                              |  4 +-
 .../lib/App/Netdisco/DB/ResultSet/Device.pm   | 10 +++-
 Netdisco/lib/App/Netdisco/Util/SNMP.pm        | 46 ++++++++-----------
 3 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/Netdisco/Changes b/Netdisco/Changes
index 4d412927..e098c4c8 100644
--- a/Netdisco/Changes
+++ b/Netdisco/Changes
@@ -9,7 +9,8 @@
   * Require SNMP::Info 3.10+, use new c_cap method to tag device remote_type
     as 'IP Phone:' or 'AP:' when those capabilities are advertised by LLDP
     or CDP
-  * Port MAC query now only returns distinct MAC's   
+  * Port MAC query now only returns distinct MAC's
+  * Keep a hint to SNMP community if new style snmp_auth config is in use
 
   [BUG FIXES]
 
@@ -17,6 +18,7 @@
     entries (Robert Kerr)
   * [#60] Incorrect format string in store_vlans (Robert Kerr)
   * Fix form reset icon on ports tab when using custom path (Daniel Tuecks)
+  * Don't store failed SNMP community in the database
 
 2.020002 - 2013-12-11
 
diff --git a/Netdisco/lib/App/Netdisco/DB/ResultSet/Device.pm b/Netdisco/lib/App/Netdisco/DB/ResultSet/Device.pm
index 9ae69e08..eabaad1c 100644
--- a/Netdisco/lib/App/Netdisco/DB/ResultSet/Device.pm
+++ b/Netdisco/lib/App/Netdisco/DB/ResultSet/Device.pm
@@ -544,7 +544,6 @@ sub delete {
   my $devices = $self->search(undef, { columns => 'ip' });
 
   foreach my $set (qw/
-    Community
     DeviceIp
     DeviceVlan
     DevicePower
@@ -555,6 +554,15 @@ sub delete {
       )->delete;
   }
 
+  $schema->resultset('Community')->search({
+    ip => { '-in' => $devices->as_query },
+    snmp_auth_tag => undef,
+  })->delete;
+
+  $schema->resultset('Community')->search(
+    { ip => { '-in' => $devices->as_query } },
+  )->update({snmp_comm_rw => undef});
+
   $schema->resultset('Admin')->search({
     device => { '-in' => $devices->as_query },
     action => { '-like' => 'queued%' },
diff --git a/Netdisco/lib/App/Netdisco/Util/SNMP.pm b/Netdisco/lib/App/Netdisco/Util/SNMP.pm
index bab60eff..99d9c541 100644
--- a/Netdisco/lib/App/Netdisco/Util/SNMP.pm
+++ b/Netdisco/lib/App/Netdisco/Util/SNMP.pm
@@ -162,47 +162,38 @@ sub _try_connect {
 sub _try_read {
   my ($info, $device, $comm) = @_;
 
-  undef $info unless (
+  return undef unless (
     (not defined $info->error)
     and defined $info->uptime
     and ($info->layers or $info->description)
     and $info->class
   );
 
-  if ($device->in_storage) {
-      # read strings are tried before writes, so this should not accidentally
-      # store a write string if there's a good read string also in config.
-      $device->update({snmp_comm => $comm->{community}})
-        if exists $comm->{community};
-      $device->update_or_create_related('community',
-        {snmp_auth_tag => $comm->{tag}}) if $comm->{tag};
-  }
-  else {
-      $device->set_column(snmp_comm => $comm->{community})
-        if exists $comm->{community};
+  if ($comm->{community}) {
+      $device->in_storage
+        ? $device->update({snmp_comm => $comm->{community}})
+        : $device->set_column(snmp_comm => $comm->{community});
   }
 
+  # regardless of device in storage, save the hint
+  $device->update_or_create_related('community',
+    {snmp_auth_tag => $comm->{tag}}) if $comm->{tag};
+
   return $info;
 }
 
 sub _try_write {
   my ($info, $device, $comm) = @_;
 
-  # SNMP v1/2 R/W must be able to read as well (?)
-  $info = _try_read($info, $device, $comm)
-    if exists $comm->{community};
-  return unless $info;
+  my $loc = $info->load_location;
+  $info->set_location($loc) or return undef;
+  return undef unless ($loc eq $info->load_location);
 
-  $info->set_location( $info->load_location )
-    or return undef;
-
-  if ($device->in_storage) {
-      # one of these two cols must be set
-      $device->update_or_create_related('community', {
-        ($comm->{tag} ? (snmp_auth_tag => $comm->{tag}) : ()),
-        (exists $comm->{community} ? (snmp_comm_rw => $comm->{community}) : ()),
-      });
-  }
+  # one of these two cols must be set
+  $device->update_or_create_related('community', {
+    ($comm->{tag} ? (snmp_auth_tag => $comm->{tag}) : ()),
+    ($comm->{community} ? (snmp_comm_rw => $comm->{community}) : ()),
+  });
 
   return $info;
 }
@@ -255,7 +246,7 @@ sub _build_communities {
     if defined $device->snmp_comm and $mode eq 'read';
 
   # first try last-known-good
-  push @communities, {read => 1, write => 1, community => $snmp_comm_rw}
+  push @communities, {write => 1, community => $snmp_comm_rw}
     if $snmp_comm_rw and $mode eq 'write';
 
   # new style snmp config
@@ -303,7 +294,6 @@ sub _build_communities {
   }
   else {
       push @communities, map {{
-        read  => 1,
         write => 1,
         community => $_,
       }} @{setting('community_rw') || []};