Move field_reserved_columns() to a static method [#1969136]

From #1735118: Convert Field API to CMI. Not really sure how to explain this one, so I'll just quote what we said on-issue.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Core/Entity/Field.phpundefined
@@ -0,0 +1,463 @@
+      if (array_intersect(array_keys($schema['columns']), field_reserved_columns())) {

@xjm:

It looks like field_reserved_columns() returns only 'deleted'. A procedural wrapper to return that seems odd; followup?

@swentel:

Not for now, this function is also used in _field_sql_storage_columnname() for instance which does not get a $field object

@yched:

Yeah, I actually have no clue what the logic around field_reserved_columns() is. This was introduced by the EFQv2 patch, and the issue has no explanations about that.

But yeah, I tried to see if it could be moved to a method on Field.php, but the consuming code won't allow that.

Comment	File	Size	Author
#9	field-reserved-columns-1969136-9.patch	3.45 KB	swentel
#9	interdiff.txt	2.47 KB	swentel
#6	field-reserved-columns-1969136-6.patch	3.46 KB	swentel

Comments

Comment #1

yched commented 12 April 2013 at 21:52

Crossposted to #1801726: EntityFieldQuery v2

As far as entity_query() is concerned, ->condition('field_name.deleted', 0) needs to be recognized as a valid condition and this is the way it recognizes it. Field.php refuses creating those columns to avoid confusion.

Comment #3

swentel commented 24 May 2013 at 23:00

Right, makes sense. Having a procedural wrapper is not that bad, the function explains what it does and it's better than hardcoding it. So as far as I'm concerned, we can just close this one.

Comment #4

yched commented 24 May 2013 at 23:07

I'd tend to inline / hardcode that "list of forbidden column names" (currently restricted to 1 item...) directly in the Field.php class...
I don't really see the value of delegating this to an external functional helper now ?

Comment #5

swentel commented 24 May 2013 at 23:09

@yched static method then ? e.g. _field_sql_storage_columnname does not get a field object ..

Comment #6

swentel commented 25 May 2013 at 04:28

Status:

Active

» Needs review

Status	File	Size
new	field-reserved-columns-1969136-6.patch	3.46 KB

Let's move this to a static method.

Comment #7

yched commented 25 May 2013 at 08:34

Status:

Needs review

» Needs work

+++ b/core/modules/field/lib/Drupal/field/Plugin/Core/Entity/Field.phpundefined
@@ -446,7 +446,7 @@ public function getSchema() {
+      if (array_intersect(array_keys($schema['columns']), $this->field_reserved_columns())) {

I think the recommended practice is static::method().

+++ b/core/modules/field/lib/Drupal/field/Plugin/Core/Entity/Field.phpundefined
@@ -563,5 +563,13 @@ public function unserialize($serialized) {
+  public static function field_reserved_columns() {

Method name needs to get CamelCased :-)