Magento Collection Loading Prematurely

Question

I am pretty new to Magento, and am creating a module to integrate third party reviews with a website I am developing.

It's all going well, but I've hit a snag, and I'm at a bit of a loose end with what exactly is going wrong.

I have to get reviews from the database corresponding to a certain product. After some googling, the query I came up with was this:

$reviews = Mage::getModel('ekomiwrapper/productReview')
            ->getCollection()
            ->addFieldToFilter('product_id', $product_id)
            ->setPageSize($reviews_count)->load();

When I visit a product page, however, if there are no reviews associated with that product, the query returns nothing, and I get the message saying there ar no reviews associated. But when at least one review is set for a product, it returns every review.

I logged the SQL query that was being executed with

Mage::log($reviews->getSelect()->__toString());

and the output was

SELECT `main_table`.* FROM `productreview` AS `main_table` WHERE (product_id = 'JDA 955760')

which to my knowledge selects only the reviews that match the product ID. Indeed, when I run this query manually, it does just that. I can only conclude that this is not the query being run.

If anyone can shine some light on what I am doing wrong here, or point me to a resource that explains the Magento ORM better, then that would really help.

Many thanks.

Update

I'm still not completely sure what the exact root of the problem was, but by replacing Mage::getModel('ekomiwrapper/productReview')->getCollection() with Mage::getResourceModel('ekomiwrapper/productReview_collection').

I made a small sandbox:

require('app/Mage.php');
$app = Mage::app();

$sku = 'FOOBAR';

$reviews = Mage::getModel('ekomiwrapper/productReview')->getCollection();
foreach($reviews->addFieldToFilter('product_id', $sku)->setPageSize(1)->setCurPage(2) as $review) {
        echo $review->getRating()."\n";
}

$reviews = Mage::getResourceModel('ekomiwrapper/productReview_collection');
foreach ($reviews->addFieldToFilter('product_id', $sku)->setPageSize(1)->setCurPage(2) as $review) {
        echo $review->getRating()."\n";
}

The only thing different is in the way I'm getting a collection, and sure enough, the first loop returns all items in the database, and the second returns only one, as it is supposed to. The same problem I had originally. Weird, huh?

So why is this happening? I decided to take a deeper look at getCollection(). At this point it's worth noting that after spending a long time searching, I'm still not sure what's wrong. I'm including what I learned just in case it helps someone in the future.

public function getCollection()
{
    return $this->getResourceCollection();
}

/**
 * Get collection instance
 *
 * @return object
 */
public function getResourceCollection()
{
    if (empty($this->_resourceCollectionName)) {
        Mage::throwException(Mage::helper('core')->__('Model collection resource name is not defined.'));
    }
    return Mage::getResourceModel($this->_resourceCollectionName, $this->_getResource());
}

But a var_dump of my model shows that _resourceCollectionName is defined, and defined to be ekomiwrapper/productReview_collection. So that's not the problem. Perhaps it's in the getResource call:

/**
 * Get resource instance
 *
 * @return Mage_Core_Model_Mysql4_Abstract
 */
protected function _getResource()
{
    if (empty($this->_resourceName)) {
        Mage::throwException(Mage::helper('core')->__('Resource is not set.'));
    }

    return Mage::getResourceSingleton($this->_resourceName);
}

So that's not the problem either, because _resourceName is defined correctly as well. Let's continue down the stack to see if we can find our problem.

So let's take a look at Mage::getResourceModel. Maybe something funny is going on there:

/**
 * Retrieve object of resource model
 *
 * @param   string $modelClass
 * @param   array $arguments
 * @return  Object
 */
public static function getResourceModel($modelClass, $arguments = array())
{
    return self::getConfig()->getResourceModelInstance($modelClass, $arguments);
}

And Mage_Core_Model_Config::getResourceModelInstance():

/**
 * Get resource model object by alias
 *
 * @param   string $modelClass
 * @param   array $constructArguments
 * @return  object
 */
public function getResourceModelInstance($modelClass='', $constructArguments=array())
{
    $factoryName = $this->_getResourceModelFactoryClassName($modelClass);
    if (!$factoryName) {
        return false;
    }
    return $this->getModelInstance($factoryName, $constructArguments);
}

I'm going to ignore finding the class name, since I know that's working. But it looks like resource models are just models in the end, which comports with my understanding thus far.

Mage_Core_Model_Config::getModelInstance()

/**
 * Get model class instance.
 *
 * Example:
 * $config->getModelInstance('catalog/product')
 *
 * Will instantiate Mage_Catalog_Model_Mysql4_Product
 *
 * @param string $modelClass
 * @param array|object $constructArguments
 * @return Mage_Core_Model_Abstract|false
 */
public function getModelInstance($modelClass='', $constructArguments=array())
{
    $className = $this->getModelClassName($modelClass);
    if (class_exists($className)) {
        Varien_Profiler::start('CORE::create_object_of::'.$className);
        $obj = new $className($constructArguments);
        Varien_Profiler::stop('CORE::create_object_of::'.$className);
        return $obj;
    } else {
        return false;
    }
}

This function is finally where the collection is created. If you can remember what it was we were doing originally. I can, but only just. Now we do the same thing for the second, working method to find a difference.

We start with Mage::getResourceModel, which we've already seen.

When we instantiated the collection by Mage_Core_Model_Abstract::getResourceCollection(), we passed a second argument, which was the resource model of the model class. I'm not sure why, but it gets passed to the collection's constructor. I don't use it, although perhaps I should. Anyway, after all that, I'm certain that my collection is a singleton and that it's the right type. Sure enough, I'm vindicated by a call to get_class().

