[Tools v2] testbot: Document the issues with composite keys and Collections.

Namely Collection->Add() uses the initial Item's primary key. If that
key changes then KeyChanged() must be called.

Signed-off-by: Francois Gouget <[email protected]>
---

There was a couple of copy/paste errors in comments in the previous 
version.

 testbot/lib/ObjectModel/Collection.pm | 13 +++++++++++
 testbot/lib/ObjectModel/Item.pm       | 43 +++++++++++++++++++++++++++++++++++
 testbot/lib/WineTestBot/Steps.pm      |  5 +++-
 testbot/lib/WineTestBot/Tasks.pm      |  5 +++-
 4 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/testbot/lib/ObjectModel/Collection.pm b/testbot/lib/ObjectModel/Collection.pm
index 47d2b9bc..9d3d3aef 100644
--- a/testbot/lib/ObjectModel/Collection.pm
+++ b/testbot/lib/ObjectModel/Collection.pm
@@ -161,6 +161,19 @@ sub Load($)
   $self->{Loaded} = 1;
 }
 
+=pod
+=over 12
+
+=item C<Add()>
+
+Instantiates a new Item and adds it to the Collection.
+
+See Item->new() for details on the Item's default column values and for
+important information regarding its primary key.
+
+=back
+=cut
+
 sub Add($)
 {
   my ($self) = @_;
diff --git a/testbot/lib/ObjectModel/Item.pm b/testbot/lib/ObjectModel/Item.pm
index fe74f35c..82195bfa 100644
--- a/testbot/lib/ObjectModel/Item.pm
+++ b/testbot/lib/ObjectModel/Item.pm
@@ -36,6 +36,29 @@ require Exporter;
 @ISA = qw(Exporter);
 @EXPORT_OK = qw(&new);
 
+
+=pod
+=over 12
+
+=item C<new()>
+
+Instantiates a new Item and calls _initialize() to set up default values for
+its columns, including the primary key.
+
+Note that it is strongly recommended that _initialize() set up the primary key
+such that the new Item can be inserted into a Collection without colliding with
+existing items.
+
+Ideally repeated calls to new() should also each generate unique initial keys
+so it is possible to add multiple new Items to a Collection before knowing
+their final key value.
+
+In all cases, it is the responsibility of the caller to adjust the initial
+primary key if needed and to call KeyChanged(). See PutColValue() for details.
+
+=back
+=cut
+
 sub new($$@)
 {
   my $class = shift;
@@ -138,6 +161,26 @@ sub GetColValue($$)
   return $self->{ColValues}{$ColName};
 }
 
+=pod
+=over 12
+
+=item C<PutColValue()>
+
+Sets the specified column value.
+
+Note that if the column is part of the primary key it is up to the caller to
+then invoke KeyChanged() so the object can be retrieved from the Collection
+using the new key value.
+
+Further note that KeyChanged() should only be called once all of the
+primary key??s columns have reached their final values: in a two column
+primary key, calling KeyChanged() with the (new1, old2) values may collide with
+another Item which would cause a spurious error even though the final
+(new1, new2) key is unique.
+
+=back
+=cut
+
 sub PutColValue($$$)
 {
   my ($self, $ColName, $Value) = @_;
diff --git a/testbot/lib/WineTestBot/Steps.pm b/testbot/lib/WineTestBot/Steps.pm
index 6f2b399e..4161f0b0 100644
--- a/testbot/lib/WineTestBot/Steps.pm
+++ b/testbot/lib/WineTestBot/Steps.pm
@@ -47,9 +47,12 @@ sub InitializeNew($$)
 {
   my ($self, $Collection) = @_;
 
-  $self->Status("queued");
+  # Make up an initial, likely unique, key so the Step can be added to the
+  # Collection
   my $Keys = $Collection->GetKeys();
   $self->No(scalar @$Keys + 1);
+
+  $self->Status("queued");
   $self->Type("single");
   $self->InStaging(1);
   $self->DebugLevel(1);
diff --git a/testbot/lib/WineTestBot/Tasks.pm b/testbot/lib/WineTestBot/Tasks.pm
index 62eff5d7..efe1b21b 100644
--- a/testbot/lib/WineTestBot/Tasks.pm
+++ b/testbot/lib/WineTestBot/Tasks.pm
@@ -50,10 +50,13 @@ sub InitializeNew($$)
 {
   my ($self, $Collection) = @_;
 
-  $self->Status("queued");
+  # Make up an initial, likely unique, key so the Task can be added to the
+  # Collection
   my $Keys = $Collection->GetKeys();
   $self->No(scalar @$Keys + 1);
 
+  $self->Status("queued");
+
   $self->SUPER::InitializeNew($Collection);
 }
 
-- 
2.14.2