Array to String conversion notice

[KeKs0r]

This pull request adresses issue #101

[danhunsaker]

Maybe I'm missing something, but shouldn't this be comparing values instead of keys? Or at least in addition to?

[KeKs0r]

You are right. I changed it instead of pseudo intersecting to a recursive diff function.

[danhunsaker]

Just seems that it would be good to compare values when checking for duplicate jobs. I can have two jobs with the same keys but wildly different values, and I'd hate those to be treated as the same job.

Just saw the new commit. Seems good, though I have to wonder why you use isset() in one part and array_key_exists() in the other...

[KeKs0r]

To be honest I copied this from the PHP Bug Report where people provided a recursive function. I think it uses isset() and array_key_exists() in order check in one scenario only if a key is present and the other case prevents it from being NULL.

[danhunsaker]

Odd. We probably care about NULLs in both cases, here...

[KeKs0r]

If NULL values should be compared we need to use array_key_exists which is used in the second case. In the first case isset is used because it doesn't matter we have checked before if it is an array. Which is good, in a case that we have in one array an empty array and in the other under the same key just a NULL value this would be treated as difference. But this is probably a VERY rare edge case.

[danhunsaker]

Noting that isset() is faster, it's good to use it whenever possible. But because of PHP's short-circuit behavior with boolean operations, isset() isn't an option in the second loop, because it would never check NULL !== $value cases, which we'd want it to. So this is the required approach.

Probably be good to check coding standards compliance, though. Not sure why people are so fond of else if instead of elseif, either...

[KeKs0r]

I am flexible with the coding standard. I use whatever my IDE is doing to my code. Do I need to adapt anything in order to get this merged?

[javaguirre]

I don't know if it's related but I've been debugging a problem with php-resque-scheduler, I had a problem in this line of php-resque-scheduler, there was a problem with zrangebyscore.

https://github.com/chrisboulton/php-resque-scheduler/blob/master/lib/ResqueScheduler.php#L228

$items = Resque::redis()->zrangebyscore('delayed_queue_schedule', '-inf', $at, array('limit' => array(0, 1)));

Removing the array('limit'...) solve all my issues, It seems that's not working, It also solved my problem with array string conversion.

$items = Resque::redis()->zrangebyscore('delayed_queue_schedule', '-inf', $at);

[danhunsaker]

@javaguirre - If you search the issues for Scheduler, you'll find references to a difference in how Credis handles zrangebyscore() and how the PHP Redis extension handles it. If you have the extension installed, the code will work as written in the repo (that is, with the limit intact). If not, Credis's own implementation kicks in and builds the request incorrectly because of some magic it tries to do with the command - you can simply exchange the array('limit' => array(0, 1)) portion with 'limit', 0, 1 to get it working in that case. If someone has time to add it, it would be nice to see a PR that handles this difference more elegantly, since we need the limit intact to prevent acting on too many items at once. But no, this is not a related issue.

@KeKs0r - Indentation should be consistent with the rest of the file (spaces versus tabs and that kind of thing), if, foreach, and other block statements should always be enclosed in {}s (even if they only have a single line of content), and so forth. I'm pretty sure compliance with PSRs is in effect (though feel free to ignore PSR-2's indentation recommendation if the rest of the file already uses tabs or more spaces than two...). To make sure, you could do worse than ask one of the project maintainers (@michelsalib, @mrbase, @ruudk).

[ruudk]

@KeKs0r Please reformat the code to this and test it:

    /**
     * Intersect of recursive arrays
     * needed for enqueueOnce
     *
     * @param array $array1
     * @param array $array2
     * @return array
     */
    protected static function array_diff_assoc_recursive($array1, $array2)
    {
        $difference = array();
        foreach($array1 as $key => $value) {
            if(is_array($value)) {
                if(!isset($array2[$key]) || !is_array($array2[$key])) {
                    $difference[$key] = $value;
                } else {
                    $new_diff = self::array_diff_assoc_recursive($value, $array2[$key]);
                    if(!empty($new_diff)) {
                        $difference[$key] = $new_diff;
                    }
                }
            } else {
                if(!array_key_exists($key, $array2) || $array2[$key] !== $value) {
                    $difference[$key] = $value;
                }
            }
        }

        return $difference;
    }

[javaguirre]

@danhunsaker Thank you, I'll do that. :-)

[ruudk]

@KeKs0r Any news on this?

[eXtreme]

Having the same problem with enqueueOnce. For me it is "caused" by setting auto_retry strategy for a job which is then part of "args" and fails in enqueueOnce @KeKs0r any progress?

[eXtreme]

BTW I don't think array_diff_assoc_recursive is doing the right job because it does not enqueue job with different values of args. I've tried array_intersect_recursive like in here: http://php.net/manual/en/function.array-intersect.php#110590 and it works OK.

[mpclarkson]

Hi there, as it's clear that this bundle is no longer being maintained I've published a fork at https://github.com/mpclarkson/resque-bundle where I have already incorporated a bundle of changes and improvements. Your contributions would be appreciated. This is still a bug.