Solidify caption/holding relationship, improve MFHD::Holding comparisons

[This commit has been squashed for merging. LFW]

  * This commit:
    - Makes sure that holding data is valid for the given caption
      for new holding objects
    - Teaches field_values() to fall back to '*' (unknown marker)
      when a holding is missing data
    - Allows the caption() method to be a setter

  * This commit:
    - Makes the comparison operator consider chron data, not just
      enumeration data
    - Teaches the comparison operator a way to handle 'unsure' data
      (that is, data presented in brackets [])

  * The code was assuming the $end_holding param would be uncompressed,
    but this was not stated anywhere, nor enforced.  Let's allow the
    method to take both compressed and uncompressed holdings as the "end"
    (and handle it appropriately).

  * Add some holdings with missing and unsure data to test the new
    comparison operators handling of such data.

Signed-off-by: Dan Wells <dbw2@calvin.edu>
Signed-off-by: Lebbeous Fogle-Weekley <lebbeous@esilibrary.com>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD/Holding.pm b/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD/Holding.pm
index 920bfd2..37f985a 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD/Holding.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD/Holding.pm
@@ -42,6 +42,9 @@ sub new {
             if (exists($self->{_mfhdh_FIELDS}->{$key})) {
                 carp("Duplicate, non-repeatable subfield '$key' found, ignoring");
                 next;
+            } elsif (!$caption->capfield($key)) {
+                carp("Subfield '$key' has no corresponding caption, ignoring");
+                next;
             }
             if ($self->{_mfhdh_COMPRESSED}) {
                 $self->{_mfhdh_FIELDS}->{$key}{HOLDINGS} = [split(/\-/, $val, -1)];
@@ -116,8 +119,15 @@ sub field_values {
     if (exists $self->fields->{$key}) {
         my @values = @{$self->fields->{$key}{HOLDINGS}};
         return \@values;
+    } elsif ($self->caption->capfield($key)) {
+        carp("No values found for existing caption subfield '$key', returning '*' (unknown value indicator)");
+        if ($self->is_compressed) {
+            return ['*', '*'];
+        } else {
+            return ['*'];
+        }
     } else {
-        return undef;
+        return;
     }
 }
 
@@ -161,6 +171,11 @@ sub is_open_ended {
 
 sub caption {
     my $self = shift;
+    my $caption = shift;
+
+    if ($caption) {
+        $self->{_mfhdh_CAPTION} = $caption;
+    }
 
     return $self->{_mfhdh_CAPTION};
 }
@@ -625,12 +640,18 @@ sub compressed_to_last {
 #
 # Creates or replaces an end of a compressed holding
 #
+# If $end_holding does not share caption data with $self, results
+# will be unpredicable
+#
 sub compressed_end {
     my $self = shift;
     my $end_holding = shift;
 
     my %changes;
-    if ($end_holding) {
+    if ($end_holding and !$end_holding->is_open_ended) {
+        if ($end_holding->is_compressed) {
+            $end_holding = $end_holding->clone->compressed_to_last;
+        }
         foreach my $key (keys %{$self->fields}) {
             my @values = @{$self->field_values($key)};
             my @end_values = @{$end_holding->field_values($key)};
@@ -638,7 +659,7 @@ sub compressed_end {
             $self->fields->{$key}{HOLDINGS} = \@values;
             $changes{$key} = join('-', @values);
         }
-    } elsif (!$self->is_open_ended) { # make open-ended if no $end_holding
+    } elsif (!$self->is_open_ended) { # make open-ended if no $end_holding (or $end_holding was open ended)
         foreach my $key (keys %{$self->fields}) {
             my @values = @{$self->field_values($key)};
             $self->fields->{$key}{HOLDINGS} = [$values[0]];
@@ -883,12 +904,19 @@ sub _compare {
 
     # start doing the actual comparison
     my $result;
-    foreach my $key ('a'..'f') {
+    foreach my $key ('a'..'f', 'i'..'m') {
         if (defined($holding_1->field_values($key))) {
             if (!defined($holding_2->field_values($key))) {
                 return 1; # more details equals 'greater' (?)
             } else {
-                $result = $holding_1->field_values($key)->[0] <=> $holding_2->field_values($key)->[0];
+                my $holding_1_value = $holding_1->field_values($key)->[0];
+                my $holding_1_unsure = ($holding_1_value =~ s/\[|\]//g);
+                my $holding_2_value = $holding_2->field_values($key)->[0];
+                my $holding_2_unsure = ($holding_2_value =~ s/\[|\]//g);
+                $result = $holding_1_value <=> $holding_2_value;
+                if (!$result) { # they are 'equal' but we will sort 'maybe' values before 'sure' values (TODO: rethink this is it complicates some algorithms)
+                    $result = $holding_2_unsure <=> $holding_1_unsure;
+                }
             }
         } elsif (defined($holding_2->field_values($key))) {
             return -1; # more details equals 'greater' (?)

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD/test/mfhddata2.txt b/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD/test/mfhddata2.txt
index 65a6fe8..41560c2 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD/test/mfhddata2.txt
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD/test/mfhddata2.txt
@@ -50,25 +50,29 @@
 853 20 $81$av.$bno.$u12$vr$i(year)$j(month)$wm$x01
 863 40 $81.1$a1$b4-7$i1990$j04-07
 863 40 $81.2$a1$b4-7$i1990$j04-07
-863 41 $81.3$a1$b4$i1990$j04
+863 41 $81.3$a1$b4$i1990$j[04]
 863 41 $81.4$a1$b4$i1990$j04
-863 40 $81.5$a1-$b3-$i1990-$j03-
-863 40 $81.6$a1$b3-6$i1990$j03-06
-863 40 $81.7$a1$b3-5$i1990$j03-05
-863 41 $81.8$a1$b3$i1990$j03
-863 41 $81.9$a1$b2$i1990$j02
+863 41 $81.5$a1$b4$i1990$j[04]
+863 40 $81.6$a1-$b3-$i1990-$j03-
+863 40 $81.7$a1$b3-6$i1990$j03-06
+863 40 $81.8$a1$b3-5$i1990$j03-05
+863 41 $81.9$a1$b3$i1990$j03
+863 41 $81.10$a1$b2$i1990$j02
+863 41 $81.11$a1$b2$i1990
 
 245 00 $aComparison test, sorted
 853 20 $81$av.$bno.$u12$vr$i(year)$j(month)$wm$x01
-863 41 $81.1$a1$b2$i1990$j02
-863 41 $81.2$a1$b3$i1990$j03
-863 40 $81.3$a1$b3-5$i1990$j03-05
-863 40 $81.4$a1$b3-6$i1990$j03-06
-863 40 $81.5$a1-$b3-$i1990-$j03-
-863 41 $81.6$a1$b4$i1990$j04
-863 41 $81.7$a1$b4$i1990$j04
-863 40 $81.8$a1$b4-7$i1990$j04-07
-863 40 $81.9$a1$b4-7$i1990$j04-07
+863 41 $81.1$a1$b2$i1990$j*
+863 41 $81.2$a1$b2$i1990$j02
+863 41 $81.3$a1$b3$i1990$j03
+863 40 $81.4$a1$b3-5$i1990$j03-05
+863 40 $81.5$a1$b3-6$i1990$j03-06
+863 40 $81.6$a1-$b3-$i1990-$j03-
+863 41 $81.7$a1$b4$i1990$j[04]
+863 41 $81.8$a1$b4$i1990$j[04]
+863 41 $81.9$a1$b4$i1990$j04
+863 40 $81.10$a1$b4-7$i1990$j04-07
+863 40 $81.11$a1$b4-7$i1990$j04-07
 
 245 00 $aGet combined holdings
 853 20 $81$av.$bno.$u12$vr$i(year)$j(month)$wm$x01