Index: main/trunk/greenstone2/perllib/cgiactions/metadataaction.pm
===================================================================
--- main/trunk/greenstone2/perllib/cgiactions/metadataaction.pm	(revision 31588)
+++ main/trunk/greenstone2/perllib/cgiactions/metadataaction.pm	(revision 31589)
@@ -127,10 +127,11 @@
 	#REMOVE METHODS
 	"remove-import-metadata" => { 
-		'compulsory-args' => [ "d", "metaname", "metavalue" ], #TODO: add f argument
-		'optional-args'   => [ "metapos" ] }, # only provide metapos arg for SIMPLE collections
+		'compulsory-args' => [ "d", "metaname" ], #TODO: add f argument
+		'optional-args'   => [ "metapos", "metavalue", "metamode" ] }, # only provide metapos arg for SIMPLE collections.
+# Metavalue is now an optional arg for remove_import_metadata() based on what the implementation did, which allowed metavalue to be undefined, and if so, used metapos.
 					 
 	"remove-archives-metadata" => { 
 		'compulsory-args' => [ "d", "metaname" ], #TODO: add f argument
-		'optional-args'   => [ "metapos", "metavalue" ] },
+		'optional-args'   => [ "metapos", "metavalue", "metamode" ] },
 
 	"remove-live-metadata" => {
@@ -144,5 +145,5 @@
 	"remove-metadata" => { # generic remove-meta function. The 'where' param can be set to any combination of index|archives|import|live. docid d is still compulsory for setting index, archives and live meta
 		'compulsory-args' => [ "d", "metaname" ],
-		'optional-args'   => [ "where", "metapos", "metavalue" ] },
+		'optional-args'   => [ "where", "metapos", "metavalue", "metamode" ] }, # metamode is optional since remove-metadata can call any of remove_import_meta and remove_archives_meta, remove_index_meta, of which the first two accept metamode as an optional param
 
 	#INSERT METHODS
@@ -263,5 +264,8 @@
 
     my $where = $self->{'where'};
