The plugin condition provides an array of allowed plugins thus a plain comparison with the plugin name of the given entity will fail in every case.
I'm not sure if $conditions['plugin'] is always an array thus I've added a little overhead in the code to make sure $conditions['plugin'] is always an array for the comparison.

CommentFileSizeAuthor
#3 rules-entity-controller-apply-conditions.patch821 bytesdas-peter
rules-entity-controller-apply-conditions.patch820 bytesdas-peter

Comments

das-peter’s picture

das-peter commented

Just had a good chat with DamZ about this issue.
It seems that conditions never were thought to be arrays.
Here the main arguments from DamZ why we should review this implementation:

  • EntityAPIController::applyConditions() doesn't support it
  • ::buildQuery() supports it by chance
  • $condition is just completely deprecated in favor of the way more powerful EntityFieldQuery

Atm. the array values shouldn't have a negative affect since the two conditions, that could contain an array as value, are unset before the other conditions are passed to the parent implementation of applyConditions().

Since I'm not feeling competent enough to make a decision here, and I'm just happy to see my default rule set now, no patch is attached. :)

fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Needs review » Needs work

Oh I see, that code didn't work with arrays. :/

Using EntityFieldQuery() doesn't work in that case, as rules may be defined in code - thus the EntityFieldQuery would miss those. entity API could provide an EntityFieldQuery implementation taking care of that - however I'm not sure whether that's the way to go. See #1008810: Store all exportables in the db.

Anyway, as of now your patch makes sense, but if(isset( doesn't comply to the coding standards.

das-peter’s picture

das-peter commented
Status: Needs work » Needs review
StatusFileSize
new rules-entity-controller-apply-conditions.patch821 bytes

Damn, 4 lines of code and I manage it to mess it up :)

fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Needs review » Fixed

hehe, thanks committed. :)

fago’s picture

fago
Primary language German
Location Vienna
commented
Title: RulesEntityController::applyConditions error on evaluating plugin condition » problem applying conditions with an array as value
Project: Rules » Entity API
Version: 7.x-2.x-dev » 7.x-1.x-dev
Component: Rules Engine » Entity CRUD controller
Status: Fixed » Active

Looking at this again, I think this is a bug in the entity API:

db conditions work like that:

   *   The comparison operator, such as =, <, or >=. It also accepts more
   *   complex options such as IN, LIKE, or BETWEEN. Defaults to IN if $value is
   *   an array, and = otherwise.

However the logic used for applying conditions to cached entities doesn't default to IN for arrays.

fago’s picture

fago
Primary language German
Location Vienna
commented
Project: Entity API » Drupal core
Version: 7.x-1.x-dev » 7.x-dev
Component: Entity CRUD controller » base system

>However the logic used for applying conditions to cached entities doesn't default to IN for arrays.
As noted in #1081902-11: DrupalEntityControllerInterface::load - doc needs to clarify $conditions I think we have the same problem in core, thus assigning to core.

From looking at the logic entity_load() it would continue with loading the entity from the DB in that case though.

>Just had a good chat with DamZ about this issue.
>It seems that conditions never were thought to be arrays.

I agree that this might be an "unintentional" feature, but removing it right now might break some existing code.

damien tournoud’s picture

damien tournoud commented

I agree that this might be an "unintentional" feature.

There is no such thing. The documentation for $conditions never stated that the values can themselves be arrays.

Status: Active » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.