Remove UNIQUE for slug in wp_terms

[nacin]

To set us up for future changes to the taxonomy API, we should remove the UNIQUE key for 'slug' for wp_terms. Said future changes include:

• Splitting shared terms on update (#5809)
• Stop creating shared terms (#21950)
• Forcibly split remaining shared terms
• Merge wp_terms and wp_term_taxonomy (I can dream, right?)

The term_exists() check should prevent duplicate terms from ever being inserted (before, of course, we fix #5809 and #21950). But, this needs unit tests, particularly because there is a case where term_exists() breaks. See #17689, which blocks this ticket.

For the version 3.5 we would need:

• Remove the UNIQUE from slug on wp_terms
• Improvments on term_exists()

Just checking so we can delivery something on 3.5

Is this part of 3.5 or 3.6?

No unit tests; #17689 not fixed yet. Punting.

Yeah, I thought this was punted a while ago.

Fixing #5809 requires this to be fixed to get the ball rolling for ​http://make.wordpress.org/core/2013/07/28/potential-roadmap-for-taxonomy-meta-and-post-relationships/. #17689 must be fixed first.

We can do #5809 and this in the same release.

There's a patch now. A teeny, tiny one.

Just a hair late for 3.9-early, now ;-)

This ticket was mentioned in IRC in #wordpress-dev by helen. ​View the logs.

#17689 is in, let's do this pronto.

This ticket was mentioned in IRC in #wordpress-dev by simonwheatley. ​View the logs.

I'm not clear on the testing requirement here; there are tests newly added by #17689 (test_duplicate_name) which indirectly exercise term_exists, but I'm happy to write more direct testing if that's required.

Replying to ircbot:

This ticket was mentioned in IRC in #wordpress-dev by simonwheatley. ​View the logs.
@nacin – "I don't think the current API in any way allows for multiple duplicate term slugs to sneak in the database, except possibly by race condition. which I am concerned about."

I've been thinking about Nacin's concerns above, and AFAIK it's not possible to have a UNIQUE constraint across the wp_term and wp_term_taxonomy tables. If this (race conditions allowing duplicate terms) is a big enough concern to block this ticket, and we need to block the possibility of duplicate terms with a constraint at the database level, then one alternative is to lose the wp_terms table (​as mentioned in the potential taxonomy roadmap) so we can have a UNIQUE constraint in the restructured wp_term_taxonomy on taxonomy and slug (and name, perhaps). This is obviously (somewhat understating things) a big change… but we're considering some other structure changes, so may be a good time to get it in? I imagine we'd also want to have time and resource to test both core and existing plugins?

Beef up tests for term_exists

Additional tests for term_exists:

• Don't use rand_str in test_term_exists_known or test_term_exists_unknown, instead specify a term name (allows for debugging tests more easily if the term name is in DB already)
• Check optional $taxonomy parameter
• Check optional `$parent parameter
• Incidentally check inserting a term with a parent
• Check specifying $term as integer other than 0 (in this case 99999)
• No need to assert the term deletion (we're testing term deletion functionality elsewhere)

This ticket was mentioned in IRC in #wordpress-dev by simonwheatley. ​View the logs.

This ticket was mentioned in IRC in #wordpress-dev by helen. ​View the logs.

Replying to simonwheatley:

Those additional tests look good, and I'm definitely a fan of removing uses of rand_str (actually seen that cause collisions in rare cases).

Looking forward, let's put together a list of further areas that need testing. No one needs to actually write the tests yet, just pull together a list of what we need to test so we can divvy up work and get started. Thoughts?

Let's do this in 4.1 early so we can get lots of nice tests in place and testing done.

term_exists-tests.02.patch is an exhaustive set of tests for term_exists(), as well as some tests that show the duplicate-term failure conditions for wp_insert_term().

These tests look great.

In 29798:

Improve unit test coverage for duplicate term creation.

These include an exhaustive set of tests for term_exists(), as well as tests
for wp_insert_term() that demonstrate failure when attempting to create a
duplicate term.

Props simonwheatley for an initial patch.

See #22023.

The tests committed in r29798 are all passing. As long as all term creation passes through wp_insert_term() and uses term_exists(), I'm pretty confident that we're close to being ready to remove the UNIQUE key.

This ticket was mentioned in IRC in #wordpress-dev by johnbillion. ​View the logs.

See #29589, an edge case where term_exists() fails.

In 29830:

Improve unit test coverage for wp_insert_term().

See #5809, #22023.

In 29875:

Improve unit test coverage for wp_update_term().

See #5809, #22023.

YOLO

In 30056:

Remove UNIQUE key from 'slug' column of terms table.

Each slug is a unique and beautiful snowflake, but let's enforce that
uniqueness elsewhere.

Props hotchkissconsulting.
Fixes #22023.