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

Duplication of post meta keys #258

Closed
arsendovlatyan opened this issue Nov 26, 2018 · 7 comments
Closed

Duplication of post meta keys #258

arsendovlatyan opened this issue Nov 26, 2018 · 7 comments
Milestone

Comments

@arsendovlatyan
Copy link
Contributor

I was testing Distributor with Woo-commerce (using external connection) and all of the sudden noticed that meta keys getting duplicated in destination, with every update i have more and more duplicated meta keys.

I'm not sure if this is related to 1.3.4 release or #200 , but couldn't find similar bug-report and want to make sure that you are aware about it.

P.C. i have used fresh WordPress install (4.9.8) with fresh WooCommerce (3.5.1) and last version of Distributor 1.3.4

@helen @dkotter want to make sure that you are aware about this issue, we are not using Distributor in production yet, but if other do, it can add a mess in post meta table.

@madmax3365 to follow

@jakemgold
Copy link
Member

Likely related to 1.3.4 - we tried to handle a situation where the same meta key is used multiple times on the source side (intentionally - WordPress supports that use case) - and it seems likely that this is unintended side effect with WooCommerce. We'll look into, but in the mean time, you can always roll back to 1.3.3 via Github.

@helen
Copy link
Contributor

helen commented Nov 26, 2018

@arsendovlatyan Looking into this - when you say "with every update", what kind of update are you talking about?

@arsendovlatyan
Copy link
Contributor Author

@helen Just regular post update, from admin interface, in source/original website.
@jakemgold unfortunately, it's not related with WooCommerce, even if WooCommerce is disabled and testing on regular posts, it still have the same issue.

Original:
image

Distributed:
image

@jakemgold
Copy link
Member

Quick clarification - we only see the empty "_test_meta" field being duplicated in your example; all the other fields seem fine. Can you confirm that all or some other fields are also duplicating? Is it just empty key values?

@arsendovlatyan
Copy link
Contributor Author

Yes, I can confirm that only empty (or NULL) meta values will be duplicated.

arsendovlatyan added a commit to NovemBit/distributor that referenced this issue Nov 26, 2018
@arsendovlatyan
Copy link
Contributor Author

Please consider my pull request, since it will solve another issue as well - distribution of serialized meta values, currently it's not working.
We are using get_post_meta to retrieve existing meta values $existing_meta = get_post_meta( $post_id ); , because meta_key is not provided, maybe_unserialize won't be applied automatically, so, we have to apply it manually before using in update_post_meta as $prev_value argument.

I have tested many different cases, and seems like everything works fine.

The only thing that i have noticed, is when you have 2 or more meta values, with the same keys and one of them is empty ('' or 0), in that case, all meta values in distention will eventually be reset to empty.
In practice this case can happen only if there are update requests to database with meta ID, as much as I know, there is no built-in WordPress functions to handle such updates, so, it must be very rare case and can be handled by applying appropriate hooks.

arsendovlatyan added a commit to NovemBit/distributor that referenced this issue Nov 26, 2018
@helen helen added this to the 1.3.5 milestone Dec 5, 2018
@helen
Copy link
Contributor

helen commented Dec 5, 2018

Commented on the PR about this too but also want to summarize here:

The only thing that i have noticed, is when you have 2 or more meta values, with the same keys and one of them is empty ('' or 0), in that case, all meta values in distention will eventually be reset to empty.

I think this is the same cause as what I saw in testing #259, which is that all those duplicated meta keys get set to the same value because of the way update_post_meta() works (doesn't take an ID). We also don't actually delete any meta at any point, so they just kind of linger there. I think we need to address post meta deletion for sure, but carefully and separately. For now, this is a good enough fix for 1.3.5 and I think will actually also alleviate a back-compat issue I noticed (although I didn't test to make sure it truly works correctly, it just won't fully break).

helen pushed a commit that referenced this issue Dec 5, 2018
* fix of meta duplication, when meta value is empty or null #258
refactoring of set_meta function

* fix of meta duplication, when meta value is empty or null #258
refactoring of set_meta function

* fix undefined variable notice.

* PHPCS and other formatting fixes

* Don't store the response of `update_post_meta()` (yet)

We should definitely report back any errors in storing meta; however we need to do it for both update and add and it needs to store into an identifiable array since it's inside a loop. Let's pick that up for 1.4 for sure.

* Cast `$meta_values` to an array first

Older versions of Distributor might not return an array for everything, this doesn't guarantee back-compat but at least it shouldn't explode anymore.
@helen helen closed this as completed Dec 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants