Francois Gouget : testbot/web: Use Collection::GetSortedItems() for collection blocks.

[Alexandre Julliard]

Module: tools
Branch: master
Commit: a40ee48f7e74fa13777950ba73ec61bd27dec221
URL:    https://source.winehq.org/git/tools.git/?a=commit;h=a40ee48f7e74fa13777950ba...

Author: Francois Gouget 
Date:   Wed Apr 13 19:44:53 2022 +0200

testbot/web: Use Collection::GetSortedItems() for collection blocks.

This adds default Compare() methods to the Jobs and Patches classes
which allows removing all the CollectionBlock SortKeys() methods.

Signed-off-by: Francois Gouget 
Signed-off-by: Alexandre Julliard

---

testbot/lib/ObjectModel/CGI/CollectionBlock.pm | 15 +++++++-------
 testbot/lib/WineTestBot/Jobs.pm                |  8 ++++++++
 testbot/lib/WineTestBot/Patches.pm             |  8 ++++++++
 testbot/lib/WineTestBot/VMs.pm                 | 23 ---------------------
 testbot/web/JobDetails.pl                      | 28 +++++++++-----------------
 testbot/web/Submit.pl                          | 13 ++++++------
 testbot/web/admin/VMsList.pl                   |  7 -------
 testbot/web/index.pl                           | 15 --------------
 8 files changed, 39 insertions(+), 78 deletions(-)

diff --git a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
index 23825d7..f124e64 100644
--- a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
+++ b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
@@ -176,11 +176,11 @@ sub DisplayProperty($$)
   return $PropertyDescriptor->GetClass ne "Detailref";
 }

-sub SortKeys($$)
+sub GetSortedItems($$)
 {
-  my ($self, $Keys) = @_;
+  my ($self, $Items) = @_;

-  return $Keys;
+  return $self->{Collection}->GetSortedItems($Items);
 }

sub GetDisplayValue($$$)
@@ -428,16 +428,15 @@ EOF
   print "<tbody>\n";
   my $DetailsPage = $self->GetDetailsPage();
   my $Row = 0;
-  my $Keys = $self->SortKeys($self->{Collection}->GetKeys());
-  foreach my $Key (@$Keys)
+  my $Items = $self->{Collection}->GetSortedItems();
+  foreach my $Item (@$Items)
   {
     my $Class = ($Row % 2) == 0 ? "even" : "odd";
-    my $Item = $self->{Collection}->GetItem($Key);
     $self->GenerateDataRow($Item, $PropertyDescriptors, $DetailsPage,
                            $Class, $ItemActions);
     $Row++;
   }
-  if (@$Keys == 0)
+  if (@$Items == 0)
   {
     print "<tr class='even'><td colspan='0'>No entries</td></tr>\n";
   }
@@ -445,7 +444,7 @@ EOF
   print "</tbody>\n";
   print "</table>\n";

-  if (@$ItemActions != 0 && @$Keys != 0)
+  if (@$ItemActions != 0 && @$Items != 0)
   {
     print <<EOF;
 <div class='CollectionBlockActions'>
diff --git a/testbot/lib/WineTestBot/Jobs.pm b/testbot/lib/WineTestBot/Jobs.pm
index a5ae135..c082a36 100644
--- a/testbot/lib/WineTestBot/Jobs.pm
+++ b/testbot/lib/WineTestBot/Jobs.pm
@@ -111,6 +111,14 @@ sub InitializeNew($$)
   $self->SUPER::InitializeNew($Collection);
 }

+sub Compare($$)
+{
+  my ($self, $B) = @_;
+
+  # Hack: Use direct attribute access for performance
+  return $B->{ColValues}{Id} <=> $self->{ColValues}{Id}; # newest first by default
+}
+
 =pod
 =over 12

diff --git a/testbot/lib/WineTestBot/Patches.pm b/testbot/lib/WineTestBot/Patches.pm
index 6891fa5..d2ac52e 100644
--- a/testbot/lib/WineTestBot/Patches.pm
+++ b/testbot/lib/WineTestBot/Patches.pm
@@ -58,6 +58,14 @@ sub InitializeNew($$)
   $self->SUPER::InitializeNew($Collection);
 }

