[wp-trac] [WordPress Trac] #35658: Provide additional data for registered meta through register_meta()

WordPress Trac noreply at wordpress.org
Thu Jun 30 00:08:05 UTC 2016


#35658: Provide additional data for registered meta through register_meta()
-------------------------------------------------+-------------------------
 Reporter:  jeremyfelt                           |       Owner:  jeremyfelt
     Type:  enhancement                          |      Status:  assigned
 Priority:  normal                               |   Milestone:  4.6
Component:  Options, Meta APIs                   |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  needs-unit-tests needs-testing has-  |     Focuses:
  patch rest-api                                 |
-------------------------------------------------+-------------------------

Comment (by MikeSchinkel):

 Replying to [comment:67 jeremyfelt]:
 > After reading through `WP_Object_Type`...

 Thank you for taking the time to review it.

 > I'd like to avoid the overhead of a `WP_Object_Type` object this time
 around.

 Understood.  Your comment as well as @jtsternberg's comment made me
 rethink the approach of the 35658.5.diff patch and was something I wanted
 to address for a long time, which is to create a `::get_instance()`
 ''"~factory"'' method and to make `WP_Object_Type` immutable.

 '''Immutability means '''two(2)  things:''' lower overhead in terms of
 memory use''' when used frequently -- only the cost of aN object reference
 vs. the cost of a string references and alls its characters -- and it
 makes `WP_Object_Type` truly just a data typing mechanism which is
 effectively all I ever intended it to be.

 In addition your comment made me think through the naming of the major
 type that had always bothered me because of its inconsistency ''(i.e.
 `'post'`, `'term'`, `'user'` and `'comment'`.)''  The `->type` property
 that I used in the earlier patch was too generically named, and calling it
 `object_type` such as in 35658.8.diff would mean forcing the `object_type`
 name for the major type and thus ''(possibly)'' inadvertently
 standardizing on a new core notation for object type/subtype which as you
 said might create a problem for forward compatibility.

 So for the latest patch #35658.9.diff there is `type_group` which seemed
 the best. In this context a `'post'` is a ''"type group"'' of post types.
 A `'term'` is a ''"type group"'' for taxonomies. And so on.

 Back to immutability; the latest patch declares the  `->_type_group` and
 `->subtype` properties to be `private` and then exposed `->type_group()`
 and `->subtype()` methods. In addition I changed `__construct()` to
 `private` so it can only be instantiated by `::get_instance()` which will
 re-dispense an existing object anytime the same string representation for
 object type is requested ''(this is where the memory overhead is
 improved.)''

 > We only have "registered" subtypes for posts, though #12668 has made
 some things around comment types more friendly.

 Great point, and that make sense.

 Given that I would take it one step further; use `post_name` instead of
 `subtype` in the code and have `post_name` map to `subtype` in the case of
 `type_group` being equal to `'post'`, and that is what the latest patch
 offers.  It has a `->post_type()` method that returns `'post' ===
 $this->_type_group ? $this->_subtype : null` which ensures only post types
 are relevant at this point.

 If the future when a movement is made to support all subtypes then the
 change will be able to be explicit in the code. Better to approach it that
 way for clarity now.

 > It's a new one to core, but it's worth thinking about using a similarly
 formatted string in this context.

 It is actually not as you know that @ocean90
 [https://wordpress.slack.com/archives/core/p1467148281002121 pointed out
 on Slack] after your comment here. In fact is was what @nacin suggested
 last year ''(specifically using the the colon because of `image:gif` and
 `image:jpeg`)'' when reviewing the work I did on the Fields API.

 > I realized that I may want to register a meta key to multiple post
 types. With some work on [attachment:35658.4.diff], we could turn that
 into:
 >
 > {{{
 > register_meta( 'post', 'issue', array( 'object_subtype' => array(
 'newsletter', 'magazine' ), ... );
 > }}}
 >
 > By turning the first argument into one that contains both the object and
 object subtype, we would need to register it twice:
 >
 > {{{
 > register_meta( 'post:newsletter', 'issue', $args );
 > register_meta( 'post:magazine', 'issue', $args );
 > }}}
 >

 Actually the use of a `WP_Object_Type` does not preclude this at all.
 Nothing says that the first parameter can't be either an object type OR a
 type group, and if a type group then look for subtypes in the array. But I
 would suggest subtypes be provides using `post_type`, `taxonomy`, etc.
 Then inside `register_meta()` you can inspect what is passed and combine
 as appropriate:

 {{{
 register_meta( 'post', 'issue', array( 'post_type' => array( 'newsletter',
 'magazine' ), ... );
 }}}

 The new patch however does not attempt this yet because of uncertainty if
 this latest patch will even be considered.

 > From a previous [comment:10 comment of mine] that asked a bunch of
 questions:
 > If we can assume that a post type has always been registered before the
 meta key, then we could accept the object subtype as the first argument
 and then look up it's parent object type. However, this would lock us into
 object subtype uniqueness as comments/posts/terms/users could one day have
 conflicting subtype slugs.

 Accepting the subtype does not sounds like a good idea to me.

 > I think we should avoid the "any" subtype now and ask for explicit
 registration from developers.

 I am wondering why you would object to this? Are you objecting just on the
 grounds of meta, or for the concept of object type in general?

 I can see why it might not be a good idea for meta, but in general it can
 be quite useful.  The latest patch does not specifically disallow use of
 `'any'` for meta, but it could...

 Lastly, as you can tell I would really like to see `WP_Object_Type` make
 its way into core. @sc0ttkclark and I discussed this a length last year
 and I think he is generally supportive of object type class. I do think an
 `WP_Object_Type` class would help for meta though I certainly can
 appreciate how it would not be required.

 I'd ask that you please consider it once more -- and consider how it can
 help fields soon and relationships in the future too -- but if you are not
 willing to tackle it now then I would at least suggest wiping all
 `object_type` and `subtype` terminology and just going with
 `$args['post_type']` for now since all you want to have subtypes is
 `'posts'`.  And maybe adopt the `type_group` terminology too?

--
Ticket URL: <https://core.trac.wordpress.org/ticket/35658#comment:72>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list