Object constructors shouldn't require an id if passed a $oRow object. Gets rid of

objectGetInstanceFromRow() since we can pass the row into the constructor instead.

include/appData.php

@@ -17,7 +17,7 @@ class appData
 
     function appData($iId = null, $oRow = null)
     {
-        if(!$iId)
+        if(!$iId && !$oRow)
             return;
 
         if(!$oRow)

include/application.php

@@ -46,8 +46,9 @@ class Application {
     function Application($iAppId = null, $oRow = null)
     {
         // we are working on an existing application
-        if(is_numeric($iAppId))
+        if(!$iAppId && !$oRow)
-        {
+            return;
+
         if(!$oRow)
         {
             /* fetch this applications information */
@@ -60,7 +61,7 @@ class Application {
 
         if($oRow)
         {
-                $this->iAppId = $iAppId;
+            $this->iAppId = $oRow->appId;
             $this->iVendorId = $oRow->vendorId;
             $this->iCatId = $oRow->catId;
             $this->iSubmitterId = $oRow->submitterId;
@@ -93,7 +94,6 @@ class Application {
             }
         }
     }
-    }
 
     function _internal_retrieve_all_versions()
     {
@@ -847,11 +847,6 @@ class Application {
         return $aCells;
     }
 
-    function objectGetInstanceFromRow($oRow)
-    {
-        return new application($oRow->appId, $oRow);
-    }
-
     function objectOutputTableRow($oObject, $sClass, $sEditLinkLabel)
     {
         $oUser = new user($this->iSubmitterId);

include/application_queue.php

@@ -10,13 +10,13 @@ class application_queue
     var $oApp;
     var $oVendor;
 
-    function application_queue($iAppId = null)
+    function application_queue($iAppId = null, $oRow = null)
     {
-        $this->oApp = new application($iAppId);
+        $this->oApp = new application($iAppId, $oRow);
 
         /* If this is an existing application then there must be a version
            accompanying it */
-        if($iAppId)
+        if($this->oApp->iAppId)
         {
             /* Normal users do not get a aVersionsIds property, so we have to fetch
                the versionId manually.  Normal users only have access to rejected
@@ -302,11 +302,6 @@ class application_queue
         return $this->oApp->objectGetHeader();
     }
 
-    function objectGetInstanceFromRow($oRow)
-    {
-        return application::objectGetInstanceFromRow($oRow);
-    }
-
     function objectOutputTableRow($oObject, $sClass, $sEditLinkLabel)
     {
         return $this->oApp->objectOutputTableRow($oObject, $sClass, $sEditLinkLabel);

include/distribution.php

@@ -20,8 +20,9 @@ class distribution {
     function distribution($iDistributionId = null, $oRow = null)
     {
         // we are working on an existing distribution.
-        if(is_numeric($iDistributionId))
+        if(!$iDistributionId && !$oRow)
-        {
+            return;
+
         // We fetch the data related to this distribution.
         if(!$oRow)
         {
@@ -34,7 +35,7 @@ class distribution {
 
         if($oRow)
         {
-                $this->iDistributionId = $iDistributionId;
+            $this->iDistributionId = $oRow->distributionId;
             $this->sName = $oRow->name;
             $this->sUrl = $oRow->url;
             $this->sSubmitTime = $oRow->submitTime;
@@ -74,7 +75,6 @@ class distribution {
             }
         }
     }
-    }
 
     // Creates a new distribution.
     function create()
@@ -453,11 +453,6 @@ class distribution {
                                 $iStart, $iRows);
     }
 
-    function objectGetInstanceFromRow($oRow)
-    {
-        return new distribution($oRow->distributionId, $oRow);
-    }
-
     /* arg1 = OM object, arg2 = CSS style, arg3 = text for edit link */
     function objectOutputTableRow($oManager, $sClass, $sEditLinkLabel)
     {

include/downloadurl.php

@@ -17,15 +17,21 @@ class downloadurl
     var $bQueued;
 
     /* Fetch the data if id is given */
-    function downloadurl($iId = NULL)
+    function downloadurl($iId = NULL, $oRow = NULL)
+    {
+        if(!$iId && !$oRow)
+            return;
+
+        if(!$oRow)
         {
             $hResult = query_parameters("SELECT id, versionId, description, url,
                 submitTime, submitterId, queued FROM appData WHERE id = '?'",
                     $iId);
 
             if($hResult && mysql_num_rows($hResult))
-        {
                 $oRow = mysql_fetch_object($hResult);
+        }
+
         if($oRow)
         {
             $this->iId = $oRow->id;
@@ -37,7 +43,6 @@ class downloadurl
             $this->bQueued = ($oRow->queued == "true") ? TRUE : FALSE;
         }
     }
-    }
 
     /* Display download links for a given version */
     function display($iVersionId)
@@ -407,11 +412,6 @@ class downloadurl
         $oAppData->objectOutputTableRow($oObject, $sClass, $sEditLinkLabel);
     }
 
-    function objectGetInstanceFromRow($oRow)
-    {
-        return new appData($oRow->id, $oRow);
-    }
-
     function getOutputEditorValues($aClean)
     {
         $this->sUrl = $aClean['sDownloadUrlUrl'];

include/maintainer.php

@@ -18,13 +18,19 @@ class maintainer
     var $bQueued;
     var $sReplyText;
 
-    function maintainer($iMaintainerId = "")
+    function maintainer($iMaintainerId = null, $oRow = null)
+    {
+        if(!$iMaintainerId && !$oRow)
+            return;
+
+        if(!$oRow)
         {
             $sQuery = "SELECT * FROM appMaintainers WHERE maintainerId = '?'";
             $hResult = query_parameters($sQuery, $iMaintainerId);
             if($hResult)
-        {
                 $oRow = mysql_fetch_object($hResult);
+        }
+
         if($oRow)
         {
             $this->iMaintainerId = $oRow->maintainerId;
@@ -37,7 +43,6 @@ class maintainer
             $this->bQueued = $oRow->queued;
         }
     }
-    }
 
     function create()
     {
@@ -218,7 +223,7 @@ class maintainer
 
         /* Excluding requests for queued apps and versions, as these will be
            handled automatically */
-        $sQuery = "SELECT DISTINCT maintainerId, appMaintainers.submitTime FROM 
+        $sQuery = "SELECT DISTINCT appMaintainers.* FROM 
             appMaintainers, appFamily, appVersion WHERE
             appMaintainers.queued = '?'
             AND
@@ -486,11 +491,6 @@ class maintainer
         echo "</td></tr></table></div>\n\n";    
     }
 
-    function objectGetInstanceFromRow($oRow)
-    {
-        return new maintainer($oRow->maintainerId, $oRow);
-    }
-
     function canEdit()
     {
         if($_SESSION['current']->hasPriv("admin"))

include/objectManager.php

@@ -59,7 +59,7 @@ class ObjectManager
     function display_table($aClean)
     {
         $this->checkMethods(array("ObjectGetEntries", "ObjectGetHeader",
-             "ObjectGetInstanceFromRow", "ObjectOutputTableRow", "canEdit"));
+             "ObjectOutputTableRow", "canEdit"));
 
         $oObject = new $this->sClass();
 
@@ -108,8 +108,7 @@ class ObjectManager
         /* output each entry */
         for($iCount = 0; $oRow = mysql_fetch_object($hResult); $iCount++)
         {
-            $oObject = call_user_func(array($this->sClass,
+            $oObject = new $this->sClass(null, $oRow);
-                                            "objectGetInstanceFromRow"), $oRow);
 
             /* arg1 = OM object, arg2 = CSS style, arg3 = text for edit link */
             $oObject->objectOutputTableRow($this, ($iCount % 2) ? "color0" : "color1",
@@ -295,7 +294,7 @@ class ObjectManager
 
         for($i = 0; $oRow = mysql_fetch_object($hResult); $i++)
         {
-            $oCandidate = $oObject->objectGetInstanceFromRow($oRow);
+            $oCandidate = new $this->sClass(null, $oRow);
             if($oCandidate->objectGetId() == $this->iId)
             {
                 $i++;

include/screenshot.php

@@ -32,10 +32,13 @@ class Screenshot {
     /**    
      * Constructor, fetches the data and image objects if $iScreenshotId is given.
      */
-    function Screenshot($iScreenshotId = null)
+    function Screenshot($iScreenshotId = null, $oRow = null)
     {
         // we are working on an existing screenshot
-        if(is_numeric($iScreenshotId))
+        if(!$iScreenshotId && !$oRow)
+            return;
+
+        if(!$oRow)
         {
             $hResult = query_parameters("SELECT appData.*, appVersion.appId AS appId
                     FROM appData, appVersion 
@@ -43,11 +46,12 @@ class Screenshot {
                     AND id = '?'
                     AND type = 'screenshot'", $iScreenshotId);
             if($hResult)
-            {
                 $oRow = mysql_fetch_object($hResult);
+        }
+
         if($oRow)
         {
-                    $this->iScreenshotId = $iScreenshotId;
+            $this->iScreenshotId = $oRow->id;
             $this->sDescription = $oRow->description;
             $this->iAppId = $oRow->appId;
             $this->iVersionId = $oRow->versionId;
@@ -58,8 +62,6 @@ class Screenshot {
             $this->hFile = null;
         }
     }
-        }
-    }
  
 
     /**
@@ -557,11 +559,6 @@ class Screenshot {
             return appData::mustBeQueued();
     }
 
-    function objectGetInstanceFromRow($oRow)
-    {
-        return new appData($oRow->id, $oRow);
-    }
-
     /* arg1 = OM object, arg2 = CSS style, arg3 = text for edit link */
     function objectOutputTableRow($oObject, $sClass, $sEditLinkLabel)
     {

include/testData.php

@@ -27,8 +27,9 @@ class testData{
     function testData($iTestingId = null, $oRow = null)
     {
         // we are working on an existing test
-        if($iTestingId)
+        if(!$iTestingId && !$oRow)
-        {
+            return;
+
         // We fetch the data related to this test.
         if(!$oRow)
         {
@@ -58,7 +59,6 @@ class testData{
             $this->sQueued = $oRow->queued;
         }
     }
-    }
 
     // Creates a new Test Results.
     function create()
@@ -552,7 +552,8 @@ class testData{
         if(!$hResult)
             return 0;
 
-        $oRow = mysql_fetch_object($hResult);
+        if(!$oRow = mysql_fetch_object($hResult))
+            return 0;
 
         return $oRow->testingId;
     }
@@ -902,11 +903,6 @@ class testData{
         return $aCells;
     }
 
-    function objectGetInstanceFromRow($oRow)
-    {
-        return new testData($oRow->testingId, $oRow);
-    }
-
     function objectOutputTableRow($oObject, $sClass, $sEditLinkLabel)
     {
         $oVersion = new version($this->iVersionId);

include/testData_queue.php

@@ -5,9 +5,9 @@ class testData_queue
     var $oTestData;
     var $oDistribution;
 
-    function testData_queue($iTestId = null)
+    function testData_queue($iTestId = null, $oRow = null)
     {
-        $this->oTestData = new testData($iTestId);
+        $this->oTestData = new testData($iTestId, $oRow);
         $this->oDistribution = new distribution($this->oTestData->iDistributionId);
     }
 
@@ -132,11 +132,6 @@ class testData_queue
         return $this->oTestData->objectGetHeader();
     }
 
-    function objectGetInstanceFromRow($oRow)
-    {
-        return testData::objectGetInstanceFromRow($oRow);
-    }
-
     function objectOutputTableRow($oObject, $sClass, $sEditLinkLabel)
     {
         return $this->oTestData->objectOutputTableRow($oObject, $sClass, $sEditLinkLabel);

include/vendor.php

@@ -19,8 +19,9 @@ class Vendor {
     function Vendor($iVendorId = null, $oRow = null)
     {
         // we are working on an existing vendor
-        if(is_numeric($iVendorId))
+        if(!$iVendorId && !$oRow)
-        {
+            return;
+
         if(!$oRow)
         {
             /*
@@ -35,7 +36,7 @@ class Vendor {
 
         if($oRow)
         {
-                $this->iVendorId = $iVendorId;
+            $this->iVendorId = $oRow->vendorId;
             $this->sName = $oRow->vendorName;
             $this->sWebpage = $oRow->vendorURL;
             $this->sQueued = $oRow->queued;
@@ -47,7 +48,7 @@ class Vendor {
         $sQuery = "SELECT appId
                     FROM appFamily
                     WHERE vendorId = '?'";
-            if($hResult = query_parameters($sQuery, $iVendorId))
+        if($hResult = query_parameters($sQuery, $this->iVendorId))
         {
             while($oRow = mysql_fetch_object($hResult))
             {
@@ -55,7 +56,6 @@ class Vendor {
             }
         }
     }
-    }
 
 
     /**
@@ -222,11 +222,6 @@ class Vendor {
         return $aCells;
     }
 
-    function objectGetInstanceFromRow($oRow)
-    {
-        return new vendor($oRow->vendorId, $oRow);
-    }
-
     /* arg1 = OM object, arg2 = CSS style, arg3 = text for edit link */
     function objectOutputTableRow($oObject, $sClass = "", $sEditLinkLabel)
     {

include/version.php

@@ -44,8 +44,9 @@ class Version {
     function Version($iVersionId = null, $oRow = null)
     {
         // we are working on an existing version
-        if(is_numeric($iVersionId))
+        if(!$iVersionId && !$oRow)
-        {
+            return;
+
         /*
         * We fetch the data related to this version.
         */
@@ -62,7 +63,7 @@ class Version {
 
             if($oRow)
             {
-                    $this->iVersionId = $iVersionId;
+                $this->iVersionId = $oRow->versionId;
                 $this->iAppId = $oRow->appId;
                 $this->iSubmitterId = $oRow->submitterId;
                 $this->sSubmitTime = $oRow->submitTime;
@@ -76,7 +77,6 @@ class Version {
             }
         }
     }
-    }
 
 
     /**
@@ -1362,11 +1362,6 @@ class Version {
         echo html_tr($aCells, $sClass);
     }
 
-    function objectGetInstanceFromRow($oRow)
-    {
-        return new version($oRow->versionId, $oRow);
-    }
-
     function objectDisplayQueueProcessingHelp()
     {
         echo "<p>This is the list of versions waiting for your approval, ".

include/version_queue.php

@@ -6,12 +6,15 @@ class version_queue
     var $oVersion;
     var $oDownloadUrl;
 
-    function version_queue($iVersionId = null)
+    function version_queue($iVersionId = null, $oRow = null)
     {
-        $this->oVersion = new version($iVersionId);
+        $this->oVersion = new version($iVersionId, $oRow);
         $iTestingId = null;
         $iDownloadUrlId = null;
 
+        if(!$iVersionId)
+            $iVersionId = $this->oVersion->iVersionId;
+
         if($iVersionId)
         {
             $iTestingId = testData::getNewestTestIdFromVersionId($iVersionId,
@@ -163,11 +166,6 @@ class version_queue
         return $this->oVersion->objectGetHeader();
     }
 
-    function objectGetInstanceFromRow($oRow)
-    {
-        return version::objectGetInstanceFromRow($oRow);
-    }
-
     function objectOutputTableRow($oObject, $sClass, $sEditLinkLabel)
     {
         return $this->oVersion->objectOutputTableRow($oObject, $sClass, $sEditLinkLabel);

unit_test/test_om_objects.php

@@ -17,6 +17,8 @@ function test_class($sClassName, $aTestMethods)
     $oObject = new ObjectManager("");
     $oObject->sClass = $sClassName;
 
+    $iAppId = 65555;
+
     /* Check whether the required methods are present */
     if(!$oObject->checkMethods($aTestMethods, false))
     {
@@ -32,6 +34,82 @@ function test_class($sClassName, $aTestMethods)
         echo "Failed to create and log in user.\n";
         return FALSE;
     }
+
+    /* Test the class constructor */
+    if(!$oTestObject = create_object($sClassName, $oUser, $iAppId))
+        return FALSE;
+
+    /* Should return 1 or more, since there may be entries present already */
+    $hResult = $oTestObject->objectGetEntries(false, false);
+
+    if(!$hResult)
+    {
+        echo "Got '$hResult' instead of a valid MySQL handle\n";
+        echo "FAILED\t\t$sClassName::$sClassName\n";
+        $oTestObject->delete();
+        return FALSE;
+    }
+
+    if(!($oRow = mysql_fetch_object($hResult)))
+    {
+        echo "Failed to fetch MySQL object\n";
+        echo "FAILED\t\t$sClassName::$sClassName\n";
+        $oTestObject->delete();
+        return FALSE;
+    }
+
+    $oNewTestObject = new $sClassName(null, $oRow);
+    switch($sClassName)
+    {
+        case "application":
+            $iReceived = $oNewTestObject->iAppId;
+        break;
+        case "application_queue":
+            $iReceived = $oNewTestObject->oApp->iAppId;
+        break;
+        case "distribution":
+            $iReceived = $oNewTestObject->iDistributionId;
+        break;
+        case "downloadurl":
+            $iReceived = $oNewTestObject->iId;
+        break;
+        case "maintainer":
+            $iReceived = $oNewTestObject->iMaintainerId;
+        break;
+        case "testData":
+            $iReceived = $oNewTestObject->iTestingId;
+        break;
+        case "testData_queue":
+            $iReceived = $oNewTestObject->oTestData->iTestingId;
+        break;
+        case "vendor":
+            $iReceived = $oNewTestObject->iVendorId;
+        break;
+        case "version":
+            $iReceived = $oNewTestObject->iVersionId;
+        break;
+        case "version_queue":
+            $iReceived = $oNewTestObject->oVersion->iVersionId;
+        break;
+        case "screenshot":
+            $iReceived = $oNewTestObject->iScreenshotId;
+        break;
+    }
+
+    if(!$iReceived || !is_numeric($iReceived))
+    {
+        echo "Got '$iReceived' instead of a valid id\n";
+        echo "FAILED\t\t$sClassName::$sClassName()\n";
+        $oTestObject->delete();
+        return FALSE;
+    }
+
+    echo "PASSED\t\t$sClassName::$sClassName\n";
+
+    $oTestObject->delete();
+
+    cleanup($sClassName, $iAppId);
+
     /* Test the methods' functionality */
     foreach($aTestMethods as $sMethod)
     {
@@ -40,6 +118,49 @@ function test_class($sClassName, $aTestMethods)
             /* Should also test for queued entries, but vendor does not support
                queueing yet */
             case "objectGetEntries":
+                if(!$oTestObject = create_object($sClassName, $oUser, $iAppId))
+                    return FALSE;
+
+                /* Should return 1 or more, since there may be entries present already */
+                $iExpected = 1;
+                $hResult = $oTestObject->objectGetEntries(false, false);
+                $iReceived = mysql_num_rows($hResult);
+                $oTestObject->delete();
+                if($iExpected > $iReceived)
+                {
+                    echo "Got $iReceived instead of >= $iExpected\n";
+                    echo "FAILED\t\t$sClassName::$sMethod\n";
+                    $oTestObject->delete();
+                    return FALSE;
+                }
+
+                /* Class specific clean-up */
+                cleanup($sClassName, $iAppId);
+
+                echo "PASSED\t\t$sClassName::$sMethod\n";
+            break;
+        }
+    }
+
+    $oUser->delete();
+
+    echo "PASSED\t\t".$oObject->sClass."\n";
+    return TRUE;
+}
+
+function cleanup($sClassName, $iAppId)
+{
+    switch($sClassName)
+    {
+        case "maintainer":
+            $oApp = new application($iAppId);
+            $oApp->delete();
+        break;
+    }
+}
+
+function create_object($sClassName, $oUser, $iAppId)
+{
     $oUser->addPriv("admin");
     $oTestObject = new $sClassName();
     /* Set up one test entry, depending on class */
@@ -53,8 +174,8 @@ function test_class($sClassName, $aTestMethods)
             $oTestObject->sUrl = "http://appdb.winehq.org/";
             $oTestObject->sDescription = "DANGER";
             $oTestObject->iVersionId = 65000; // Just needs to be != 0 
+        break;
         case "maintainer":
-                        $iAppId = 65555;
             $oApp = new application();
 
             if(!$oApp->create())
@@ -88,38 +209,12 @@ function test_class($sClassName, $aTestMethods)
         if(!$hResult)
         {
             echo "FAILED\t\t$sClassName to create screenshot entry";
-                        returN FALSE;
+            return FALSE;
         }
         $oTestObject->iScreenshotId = mysql_insert_id();
     }
 
-                /* Should return 1 or more, since there may be entries present already */
+    return $oTestObject;
-                $iExpected = 1;
-                $hResult = $oTestObject->objectGetEntries(false, false);
-                $iReceived = mysql_num_rows($hResult);
-                $oTestObject->delete();
-                if($iExpected > $iReceived)
-                {
-                    echo "Got $iReceived instead of >= $iExpected\n";
-                    echo "FAILED\t\t$sClassName::$sMethod\n";
-                    return FALSE;
-                }
-                /* Class specific clean-up */
-                switch($sClassName)
-                {
-                    case "maintainer":
-                        $oApp->delete();
-                    break;
-                }
-                echo "PASSED\t\t$sClassName::$sMethod\n";
-            break;
-        }
-    }
-
-    $oUser->delete();
-
-    echo "PASSED\t\t".$oObject->sClass."\n";
-    return TRUE;
 }
 
 function test_object_methods()
@@ -132,7 +227,6 @@ function test_object_methods()
                           "getOutputEditorValues",
                           "objectGetEntries",
                           "objectGetHeader",
-                          "objectGetInstanceFromRow",
                           "objectOutputTableRow",
                           "objectMakeLink",
                           "objectMakeUrl",