Unable to set tv values from plugin

[frankie_tech]

I have a plugin that is supposed to calculate a “similarity score” between references based on template variables. While the iterator is pulling multiple resources, the first foreach is only run once. Then at the end the tv value isn’t set for the first iteration of the first foreach loop. I’m just trying to get it to run on OnDocFormSave first and then I’ll test the rest of the events. This is the plugin code:

<?php

$templateId = 6;
// Only proceed for resources with template ID 6 on add, update, delete, publish/unpublish
$eventTypes = ['OnDocFormSave', 'OnDocPublished', 'OnDocUnpublished', 'OnResourceDelete'];
if (in_array($modx->event->name, $eventTypes) && $resource->get('template') == $templateId) {
    $found_resources = $modx->getIterator('modResource', [ 'template' => $templateId ]);
    
    # Logs the right number of resources
    $modx->log(1, $modx->getCount('modResource', [ 'template' => $templateId ]));
    
    foreach($found_resources as $idx => $current_resource) {
        $current_id = $current_resource->get('id');
        $similar_resources = [];
        $modx->log(1, "Processing resource with ID: $current_id");
        foreach($found_resources as $resource) {
            $id = $resource->get('id');
            
            if ($current_id != $id) {
                # This function is cut for brevity
                $similarity_score = calculateSimilarity($current_resource, $resource);
                $similar_resources[$id] = $similarity_score;
            }
        }
        
        arsort($similar_resources);
        $top_matches = array_slice(array_keys($similar_resources), 0, 2);
        
        $match_values =implode(',', $top_matches);
        $modx->log(1, $match_values);
        $modx->log(1, print_r($similar_resources));
        if (!$current_resource->setTVValue('matchValues', $match_values)) {
            # This does not log
            $modx->log(1, "Could not set matchValues to $match_values");
        } else {
            # pretty sure I don't need to do this, it isn't with or without this
            $current_resource->save();
        }
    }
}

[halftrainedharry]

@frankie_tech

foreach($found_resources as $idx => $current_resource) {
        ...
        foreach($found_resources as $resource) {

I really don’t think you can use the same iterator in nested foreach loops like this. The inner foreach loop will iterate to the last item of the list and this will also affect the value of the iterator of the outer loop.

If there aren’t too many resources, you could use $modx->getCollection() instead, and then copy the returned array and use it for the inner loop.
Or maybe run $modx->getIterator() twice at the beginning for the two loops.

[frankie_tech]

So currently there are 6, but the client is likely to get up to 50 by the end of the year. I’m guilty of overly optimizing in advance, in your experience would that be an issue for getCollection?

[halftrainedharry]

50 is not an issue.
If there where thousands of resources, it would probably create a problem as the whole array of resources would have to be kept in the memory.

---

I think the code should work, if you just replace getIterator() with getCollection().

[frankie_tech]

Yeah, that got it working! It makes sense that an iterator wouldn’t be able to duplicate like that.