Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make get_object_terms work with and without object caching. #105

Merged
merged 1 commit into from Dec 2, 2014

Conversation

Projects
None yet
2 participants
@joshlevinson
Copy link
Contributor

commented Dec 2, 2014

When get_object_terms resorts to using get_the_terms, get_object_term_cache is called.
The data stored by update_object_term_cache is an associative array, the key being the term id.
This causes $name[0]->slug to attempt to access an array item that doesn't exist.
This PR makes use of the key function to get the first item from the array regardless of whether it's associative or not.

Make get_object_terms work with and without object caching.
When get_object_terms resorts to using get_the_terms, get_the_terms calls get_object_term_cache.
The data stored by update_object_term_cache stores it in an associative array, the key being the term id.
This causes $name[0]->slug to attempt to access an array item that doesn't exist.
This PR makes use of the `key` function to get the first item from the array regardless of whether it's associative or not.
@joshlevinson

This comment has been minimized.

Copy link
Contributor Author

commented Dec 2, 2014

I did some poking around because this behavior felt unnatural to me—that get_the_terms can return two different types of results depending on whether or not the results are present in the object cache. It's definitely a requirement to keep the associative array style based on how wp_cache_add works ($key, $data, $group - $key being the term_id).

However, I feel like the behavior should still be consistent. This leaves the only option being to change wp_get_object_terms so it returns an associative array instead of the current indexed one.

I compared the taxonomy functions with how metadata is retrieved to determine whether they currently behave similarly (in which case I would think wp_get_object_terms should be left alone).
get_metadata pulls from either wp_cache_get or update_meta_cache, which will both yield an associative array with the meta_id being the key. Taxonomies are different in that they're arrays of objects, so they have identifiers without using an array key. I still feel that taxonomies could benefit from a similar pattern though.

Does this sound like a good first trac ticket for me to take on?

jtsternberg added a commit that referenced this pull request Dec 2, 2014

Merge pull request #105 from joshlevinson/master
Make get_object_terms work with and without object caching. Props @joshlevinson

@jtsternberg jtsternberg merged commit 8857bdb into CMB2:master Dec 2, 2014

2 checks passed

ci/scrutinizer Scrutinizer: No new issues — Tests: passed
Details
continuous-integration/travis-ci The Travis CI build passed
Details
@jtsternberg

This comment has been minimized.

Copy link
Member

commented Dec 2, 2014

Josh, hmm these are good thoughts, and that does sound like a good trac ticket to open, though one may exist already. Let me know what you find!

@jtsternberg

This comment has been minimized.

Copy link
Member

commented Dec 2, 2014

And thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.