-    if (!$where) {	
+    if (!$where || ($where =~ m/^\s*$/)) { # 0, "0", "" and undef are all false. All else is true.
+	# What is truth in perl: http://www.berkeleyinternet.com/perl/node11.html
+	# and http://www.perlmonks.org/?node_id=33638
+
 	$where = "index"; # default behaviour is to get the values from index
     }
@@ -757,5 +761,5 @@
 
     # Obtain the specified metadata value
-    $metapos = 0 if (!defined $metapos);
+    $metapos = 0 if (!defined $metapos || ($metapos =~ m/^\s*$/));
     my $metavalue = $doc_rec->{$metaname}->[$metapos];
     $gsdl_cgi->generate_ok_message("$metavalue");
@@ -782,5 +786,5 @@
 	my $metaname  = $self->{'metaname'};
 	my $metapos = $self->{'metapos'};
-	$metapos = 0 if (!defined $metapos); # gets the first value by default since metapos defaults to 0
+	$metapos = 0 if (!defined $metapos || ($metapos =~ m/^\s*$/)); # gets the first value by default since metapos defaults to 0
 
 	my $infodbtype = $self->{'infodbtype'};
@@ -910,5 +914,5 @@
 	my $metaname = $self->{'metaname'};
 	my $metapos = $self->{'metapos'};
-	$metapos = 0 if (!defined $metapos);
+	$metapos = 0 if (!defined $metapos || ($metapos =~ m/^\s*$/));
 	
 	my $arcinfo_doc_filename = &dbutil::get_infodb_file_path($infodbtype, "archiveinf-doc", $archive_dir);
@@ -1170,6 +1174,6 @@
 
     my $import_file  = $self->{'f'};
-    if ((!defined $docid) && (!defined $import_file)) {
-	$gsdl_cgi->generate_error("No docid (d=...) or import file (f=) specified.");
+    if ((!defined $docid || $docid =~ m/^\s*$/) && (!defined $import_file || $import_file =~ m/^\s*$/)) {
+	$gsdl_cgi->generate_error("No docid (d=...) or import file (f=) specified."); # at least d or f must be specified
     } 
 
@@ -1182,5 +1186,5 @@
     my $metamode   = $self->{'metamode'};
     if ((!defined $metamode) || ($metamode =~ m/^\s*$/)) {
-	# make "accumulate" the default (less destructive, as won't actually 
+	# make "accumulate" the default (less destructive, as it won't actually 
 	# delete any existing values)
 	$metamode = "accumulate";
@@ -1190,4 +1194,6 @@
     my $metapos   = $self->{'metapos'}; # don't force undef to 0. Undef has meaning when metamode=override
     my $prevmetavalue = $self->{'prevmetavalue'};
+    $metapos = undef if(defined $metapos && ($metapos =~ m/^\s*$/));
+    $prevmetavalue = undef if(defined $prevmetavalue && ($prevmetavalue =~ m/^\s*$/));
 
     my $arcinfo_doc_filename = &dbutil::get_infodb_file_path($infodbtype, "archiveinf-doc", $archive_dir);
@@ -1227,7 +1233,10 @@
                                   # Don't append "|| undef", since if metapos=0 it will then be set to undef
 
+    $metapos = undef if(defined $metapos && ($metapos =~ m/^\s*$/));
+    $prevmetavalue = undef if(defined $prevmetavalue && ($prevmetavalue =~ m/^\s*$/));
+
     my $metamode   = $self->{'metamode'};
     if ((!defined $metamode) || ($metamode =~ m/^\s*$/)) {
-	# make "accumulate" the default (less destructive, as won't actually 
+	# make "accumulate" the default (less destructive, as it won't actually 
 	# delete any existing values)
 	$metamode = "accumulate";
@@ -1284,6 +1293,9 @@
     my $prevmetavalue = $self->{'prevmetavalue'};
 
+    $metapos = undef if(defined $metapos && ($metapos =~ m/^\s*$/));
+    $prevmetavalue = undef if(defined $prevmetavalue && ($prevmetavalue =~ m/^\s*$/));
+
     print STDERR "SETTING INDEX METADATA ENTRY\n";
-	my $status = $self->set_index_metadata_entry($collect_dir,$collect,$infodbtype,$docid,$metaname,$metapos,$metavalue,$metamode,$prevmetavalue);
+    my $status = $self->set_index_metadata_entry($collect_dir,$collect,$infodbtype,$docid,$metaname,$metapos,$metavalue,$metamode,$prevmetavalue);
     print STDERR "DONE SETTING INDEX METADATA ENTRY\n";
     if ($status != 0) {
@@ -1351,5 +1363,5 @@
 
     my $where = $self->{'where'};
-    if(!$where) {	
+    if(!$where || ($where =~ m/^\s*$/)) {	
 	$self->set_index_metadata(@_); # call the full version of set_index_meta for the default behaviour
 	return;
@@ -1424,5 +1436,5 @@
 
     my $where = $self->{'where'};
-    if(!$where) {	
+    if(!$where || ($where =~ m/^\s*$/)) {	
 	$self->set_index_metadata_array(@_); # default behaviour is the full version of set_index_meta_array
 	return;
@@ -2206,6 +2218,8 @@
 	my $metaname = $self->{'metaname'};
 	my $metapos = $self->{'metapos'};
-
-	my $metavalue = $self->{'metavalue'} || undef; # necessary to force fallback to undef here
+	my $metavalue = $self->{'metavalue'};
+
+	$metapos = undef if(defined $metapos && ($metapos =~ m/^\s*$/));
+	$metavalue = undef if(defined $metavalue && ($metavalue =~ m/^\s*$/)); # necessary to force fallback to undef here
 
 	# if the user hasn't told us what to delete, not having given a metavalue or metapos,
@@ -2219,5 +2233,6 @@
 	}
 
-	my $metamode = $self->{'metamode'} || undef;
+	my $metamode = $self->{'metamode'};
+	$metamode = undef if(defined $metamode && ($metamode =~ m/^\s*$/));
 
 	my $arcinfo_doc_filename = &dbutil::get_infodb_file_path($infodbtype, "archiveinf-doc", $archive_dir);
@@ -2792,5 +2807,5 @@
 
 	    if ((!defined $metamode) || ($metamode =~ m/^\s*$/)) {
-		# make "accumulate" the default (less destructive, as won't actually 
+		# make "accumulate" the default (less destructive, as it won't actually 
 		# delete any existing values)
 		$metamode = "accumulate";
@@ -2810,5 +2825,5 @@
 		my $metamode  = $metatable_rec->{'metamode'} || $doc_array_rec->{'metamode'} || $self->{'metamode'};
 		if ((!defined $metamode) || ($metamode =~ m/^\s*$/)) {
-		    # make "accumulate" the default (less destructive, as won't actually 
+		    # make "accumulate" the default (less destructive, as it won't actually 
 		    # delete any existing values)
 		    $metamode = "accumulate";
@@ -2950,10 +2965,15 @@
 	my $metapos = $self->{'metapos'};
 	my $metavalue = $self->{'metavalue'};
-	if(defined $metavalue) {
+
+	$metapos = undef if(defined $metapos && ($metapos =~ m/^\s*$/));
+	$metavalue = undef if(defined $metavalue && ($metavalue =~ m/^\s*$/));
+
+	if(defined $metavalue) { # metavalue is now a compulsory arg for remove_import_metadata()
 	    $metavalue =~ s/&lt;(.*?)&gt;/<$1>/g;
 	} elsif (!defined $metapos) { # if given no metavalue or metapos to delete, default to deleting the 1st
 	    $metapos = 0;
 	}
-	my $metamode = $self->{'metamode'} || undef;
+	my $metamode = $self->{'metamode'};
+	$metamode = undef if(defined $metamode && ($metamode =~ m/^\s*$/));
 
 	# import works with metadata.xml which can have inherited metadata
@@ -3196,5 +3216,5 @@
 
     my $where = $self->{'where'};
-    if(!$where) {
+    if(!$where || ($where =~ m/^\s*$/)) {
 	$self->remove_index_metadata(@_); # call the full version of set_index_meta for the default behaviour
 	return;
@@ -3251,7 +3271,12 @@
     my $metaname  = $self->{'metaname'};
     my $metapos   = $self->{'metapos'};
-    my $metavalue = $self->{'metavalue'} || undef; # necessary to force fallback to undef here
+    my $metavalue = $self->{'metavalue'};
+
+    $metapos = undef if(defined $metapos && ($metapos =~ m/^\s*$/));
+    $metavalue = undef if(defined $metavalue && ($metavalue =~ m/^\s*$/)); # necessary to force fallback to undef here
 
     # To people who know $collect_tail please add some comments
+    # -> In collection groups, I think collect_tailname is the subcollection name,
+    # e.g. colgroup-name/col-tail-name
     # Obtain the path to the database
     my $collect_tail = $collect;
@@ -3380,16 +3405,16 @@
     # Check additional args
     my $docid = $self->{'d'};
-    if (!defined($docid)) {
+    if (!defined($docid) || ($docid =~ m/^\s*$/)) {
 	$gsdl_cgi->generate_error("No document id is specified: d=...");
     } 
     my $metaname = $self->{'metaname'};
-    if (!defined($metaname)) {
+    if (!defined($metaname) || ($metaname =~ m/^\s*$/)) {
 	$gsdl_cgi->generate_error("No metaname is specified: metadataname=...");
     } 
     my $metavalue = $self->{'metavalue'};
-    if (!defined($metavalue) || $metavalue eq "") {
+    if (!defined($metavalue) || ($metavalue =~ m/^\s*$/)) {
 	$gsdl_cgi->generate_error("No metavalue or empty metavalue is specified: metadataname=...");
     } 
-    # make "accumulate" the default (less destructive, as won't actually 
+    # make "accumulate" the default (less destructive, as it won't actually 
     # delete any existing values)
     my $metamode = "accumulate";
