2020-12-08 20:35 阅读 0

Improve match criteria

Hi Josh,

There are two commits in this pull request.

Commit #9057078 addresses some bugs I found in matching feed record for updating where the match fields were Craft element types.

In your match criteria, you are using

$criteria->elementFieldHandle = [12]
to indicate matching the IDs of Craft elements (entries, categories etc.). However, using that criteria format, I couldn't seem to get that to match any of my existing elements. But changing it to use the
criteria attribute seemed to fix the matching of Craft element types.

Also, in the

, doing some debugging I found that from step 2 onwards, the
variable initialised in
seemed to somehow be 'polluted/dirty' with the criteria used on the previous step (i.e. some of the criteria values were values used in the previous step). I thought that it might be something to do with that
variable being passed around by reference. So the fix I put in place was to initialize it with an actual clone of the Element Type's original base criteria. You may want to do some testing around this to verify if you get the same issue.

Commit #f2d257c is just a little addition I was hoping to have added in that in the future would support matching elements on additional element criteria attributes that have been programatically added using a plugin hook (which I will suggest in a future pull request). I have found this useful where the matching criteria needs to take into account special cases, such as where two elements have the same name etc. I have had success with programatically adding in some custom criteria to the

option (using a plugin hook) to ensure that the correct element is chosen as a match in special cases.

Let me know your thoughts.


  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享

4条回答 默认 最新

  • weixin_39625782 weixin_39625782 2020-12-08 20:35

    Thanks for these PR's, I'm going to test these out. A note on the 'dirty' criteria, its actually done for performance reasons. The criteria is setup before the feed starts, and only the attributes you're matching existing element on get changed each step. The performance improvements are small, but certainly noticeable on larger feeds.

    I'll test the rest of your PR shortly, but all looks good from face value.

    点赞 评论 复制链接分享
  • weixin_39917046 weixin_39917046 2020-12-08 20:35

    No probs.

    Regarding the 'dirty' criteria, what I was finding was the following.

    Say on step 1, passing

    into the
    resulted in having
    equal to
    ['and', [21]]

    Then when step 2 executes in

    , the line
    $criteria = $this-><em>criteria;</em>
    is executed to reinitialise
    with the base feed criteria. But after coming out of
    on step 2,
    would end up with the merged value of step 1 _and step 2 which is not what is expected (e.g.
    $criteria->relatedTo = ['and', [21, 37]]
    ). This would mean the feed element matching would fail later in the process step.

    Not 100% certain, but the same might have been happening with simple criteria elements too (e.g.

    ). Please test. If not then it might be something to do with my
    criteria adding in

    I'm sure the problem has got something to do with PHP variables by reference etc as

    is getting passed by reference into a few methods in the step processing. In fact, it might even be that
    is getting 'dirty', rather than
    (which is being assigned its value on each step).

    In the end, I found that changing

    $criteria = $this->_criteria;
    $criteria = clone $this->_criteria;
    fixed the problem. This resulted in the start of each step having
    correctly re-initialised with just the base feed criteria before going into

    Perhaps do some testing and see if you are getting the same results.

    点赞 评论 复制链接分享
  • weixin_39625782 weixin_39625782 2020-12-08 20:35

    I don't believe it'll merge the values for criteria on each step. Take matching against a Category for example, where categories is the field handle for our Category field.

    Step 1: $criteria->categories = [0 => 45781]

    Step 2: $criteria->categories = [0 => 45681]

    The above applies for any non-element field, and the values are overwritten each step. Obviously the above doesn't really match because its an element, which is the reason for your relatedTo modification.

    A quick look shows you're using $criteria->relatedTo = array_merge($criteria->relatedTo, $feedValue); so thats likely why. Any reason for this? Commenting out if (empty($criteria->relatedTo)) { ensures that its 'reset' to [and] each step of the query. Anyway, would need to test this out a bit further.

    I would also be very against using clone $this->_criteria due to performance.

    点赞 评论 复制链接分享
  • weixin_39917046 weixin_39917046 2020-12-08 20:35

    Say I set up my feed so that the unique identifier for existing elements contains three fields where each field is a Craft Entries field type. This means that

    needs to end up containing the IDs of each of the entries in those three fields, determined from the feed data on that step (e.g.
    $criteria->relatedTo['and', [14,98,27]]

    I may be wrong, as I'm only going from memory, but without the array_merge this will not be the case, and only the ID of the last entry field will end up in the

    点赞 评论 复制链接分享