+sub Compare($$)
+{
+  my ($self, $B) = @_;
+
+  # Hack: Use direct attribute access for performance
+  return $B->{ColValues}{Id} <=> $self->{ColValues}{Id}; # newest first by default
+}
+
 =pod
 =over 12

diff --git a/testbot/lib/WineTestBot/VMs.pm b/testbot/lib/WineTestBot/VMs.pm
index 2e475b7..f0c16d1 100644
--- a/testbot/lib/WineTestBot/VMs.pm
+++ b/testbot/lib/WineTestBot/VMs.pm
@@ -809,29 +809,6 @@ sub CreateVMs(;$)
                                @PropertyDescriptors, $ScopeObject);
 }

-sub SortKeysBySortOrder($$)
-{
-  my ($self, $Keys) = @_;
-
-  # Sort retired and deleted VMs last
-  my %RoleOrders = ("retired" => 1, "deleted" => 2);
-
-  my %SortOrder;
-  foreach my $Key (@$Keys)
-  {
-    my $Item = $self->GetItem($Key);
-    $SortOrder{$Key} = [$RoleOrders{$Item->Role} || 0, $Item->SortOrder, $Item->Name];
-  }
-
-  my @SortedKeys = sort {
-    my ($soa, $sob) = ($SortOrder{$a}, $SortOrder{$b});
-    return @$soa[0] <=> @$sob[0] ||
-           @$soa[1] <=> @$sob[1] ||
-           @$soa[2] cmp @$sob[2];
-  } @$Keys;
-  return @SortedKeys;
-}
-
 sub FilterEnabledRole($)
 {
   my ($self) = @_;
diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl
index 5a24204..da4ad7e 100644
--- a/testbot/web/JobDetails.pl
+++ b/testbot/web/JobDetails.pl
@@ -56,14 +56,6 @@ sub DisplayProperty($$)
          $PropertyName eq "Ended" || $PropertyName eq "TestFailures";
 }

-sub SortKeys($$)
-{
-  my ($self, $Keys) = @_;
-
-  my @SortedKeys = sort { $a <=> $b } @$Keys;
-  return @SortedKeys;
-}
-

#
 # Item cell generation
@@ -361,10 +353,10 @@ sub InitMoreInfo($)
   my ($self) = @_;

