[wp-trac] [WordPress Trac] #14162: Introduce WP_Term class

[WordPress Trac]

#14162: Introduce WP_Term class
--------------------------------------+---------------------------
 Reporter:  scribu                    |       Owner:  boonebgorges
     Type:  enhancement               |      Status:  assigned
 Priority:  normal                    |   Milestone:  4.4
Component:  Taxonomy                  |     Version:
 Severity:  normal                    |  Resolution:
 Keywords:  needs-patch dev-feedback  |     Focuses:
--------------------------------------+---------------------------

Comment (by boonebgorges):

 @flixos90 Thanks for your patch. This is a helpful starting point. I've
 just spent a while working with it.

 > Please note that all function calls still require specification of the
 taxonomy - removing this requirement is part of #30262 I think.

 Not necessarily. Taxonomy only needs to be specified if terms in two
 taxonomies can share the same ID. After the shared-term splitting in 4.3,
 this should no longer be the case. I would strongly prefer to remove this
 requirement, so that you can simply say `get_term( $term_id )` and `$term
 = new WP_Term( $term_id )`.

 The problem with this proposal is that it breaks our current cache
 implementation. Term caches are currently sorted into buckets based on
 taxonomy: `$_term = wp_cache_get( $term_id, $taxonomy )`. But this schema
 means that we need to know the taxonomy before we check the cache. This
 makes it hard to see how `get_term( $term_id )` will work without
 restructuring how terms are cached - probably by moving them to a single
 bucket, ie `$_term = wp_cache_get( $term_id, 'terms' )`. This would mean
 that a cache reorganization would have to happen *before* (or at the same
 time as) `WP_Term` is introduced.

 This makes the job somewhat more complicated, but I think it's far better
 to introduce the new class - which represents a brand new interface for
 the API - without the extra baggage of an extraneous `$taxonomy`
 parameter.

 [attachment:14162.2.patch] is an attempt to do this. I had to change a
 couple of existing unit tests that were referencing the old cache
 locations. And there is some terrible logic built in that allows shared
 terms to continue to be split inside of `wp_update_term()` while still
 allowing `$taxonomy` to be an optional parameter. This patch passes all
 but 1 unit test, which I'm too tired to figure out at the moment.

 From flixos90's original patch, I also made a few changes. I removed the
 `to_array()` juggling in `_make_cat_compat()` in favor of removing the by-
 reference assignments, which don't really do anything in PHP5 anyway.

 I also removed the magic getters. I'm not opposed to adding them, but (a)
 it's not clear that it needs to be done in the first iteration, and (b)
 the backward compatibility concerns that led to the properties being added
 to `WP_Post` don't apply here. The discussion at
 https://core.trac.wordpress.org/ticket/21309 provides some helpful
 background. For the moment, I'd suggest that we get our cache
 implementation straight, and then we can talk about pulling in various
 bits of term functionality.

 One additional note: The `filter` and `sanitize_term()` stuff here - and
 in `WP_Post` - is quite confusing. The idea is to provide out-of-the-box
 sanitization for any term field. But we should try to avoid double-
 sanitizing, and ideally we would centralize the sanitization logic more
 than it is in `WP_Post`. This needs an audit.

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