fix(files): handle multidimensional arrays in scanner

[joshtrichards]

• Resolves: [Bug]: enabling external storage on 28.0.2 generates error during occ files:scan #43408
• Also likely fixes [Bug]: Nextcloud Hub 8, copying files to Object Storage configured as Primary Storage isn't reliable #45082 and [Bug]: Unable to upload "big" files to S3 external storage #43642

Summary

array_diff_assoc() doesn't support multidimensional arrays on its own (and, when attempted, it internally casts any embedded array elements to strings to attempt to compare them - not only generating warnings like "Array to string conversion" but also then overlooking differences).

But sometimes we're passing it a multidimensional array.

I think the reason this became a new problem in v28 is because the metadata (#40761 / etc) gets embedded as a second level array here. It happens that triggering a scan on an Object Store is one way to trigger this code path just right to see the behavior found in #43408. (In other words: there may be other cases where an n-level array ends up here, but I don't know of one off-hand; and this should accommodate any of them in any case).

TODO

• Housekeeping
  • Fix code formatting
  • Fix docblock
  • Fix commits (squash, change message)
• Confirm whether also fixes/closes [Bug]: Nextcloud Hub 8, copying files to Object Storage configured as Primary Storage isn't reliable #45082
• Confirm whether also fixes/closes [Bug]: Unable to upload "big" files to S3 external storage #43642

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[solracsf]

Another potential approach: iterating over each element in the $data array and then compare each nested element individually, instead of serialize/unserialize (not tested though, may need some adjustements).

$newData = [];
foreach ($data as $key => $value) {
    if (is_array($value)) {
        if (!array_key_exists($key, $cacheData) || !is_array($cacheData[$key])) {
            $newData[$key] = $value;
        } else {
            $nestedDiff = [];
            foreach ($value as $nestedKey => $nestedValue) {
                if (!array_key_exists($nestedKey, $cacheData[$key]) || $cacheData[$key][$nestedKey] !== $nestedValue) {
                    $nestedDiff[$nestedKey] = $nestedValue;
                }
            }
            if (!empty($nestedDiff)) {
                $newData[$key] = $nestedDiff;
            }
        }
    } else {
        if (!array_key_exists($key, $cacheData) || $cacheData[$key] !== $value) {
            $newData[$key] = $value;
        }
    }
}

[lavigne958]

I tried this fix on my instance and it works. no more error logs from array_diff_assoc.

[artonge]

Thanks for the investigation and lengthy explanation. It makes sense IMO. :)

[icewind1991]

array-diff-uassoc can be used to customize how the comparison is done.

I'm not a fan of round-tripping data through serialize if we can avoid it

[ahcm]

Sorry, if this in the wild, I just skimmed through the code.

Is it really worthwhile to only updating the diff?
Whatever DB is behind will do that probably anyways or be faster writing all.

[DaanSelen]

Is this patch slated to release for version 30 or any minor patches for 29?

[joshtrichards]

Reimplemented w/o serialization. Tests reasonably in my test bed. Mostly housekeeping items left.

Is this patch slated to release for version 30 or any minor patches for 29?

It'll likely make it into the next monthly maintenance releases for v27/v28/v29 (well before v30 is formally published).

[lavigne958]

This code has been running on my instance for the past 12h and not a single line of error has raised, it works fine 👍

[joshtrichards]

/backport to stable29

[joshtrichards]

/backport to stable28