Avoid macsuck generated SQL bug when cleaning NULL VLAN

Netdisco/Changes

@@ -9,6 +9,7 @@
 
   * Update NodeWireless entries which match both MAC and SSID found, only
   * Fix SSL-proxy behaviour by using only path+query in links (W. Gould)
+  * Avoid macsuck generated SQL bug when cleaning NULL VLAN (W. Gould)
 
 2.017000 - 2013-09-23
 

Netdisco/lib/App/Netdisco/Core/Macsuck.pm

@@ -130,53 +130,48 @@ sub store_node {
     my $nodes = schema('netdisco')->resultset('Node');
 
     # TODO: probably needs changing if we're to support VTP domains
-    my $old = $nodes->search(
-      {
-        mac => $mac,
-        vlan => $vlan,
-        -bool => 'active',
-        -not => {
-          switch => $ip,
-          port => $port,
-        },
-      });
+    my $old = $nodes->search({
+      mac => $mac,
+      vlan => $vlan,
+      -bool => 'active',
+      -not => {
+        switch => $ip,
+        port => $port,
+      },
+    });
 
     # lock rows,
     # and get the count so we know whether to set time_recent
     my $old_count = scalar $old->search(undef,
       {
         columns => [qw/switch vlan port mac/],
-        order_by => [qw/switch vlan port mac/],
         for => 'update',
       })->all;
 
     $old->update({ active => \'false' });
 
-    my $new = $nodes->search(
-      {
-        'me.switch' => $ip,
-        'me.port' => $port,
-        'me.mac' => $mac,
-      },
-      {
-        order_by => [qw/switch vlan port mac/],
-        for => 'update',
-      });
-
-    # lock rows
-    $new->search({vlan => [$vlan, 0, undef]})->first;
-
-    # upgrade old schema
-    $new->search({vlan => [0, undef]})->delete();
+    my $new = $nodes->search({
+      'me.switch' => $ip,
+      'me.port' => $port,
+      'me.mac' => $mac,
+    });
 
     # new data
-    $new->update_or_create({
-      vlan => $vlan,
-      active => \'true',
-      oui => substr($mac,0,8),
-      time_last => \$now,
-      ($old_count ? (time_recent => \$now) : ()),
-    });
+    $new->update_or_create(
+      {
+        vlan => $vlan,
+        active => \'true',
+        oui => substr($mac,0,8),
+        time_last => \$now,
+        ($old_count ? (time_recent => \$now) : ()),
+      },
+      { for => 'update' }
+    );
+
+    # upgrade old schema
+    # an entry for this MAC existed in old schema with vlan => null
+    # which now has either vlan number or 0
+    $new->search({vlan => undef})->delete({only_nodes => 1});
   });
 }
 

Netdisco/lib/App/Netdisco/DB/ResultSet/Node.pm

@@ -100,6 +100,10 @@ sub delete {
       # avoid letting DBIC delete nodes
       return 0E0;
   }
+  elsif (exists $opts->{only_nodes} and $opts->{only_nodes}) {
+      # now let DBIC do its thing
+      return $self->next::method();
+  }
   elsif (exists $opts->{keep_nodes} and $opts->{keep_nodes}) {
       # avoid letting DBIC delete nodes
       return 0E0;