my $More = $self->{More} = {};
-  my $Keys = $self->{CollectionBlock}->SortKeys($self->{Collection}->GetKeys());
-  foreach my $Key (@$Keys)
+  my $SortedStepTasks = $self->{Collection}->GetSortedItems();
+  foreach my $StepTask (@$SortedStepTasks)
   {
-    my $StepTask = $self->{Collection}->GetItem($Key);
+    my $Key = $StepTask->GetKey();
     $More->{$Key}->{Screenshot} = $self->GetParam("s$Key");

my $Value = $self->GetParam("f$Key");
@@ -569,17 +561,17 @@ function HideLog(event, url)
 EOF

print "<div class='Content'>\n";
-  my $Keys = $self->{CollectionBlock}->SortKeys($self->{Collection}->GetKeys());
-  my $KeyIndex = 0;
-  foreach my $Key (@$Keys)
+  my $Index = 0;
+  my $SortedStepTasks = $self->{Collection}->GetSortedItems();
+  foreach my $StepTask (@$SortedStepTasks)
   {
-    my $StepTask = $self->{Collection}->GetItem($Key);
+    my $Key = $StepTask->GetKey();
     my $TaskDir = $StepTask->GetTaskDir();
     my $VM = $StepTask->VM;

-    my $Prev = $KeyIndex > 0 ? "k". $Keys->[$KeyIndex-1] : "PageTitle";
-    my $Next = $KeyIndex + 1 < @$Keys ? "k". $Keys->[$KeyIndex+1] : "PageEnd";
-    $KeyIndex++;
+    my $Prev = $Index > 0 ? "k". $SortedStepTasks->[$Index-1]->GetKey() : "PageTitle";
+    my $Next = $Index + 1 < @$SortedStepTasks ? "k". $SortedStepTasks->[$Index+1]->GetKey() : "PageEnd";
+    $Index++;
     print "<h2><a name='k", $self->escapeHTML($Key), "'></a>",
           $self->escapeHTML($StepTask->GetTitle()),
           " <span class='right'><a class='title' href='#$Prev'>&uarr;</a><a class='title' href='#$Next'>&darr;</a></span></h2>\n";
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl
index 1ad94e4..8a1cc3c 100644
--- a/testbot/web/Submit.pl
+++ b/testbot/web/Submit.pl
@@ -474,10 +474,9 @@ sub _initialize($$$)
   $VMs->AddFilter("Type", ["win32", "win64", "wine"]);
   $VMs->FilterEnabledRole();

-  my $SortedKeys = $VMs->SortKeysBySortOrder($VMs->GetKeys());
-  foreach my $VMKey (@$SortedKeys)
+  my $SortedVMs = $VMs->GetSortedItems();
+  foreach my $VM (@$SortedVMs)
   {
-    my $VM = $VMs->GetItem($VMKey);
     my ($ErrMessage, $Caps) = GetMissionCaps($VM->Missions, $VM->MissionCaps);
     LogMsg "$ErrMessage\n" if (defined $ErrMessage);

@@ -492,7 +491,7 @@ sub _initialize($$$)
       }
     }

-    my $FieldBase = $self->escapeHTML($VMKey);
+    my $FieldBase = $self->escapeHTML($VM->GetKey());
     foreach my $Build (sort @Builds)
     {
       my $VMRow = {
@@ -752,10 +751,10 @@ sub GenerateFields($)
       print "<div class='ItemProperty'><label>Branch</label>",
             "<div class='ItemValue'>",
             "<select name='Branch' size='1'>";
-      my @SortedKeys = sort { $a cmp $b } @{$Branches->GetKeys()};
-      foreach my $Key (@SortedKeys)
+      my $SortedBranches = $Branches->GetSortedItems();
+      foreach my $Branch (@$SortedBranches)
       {
-        my $Branch = $Branches->GetItem($Key);
+        my $Key = $Branch->GetKey();
         print "<option value='", $self->escapeHTML($Key), "'";
         if (defined $self->{Branch} and $Key eq $self->{Branch})
         {
diff --git a/testbot/web/admin/VMsList.pl b/testbot/web/admin/VMsList.pl
index cfd5b91..f8162c6 100644
--- a/testbot/web/admin/VMsList.pl
+++ b/testbot/web/admin/VMsList.pl
@@ -43,13 +43,6 @@ sub DisplayProperty($$)
          $PropertyName eq "Description";
 }

-sub SortKeys($$)
-{
-  my ($self, $Keys) = @_;
-
-  return $self->{Collection}->SortKeysBySortOrder($Keys);
-}
-
 sub OnItemAction($$$)
 {
   my ($self, $VM, $Action) = @_;
diff --git a/testbot/web/index.pl b/testbot/web/index.pl
index 85f8df3..478cb61 100644
--- a/testbot/web/index.pl
+++ b/testbot/web/index.pl
@@ -35,14 +35,6 @@ use WineTestBot::Utils;
 my $DAYS_DEFAULT = 4;

-sub SortKeys($$)
-{
-  my ($self, $Keys) = @_;
-
-  my @SortedKeys = sort { $b <=> $a } @$Keys;
-  return @SortedKeys;
-}
-
 sub DisplayProperty($$)
 {
   my ($self, $PropertyDescriptor) = @_;
@@ -188,13 +180,6 @@ use ObjectModel::CGI::CollectionBlock;
 our @ISA = qw(ObjectModel::CGI::CollectionBlock);

-sub SortKeys($$)
-{
-  my ($self, $Keys) = @_;
-
-  return $self->{Collection}->SortKeysBySortOrder($Keys);
-}
-
 sub DisplayProperty($$)
 {
   my ($self, $PropertyDescriptor) = @_;