{"action":"created","issue":{"url":"https://api.github.com/repos/akeneo/pim-community-dev/issues/3916","labels_url":"https://api.github.com/repos/akeneo/pim-community-dev/issues/3916/labels{/name}","comments_url":"https://api.github.com/repos/akeneo/pim-community-dev/issues/3916/comments","events_url":"https://api.github.com/repos/akeneo/pim-community-dev/issues/3916/events","html_url":"https://github.com/akeneo/pim-community-dev/pull/3916","id":128167607,"number":3916,"title":"[POC] PIM-5476 : during csv product import, new products are created with extra optional values","user":{"login":"nidup","id":2104359,"avatar_url":"https://avatars.githubusercontent.com/u/2104359?v=3","gravatar_id":"","url":"https://api.github.com/users/nidup","html_url":"https://github.com/nidup","followers_url":"https://api.github.com/users/nidup/followers","following_url":"https://api.github.com/users/nidup/following{/other_user}","gists_url":"https://api.github.com/users/nidup/gists{/gist_id}","starred_url":"https://api.github.com/users/nidup/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nidup/subscriptions","organizations_url":"https://api.github.com/users/nidup/orgs","repos_url":"https://api.github.com/users/nidup/repos","events_url":"https://api.github.com/users/nidup/events{/privacy}","received_events_url":"https://api.github.com/users/nidup/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/akeneo/pim-community-dev/labels/bug","name":"bug","color":"fc2929"}],"state":"closed","locked":false,"assignee":{"login":"nidup","id":2104359,"avatar_url":"https://avatars.githubusercontent.com/u/2104359?v=3","gravatar_id":"","url":"https://api.github.com/users/nidup","html_url":"https://github.com/nidup","followers_url":"https://api.github.com/users/nidup/followers","following_url":"https://api.github.com/users/nidup/following{/other_user}","gists_url":"https://api.github.com/users/nidup/gists{/gist_id}","starred_url":"https://api.github.com/users/nidup/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nidup/subscriptions","organizations_url":"https://api.github.com/users/nidup/orgs","repos_url":"https://api.github.com/users/nidup/repos","events_url":"https://api.github.com/users/nidup/events{/privacy}","received_events_url":"https://api.github.com/users/nidup/received_events","type":"User","site_admin":false},"milestone":{"url":"https://api.github.com/repos/akeneo/pim-community-dev/milestones/8","html_url":"https://github.com/akeneo/pim-community-dev/milestones/1.4.0","labels_url":"https://api.github.com/repos/akeneo/pim-community-dev/milestones/8/labels","id":1313211,"number":8,"title":"1.4.0","description":"","creator":{"login":"nidup","id":2104359,"avatar_url":"https://avatars.githubusercontent.com/u/2104359?v=3","gravatar_id":"","url":"https://api.github.com/users/nidup","html_url":"https://github.com/nidup","followers_url":"https://api.github.com/users/nidup/followers","following_url":"https://api.github.com/users/nidup/following{/other_user}","gists_url":"https://api.github.com/users/nidup/gists{/gist_id}","starred_url":"https://api.github.com/users/nidup/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nidup/subscriptions","organizations_url":"https://api.github.com/users/nidup/orgs","repos_url":"https://api.github.com/users/nidup/repos","events_url":"https://api.github.com/users/nidup/events{/privacy}","received_events_url":"https://api.github.com/users/nidup/received_events","type":"User","site_admin":false},"open_issues":3,"closed_issues":4,"state":"open","created_at":"2015-09-18T11:58:12Z","updated_at":"2016-01-28T09:02:09Z","due_on":"2015-09-24T22:00:00Z","closed_at":null},"comments":5,"created_at":"2016-01-22T14:31:18Z","updated_at":"2016-01-28T09:02:09Z","closed_at":"2016-01-28T09:02:09Z","pull_request":{"url":"https://api.github.com/repos/akeneo/pim-community-dev/pulls/3916","html_url":"https://github.com/akeneo/pim-community-dev/pull/3916","diff_url":"https://github.com/akeneo/pim-community-dev/pull/3916.diff","patch_url":"https://github.com/akeneo/pim-community-dev/pull/3916.patch"},"body":"TODO: do another attempt\r\n - move filter data from processor to product updater too\r\n - rework rest product controller to call the product updater too\r\n\r\n\r\n\r\nWe encountered a regression since the following 1.4.4 fix, new products are created with extra optional values for media, metric, price https://github.com/akeneo/pim-community-dev/pull/3412\r\n\r\nThis issued has been raised here https://github.com/akeneo/pim-community-dev/issues/3429.\r\n\r\nIn fact, this fix shown a limitation of product updater, the logic of this class is quite complex to be able to properly handle optional attributes in products. The API allows to add optional attribute values in a product if a not empty value is provided even if the attribute is not in the family of the product. The problem with the current implementation is that it properly detects empty data only for simple attribute types (text, decimal, date, simple select, etc) but not for more complex data as prices, metric and media.\r\n\r\nThis PR fixes this behavior and provides a new command to clean empty product values already in database. This command is not efficient at all because process purge on each product, one by one, it's very long to execute on large data set but can be run in several times and does not consume too much memory.\r\n\r\nBy installing the small catalog.\r\n\r\nBefore this fix:\r\n```\r\nmysql> select count(*) from pim_catalog_product;\r\n+----------+\r\n|     5000 |\r\n+----------+\r\nmysql> select count(*) from pim_catalog_product_value;\r\n+----------+\r\n|   342029 |\r\n+----------+\r\nImport small_cat has been successfully executed.\r\nreal\t21m1.744s\r\nuser\t17m53.809s\r\nsys\t0m30.667s\r\n```\r\n\r\nAfter this fix:\r\n```\r\nmysql> select count(*) from pim_catalog_product_value;\r\n+----------+\r\n|   257010 |\r\n+----------+\r\nImport small_cat has been successfully executed.\r\nreal\t16m35.848s\r\nuser\t13m47.064s\r\nsys\t0m27.684s\r\n```\r\n\r\nThen, once the cleanup command has been runned:\r\n```\r\nmysql> select count(*) from pim_catalog_product_value;\r\n+----------+\r\n|   164388 |\r\n+----------+\r\n```\r\nAbout EmptyChecker:\r\n - one for product values object, used by the purge command (to cleanup messed DB)\r\n - one for the product updater api to filter empty product value data (structured format)\r\n\r\n**Following is not covered by this PR**\r\nThis brings back in the discussion an old but interesting topic, the amount of stored product values has a real impact on performances. We could avoid to store any empty value in the storage by skipping them in the product updater layer. Queries, completeness computation and UI now properly handle this, the only complex limitation I see is the following use case: a user adds an optional attributes in a product and does not fulfill the value.\r\n\r\nJust for test purpose (it will be really done in a improvement), if I comment the following code in the ProductUpdater::updateProductValues()\r\n```\r\nif (/*$isFamilyAttribute ||*/ $providedData || $hasValue) {\r\n```\r\nThen re-import,\r\n```\r\nmysql> select count(*) from pim_catalog_product_value;\r\n+----------+\r\n| count(*) |\r\n+----------+\r\n|   159683 |\r\n+----------+\r\nImport small_cat has been successfully executed.\r\nreal\t11m57.151s\r\nuser\t9m50.628s\r\nsys\t0m25.846s\r\n```\r\n\r\nI runned Behat with this change, it breaks 9 behat scenario during a EE build (still running) http://core-ci.akeneo.com/job/dci_behat/967/console\r\n\r\n| Q                 | A\r\n| ----------------- | ---\r\n| Specs             | Y\r\n| Behats            | -\r\n| Blue CI           | ?\r\n| Changelog updated | Y\r\n| Review and 2 GTM  | ?"},"comment":{"url":"https://api.github.com/repos/akeneo/pim-community-dev/issues/comments/176071671","html_url":"https://github.com/akeneo/pim-community-dev/pull/3916#issuecomment-176071671","issue_url":"https://api.github.com/repos/akeneo/pim-community-dev/issues/3916","id":176071671,"user":{"login":"nidup","id":2104359,"avatar_url":"https://avatars.githubusercontent.com/u/2104359?v=3","gravatar_id":"","url":"https://api.github.com/users/nidup","html_url":"https://github.com/nidup","followers_url":"https://api.github.com/users/nidup/followers","following_url":"https://api.github.com/users/nidup/following{/other_user}","gists_url":"https://api.github.com/users/nidup/gists{/gist_id}","starred_url":"https://api.github.com/users/nidup/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nidup/subscriptions","organizations_url":"https://api.github.com/users/nidup/orgs","repos_url":"https://api.github.com/users/nidup/repos","events_url":"https://api.github.com/users/nidup/events{/privacy}","received_events_url":"https://api.github.com/users/nidup/received_events","type":"User","site_admin":false},"created_at":"2016-01-28T09:02:09Z","updated_at":"2016-01-28T09:02:09Z","body":"Closed in favor of https://github.com/akeneo/pim-community-dev/pull/3939\r\n\r\nWe'll do a larger refactor on master, i do prefer pass a ugly but simple fix for this regression"}}