Incidentally, the code that works, with a single call to Mage::getResourceModel() creates an object of the same type, but not the same instance. I think that's what two objects not being equal in PHP means, anyway. So we get the resource model location then we pass it to a normal model factory. We're still in sync here. Both of the calls to this function return the right classname. So what happens next?

Another Update

After the last exercise proved fruitless, I decided to run $collection->load(true) against my collection, to see the queries being generated (thanks to Vinai for this tip).

The second collection outputs its SQL just fine, with its limit and offset. The first one does nothing. It's running its query, though, because I can see the output plain as day.

So my colleague advises me to check if the first collection is already loaded. And it is. $reviews->isLoaded() returns true.

So I now have a new question. Why is my collection loading as soon as it gets instantiated when I do it by Mage::getModel()::getCollection()?

Answer 1

Posting this as an answer, since comments just don't have the space. It is not an answer for your actual question, but an answer none the less.

In my opinion you are approaching the problem the wrong way.

My question still stands. Why not use core functionality? What does you review module give that core do not have?

The sync? Easy enough to sync into magento's core review system, and I give some examples at the end of my ramblings.

One golden rule, when doing modules, is to enhance, not replace. If there is core functionality that closely matches your planned solution, extend it, don't replace it. Core is stable, use it :) You will end up writing a lot less code, and have a more stable module in the end. Don't re-invent the wheel. It is round, and it works well :)

Magento is a complex application, and adding 3rd party solutions (for existing functionality) needlessly adds to that complexity. This is true for any complex application, not just magento.

You state that you cannot use magento's core review functionality, as you need to sync with an external system, but it looks to me (from the code snippet given) that you are pulling in all the reviews, into the magento database, into a custom table. How you do this is not known, but I assume it is done via a cron/periodic sync. So in the end, you have just duplicated what core already has, since you are not displaying the reviews from the external system directly onto the page. You are saving it in magento db first. (which is the right way, your implementation is not quite right ;) )

Looking at your code, I can already see an inherit issue. You are using product SKU's (textual data) as a means to find the data that you need to display. You are also incorrectly calling this the $product_id (which by the name of the variable, suggests a numeric value) In magento the product_id (or entity_id really) is the key value of the magento product, and you should not make matters confusing by calling a SKU field the product_id field.

Your table, should in fact not even have the SKU as part of it's structure, but a foreign key value, referencing back to the main product_entity table in magento, ideally set with 'ON DELETE CASCADE'

Using the SKU in a textual match will cause you heaps of issues down the line. What do you do if your client decides to rename all their product sku's? (this can happen, and I have seen it happen multiple times)

But enough of that, and lets get back to the issue in not using core review system, and your sync issue.

How you get the data from the external system (API, direct db connect etc) is beyond the scope of this example, and should be the only code/module you should really need to write. (the connector to/from that system)

In my example I went with a cron based solution, thus the external review system reviews will be pulled in every x minutes)

The code is of the top of my head, thus can be incomplete, or have mistakes, it serves purely as an example of how to do it using core review system.

I also gather the only link between that system and your products is the SKU (hence why you used it as your custom review table lookup key field)

class MyModule_ExternalReviewSystem_Model_Cron {

   public function importReviews($schedule){

       $externalReviews = CODE TO CONNECT TO EXTERNAL SYSTEM, 
                          AND PULL IN REVIEWS AS AN ARRAY/COLLECTION/DB 
                          ROW ITERATOR HERE.  



       foreach ($externalReviews as $key => $review) {
         $productId = $product->getIdBySku($externalReviews['sku']);
         if ($productId) {
             $data = array('nickname'=>$externalReviews['name'],
                           'detail'=>$externalReviews['review'],
                           'title'=>$externalReviews['subject']);
             $review = Mage::getModel('review/review')->setData($data);
             $review->setEntityId($review->getEntityIdByCode(Mage_Review_Model_Review::ENTITY_PRODUCT_CODE))
                    ->setEntityPkValue($productId)
                    ->setStatusId(Mage_Review_Model_Review::STATUS_APPROVED)
                    ->setCustomerId(null)
                    ->setStoreId(Mage::app()->getStore()->getId())
                    ->setStores(array(Mage::app()->getStore()->getId()))
                    ->save();
         } else {
             mage::log("Could not import {$externalReviews['sku']} - not found in local db")
         }

       } 
   }

} 

Done, now your external system review is inside magento's review system, and magento core will take care of all the rest.

To sync back from core review system is really easier. All you need to do is create an observer which listens to the review model save event, and sync that data back on a save. Thus you would listen to the event review_save_after, which gets passed the entire review object, from which you can get all the product details, the review data etc.

Using core functionality results in less code, less complexity, less errors, and way way less work for yourself. Not to mention time and money saved for your client :)

Hope this helps.

Answer 2

If the query returns the correct result, then the only conclusion is you are using a different collection to access the data you are displaying.
Check you are really using the same instance of the collection, not a clone or something, on which the collection isn't set.

Also, try using $collection->load(true) to output the SQL query that is indeed used to load the full collection. Fetching the select instance from the collection and casting it to a string my not always be accurate, as additional options might be set depending on collection properties during the load() processing for the collection.

UPDATE: (after updated question)
Probably your collections load() method is called because it's members are already accessed, probably using the implemented interfaces (iteration or count). Even though you already went over the model's getCollection() method, the culprit can't be the collection class itself, since the direct instantiation via the factory method works.
You might want to past your model class code. Somewhere in there something must be happening which causes the collection to be loaded. It sure isn't Mage_Core_Model_Abstract::getCollection()...