Api Method "deleteItemsByTagsAll()" removes unrelated items #713

di-rect · 2019-11-28T20:48:02Z

When You add multiple items that have their own tag and also share a tag it seems that when you delete an item using deleteItemsByTagsAll(['sharedTag', 'ownTag']) that all items that share 'sharedTag' are also removed.

Following the docs it should only remove items that have multiple tags or all you set for this function, not one or more like deleteItemsByTags() does.

This happens for me on Couchbase buckets.

To Reproduce
Steps to reproduce the behavior:

Save multiple items with a shared tag and their own tag (like ID)
Remove one of the items by it's IDtag and the sharedTag using deleteItemsByTagsAll(['sharedTag', 'ownTag'])

So 10 items should be in total 21 => 10 items, 10 unique tags items, 1 shared tag item

Expected behavior
One item removed and one (unique) tag, so 2 items less in the bucket and the rest should stay

Current behavior
All items that share the shared tags are removed, so also are their individual tags.

The text was updated successfully, but these errors were encountered:

Geolim4 · 2019-11-29T16:32:15Z

Hello,

I don't have much time actually, can you give me a sample of code so I can test it quickly ?

Cheers,
Georges

di-rect · 2019-11-30T00:06:23Z

Hello,

I don't have much time actually, can you give me a sample of code so I can test it quickly ?

Cheers,
Georges

Hi,

Not by hand but I think your increment, or one of the other examples, fits. I see if I can make one out of it.

Regards,

Geolim4 · 2019-12-02T09:40:08Z

Hmmm I think I know why we have this kind of behavior:

Item A with tags "shared"
Item B with tags "shared, custom"

I check if the item's tags are all effectively in the deleteItemsByTagsAll(['shared')] request. Which is the case: A and B own this tag.

But B has also an additional tag: "custom" which is ignored, why ?
If the scenario was the following:

Item A with tags "shared, shared2"
Item B with tags "shared, custom"

If deleteItemsByTagsAll(['shared, shared2')] was called, only the item A would be deleted, because B doesn't have the tag "shared2"

In a nutshell: When deleteItemsByTagsAll() is called, I'm checking for the presence of all requested tags, but I'm not checking if all the tags in the ITEM are in the REQUEST.

This strange behavior will be fixed as soon I can.
Cheers,
Georges

Geolim4 · 2019-12-02T09:41:56Z

I think that all *ItemsByTagsAll() methods are affected by this, can you confirm this behavior to me @di-rect ?

di-rect · 2019-12-02T14:22:24Z

@Geolim4 thanks for the feedback, sounds reasonable why it happened.

I cannot confirm if this happens with all *ItemsByTagsAll() I think I can test that for you but don't use them as I don't have a use-case form them in this stadium yet.

Have you seen the same behavior ?

Geolim4 · 2019-12-03T00:05:15Z

Have you seen the same behavior ?

I can't test it right now, this is the reason I'm asking you more details :)

di-rect · 2019-12-03T00:27:51Z

Have you seen the same behavior ?

I can't test it right now, this is the reason I'm asking you more details :)

I need to see if I can make an in-code test-case. Otherwise I can try to make one but I found this out in the heat of the moment as well so I keep this open and I try to test it out as well.

If I'm right I need to create one, let me try to focus on that tomorrow.

Geolim4 · 2019-12-03T23:14:35Z

In fact I'm realizing that by fixing this not "well-documented" behavior, I would introduce a massive breaking change which would go against my SEMVER commitment.

Instead I will add a new set of features in the next release of this summer called *ItemsByTagsAny()

Sorry, I can't fix this now, as it's more a not "well-documented" behavior rather than a real bug by itself.

The current behavior is:

Item1 has the following tags: shared, custom1
Item2 has the following tags: shared

Calling deleteItemsByTagsAll(['shared']); actually removes:

Item1
Item2

Why ? Because "Deleting item By All Requested Tags", here 'shared'.
Actually both items are matching this request, the behavior is indead unclear but working perfectly.
See tests/ItemTags.test.php which is testing deeply this behavior. (And fails completely if I'm fixing what you're describing:

No lets see what you describing/expecting:

The behavior would be:

Item1 has the following tags: shared, custom1
Item2 has the following tags: shared

Calling deleteItemsByTagsAll(['shared']); actually removes only item2, because:

Item1 has another tag not specified in deleteItemsByTagsAll call: custom1
Item2 is effectively removed because it has only one tag and this one is matching those passed to deleteItemsByTagsAll

So what now ?
Be patient for the V8 (around this summer), which will add *ItemsByTagsAny() support
or
Find another way to delete by tags :'(

Geolim4 · 2019-12-03T23:23:42Z

See here the list of broken tests:
https://travis-ci.org/PHPSocialNetwork/phpfastcache/jobs/620367464#L2381

I can not even "fix the tests" because this would implicitly shows that I'm introducing "incompatible breaking changes" in the middle of a minor/patch version, which is completely forbidden by SEMVER commitments.

Q: Why am I so compliant with Semver commitment ?
A: Because this is what describe the trust of developers to phpfastcache. If I'm introducing incompatible breaking changes in a "non-major" version I'm breaking the trust that users put on me.

Geolim4 · 2019-12-03T23:31:59Z

I'm not closing this bug and planned it as a feature for the V8 😄
Hope that you understand that decision.

Cheeres,
Georges

di-rect · 2019-12-04T20:18:31Z

Hi,

Thank you for the explanation and work on this, I think we have some misunderstanding.

What I see is:

The current behavior is:

Item1 has the following tags: shared, custom1
Item2 has the following tags: shared
Calling deleteItemsByTagsAll(['shared','custom1']); actually removes:

Item1
Item2

=====================================

The behavior would be:

Item1 has the following tags: shared, custom1
Item2 has the following tags: shared
Calling deleteItemsByTagsAll(['shared', 'custom1']); actually removes only item1, because:

Item1 has both tags specified in deleteItemsByTagsAll call: shared, custom1
Item2 is not removed because it has only one tag and this one is not matching all those passed to deleteItemsByTagsAll

Item 2 would have been removed when it was called with deleteItemsByTags() as that does any match.

At some point I think I understand you but at the other side maybe you explain it wrong or we are both confused ?

I hope this helps, I think it's a false positive "bug".

Regards,

Geolim4 · 2019-12-04T23:13:10Z

The current behavior is:
Item1 has the following tags: shared, custom1
Item2 has the following tags: shared
Calling deleteItemsByTagsAll(['shared','custom1']); actually removes:
Item1
Item2

Nah, this behavior is fine, I tested it yesterday, and it's being already unit-tested, see by yourself the following code:

<?php

/**
 * @author Khoa Bui (khoaofgod)  <[email protected]> https://www.phpfastcache.com
 * @author Georges.L (Geolim4)  <[email protected]>
 */

use Phpfastcache\CacheManager;
use Phpfastcache\Drivers\Files\Config as FilesConfig;
use Phpfastcache\Helper\TestHelper;

chdir(__DIR__);
require_once __DIR__ . '/../../vendor/autoload.php';
$testHelper = new TestHelper('Github issue #713 - Api Method "deleteItemsByTagsAll()" removes unrelated items');
$cacheInstance = CacheManager::getInstance('Files', new FilesConfig(['securityKey' => 'test-713']));
$cacheInstance->clear();

$item1 = $cacheInstance->getItem('item1');
$item2 = $cacheInstance->getItem('item2');

$item1->addTags(['shared', 'custom1']);
$item1->set(1337);

$item2->addTags(['shared']);
$item2->set(1337);

$cacheInstance->saveMultiple($item1, $item2);
$cacheInstance->detachAllItems();
unset($item1, $item2);
$cacheInstance->deleteItemsByTagsAll(['shared', 'custom1']);

$item1 = $cacheInstance->getItem('item1');
$item2 = $cacheInstance->getItem('item2');

var_dump($item1->isHit());
var_dump($item2->isHit());

if ($item1->isHit()) {
    $testHelper->printPassText('Item #1 is still in cache as expected.');
} else {
    $testHelper->printFailText('Item #1 is no longer in cache and so has been unexpectedly removed.');
}

$testHelper->terminateTest();

Current Output:

[PHPFASTCACHE CORE V7.1.0#11C7B17]
[PHPFASTCACHE API V2.0.4]
[PHP V7.2.5]
[Begin Test: 'Github issue #713 - Api Method "deleteItemsByTagsAll()" removes unrelated items']
---
bool(false)
bool(true)
[FAIL] Item #1 is no longer in cache and so has been unexpectedly removed.
Test duration: 0.134s

Process finished with exit code 1

As you can see, item1 has been removed while item2 still exists.

Geolim4 · 2019-12-04T23:17:42Z

(This is the test I had prepared before I realized that I would introduce a massive breaking change).

di-rect · 2019-12-05T09:50:16Z

The current behavior is:
... a lot of code ...
As you can see, item1 has been removed while item2 still exists.

In the behavior here I see all items going down to zero in my cache, which is strange. I need to investigate what goes wrong (here).

It is still a good topic to discuss :)

wolfgangvc · 2019-12-17T15:50:23Z

I think it might first be useful to have a better description of what the functions are supposed to do.

The current documentation is a bit vague and awkwardly worded

deleteItemsByTag - Removes the item from the pool by tag.
deleteItemsByTags - Removes the item from the pool by one of multiple tag names.
deleteItemsByTagsAll - Removes the item from the pool by all of multiple tag names.

As I understand their functionality they would be much better described with the following

deleteItemsByTag - Removes items from the pool that have the given tag.
deleteItemsByTags - Removes items from the pool that have at least one of the given tags.
deleteItemsByTagsAll - Removes items from the pool that have all of the given tags.

Making it clear that deleteItemsByTags is functionally the same as

foreach($tags as $tag){
    $pool->deleteItemsByTag($tag)
}

And that deleteItemsByTagsAll is functionally the same as

foreach($pool->getItems() as $item){
    foreach($tags as $tag){
        if( !in_array($tag, $items->getTags() ){
            continue 2;
        }
        $pool->deleteItem($item->getKey());
    }
}

Atleast with my testing this seems to be the case.

Geolim4 · 2019-12-22T21:44:01Z

@wolfgangvc The README already describes this behavior but maybe not enough.

Geolim4 · 2020-01-02T05:37:51Z

Hello !!

🆕 in V8: Multiple strategies ($strategy) are now supported for tagging:

TaggableCacheItemPoolInterface::TAG_STRATEGY_ONE allows you to get cache item(s) by at least ONE of the specified matching tag(s). Default behavior.
TaggableCacheItemPoolInterface::TAG_STRATEGY_ALL allows you to get cache item(s) by ALL of the specified matching tag(s) (the cache item can have additional tag(s))
TaggableCacheItemPoolInterface::TAG_STRATEGY_ONLY allows you to get cache item(s) by ONLY the specified matching tag(s) (the cache item cannot have additional tag(s))

This has been implemented as of the API v3

[BC] Removed ExtendedCacheItemPoolInterface::appendItemsByTagsAll() (replaced by strategy TaggableCacheItemPoolInterface::TAG_STRATEGY_ALL)
[BC] Removed ExtendedCacheItemPoolInterface::decrementItemsByTagsAll() (replaced by strategy TaggableCacheItemPoolInterface::TAG_STRATEGY_ALL)
[BC] Removed ExtendedCacheItemPoolInterface::deleteItemsByTagsAll() (replaced by strategy TaggableCacheItemPoolInterface::TAG_STRATEGY_ALL)
[BC] Removed ExtendedCacheItemPoolInterface::getItemsByTagsAll() (replaced by strategy TaggableCacheItemPoolInterface::TAG_STRATEGY_ALL)
[BC] Removed ExtendedCacheItemPoolInterface::incrementItemsByTagsAll() (replaced by strategy TaggableCacheItemPoolInterface::TAG_STRATEGY_ALL)
[BC] Removed ExtendedCacheItemPoolInterface::prependItemsByTagsAll() (replaced by strategy TaggableCacheItemPoolInterface::TAG_STRATEGY_ALL)
Added strategyTaggableCacheItemPoolInterface::TAG_STRATEGY_ONE usable in every **byTags** methods.
Added strategyTaggableCacheItemPoolInterface::TAG_STRATEGY_ALL usable in every **byTags** methods.
Added strategyTaggableCacheItemPoolInterface::TAG_STRATEGY_ONLY usable in every **byTags** methods.

Expect the V8 to be released by the summer 2020 :)

Cheers,
Georges

di-rect · 2020-01-30T10:00:46Z

Sorry for my late response, but this is great!

Is V8 already usable without being released ?

Thanks a lot!

Geolim4 · 2020-01-30T10:03:06Z

Sorry for my late response, but this is great!

Is V8 already usable without being released ?

Thanks a lot!

Hello, nope it's still in development status, despite the tests are ok, I would not recommend it for use in a production environment !

I still have a bunch of docs & tests to made.

di-rect · 2020-02-05T20:35:21Z

Hi,

I got mentioned by someone (that was involved here), maybe this is usable for this as well to make dev easier ? https://github.com/team-a-pro/collection

Geolim4 · 2020-02-05T21:29:10Z

See the discussion above :)

di-rect · 2020-02-05T23:32:31Z

See the discussion above :)

I know but I meant if future development makes it easier why not switch instead ? Not that it's needed, if it saves time I would say... a refactor is never a bad learning curve :)

Geolim4 · 2020-02-05T23:39:47Z

Yes it does what you meant, because the tags now supports all possible use cases. It is implemented in V8 because it's not backward compatible and would break the SemVer.

Geolim4 · 2020-02-05T23:43:23Z

All the available strategies are interfaced here:

https://github.com/PHPSocialNetwork/phpfastcache/blob/v8/lib/Phpfastcache/Core/Pool/TaggableCacheItemPoolInterface.php

di-rect · 2020-02-06T14:57:00Z

Which is great! I though I just notice you when you want to be 'lazy' in the future :)

Geolim4 self-assigned this Nov 29, 2019

Geolim4 added 6.0 6.1 7.0 7.1 [*_*] Debugging labels Nov 29, 2019

Geolim4 added the [-_-] In Process label Dec 3, 2019

Geolim4 changed the title ~~deleteItemsByTagsAll() removes unrelated items~~ Api Method "deleteItemsByTagsAll()" removes unrelated items Dec 3, 2019

Geolim4 added a commit to Geolim4/phpfastcache that referenced this issue Dec 3, 2019

Fixed PHPSocialNetwork#713

0501548

Geolim4 mentioned this issue Dec 3, 2019

Fixed #713 #715

Closed

6 tasks

Geolim4 added 8.0 :( Not fixable [*_*] Planned and removed 6.0 6.1 7.0 7.1 [*_*] Debugging [-_-] In Process labels Dec 3, 2019

Geolim4 pinned this issue Dec 3, 2019

Geolim4 added the [-_-] In Process label Dec 22, 2019

Geolim4 added a commit that referenced this issue Jan 2, 2020

Fixed #713

c75faf4

Geolim4 closed this as completed Jan 2, 2020

Geolim4 added :) Closed ^_^ Fixed and removed :( Not fixable [*_*] Planned [-_-] In Process labels Feb 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Api Method "deleteItemsByTagsAll()" removes unrelated items #713

Api Method "deleteItemsByTagsAll()" removes unrelated items #713

di-rect commented Nov 28, 2019

Geolim4 commented Nov 29, 2019

di-rect commented Nov 30, 2019

Geolim4 commented Dec 2, 2019

Geolim4 commented Dec 2, 2019

di-rect commented Dec 2, 2019

Geolim4 commented Dec 3, 2019 •

edited

Loading

di-rect commented Dec 3, 2019

Geolim4 commented Dec 3, 2019

Geolim4 commented Dec 3, 2019

Geolim4 commented Dec 3, 2019

di-rect commented Dec 4, 2019 •

edited

Loading

Geolim4 commented Dec 4, 2019

Geolim4 commented Dec 4, 2019

di-rect commented Dec 5, 2019 •

edited

Loading

wolfgangvc commented Dec 17, 2019 •

edited

Loading

Geolim4 commented Dec 22, 2019

Geolim4 commented Jan 2, 2020

di-rect commented Jan 30, 2020

Geolim4 commented Jan 30, 2020

di-rect commented Feb 5, 2020

Geolim4 commented Feb 5, 2020

di-rect commented Feb 5, 2020

Geolim4 commented Feb 5, 2020

Geolim4 commented Feb 5, 2020

di-rect commented Feb 6, 2020

Api Method "deleteItemsByTagsAll()" removes unrelated items #713

Api Method "deleteItemsByTagsAll()" removes unrelated items #713

Comments

di-rect commented Nov 28, 2019

Geolim4 commented Nov 29, 2019

di-rect commented Nov 30, 2019

Geolim4 commented Dec 2, 2019

Geolim4 commented Dec 2, 2019

di-rect commented Dec 2, 2019

Geolim4 commented Dec 3, 2019 • edited Loading

di-rect commented Dec 3, 2019

Geolim4 commented Dec 3, 2019

Geolim4 commented Dec 3, 2019

Geolim4 commented Dec 3, 2019

di-rect commented Dec 4, 2019 • edited Loading

Geolim4 commented Dec 4, 2019

Geolim4 commented Dec 4, 2019

di-rect commented Dec 5, 2019 • edited Loading

wolfgangvc commented Dec 17, 2019 • edited Loading

Geolim4 commented Dec 22, 2019

Geolim4 commented Jan 2, 2020

di-rect commented Jan 30, 2020

Geolim4 commented Jan 30, 2020

di-rect commented Feb 5, 2020

Geolim4 commented Feb 5, 2020

di-rect commented Feb 5, 2020

Geolim4 commented Feb 5, 2020

Geolim4 commented Feb 5, 2020

di-rect commented Feb 6, 2020

Geolim4 commented Dec 3, 2019 •

edited

Loading

di-rect commented Dec 4, 2019 •

edited

Loading

di-rect commented Dec 5, 2019 •

edited

Loading

wolfgangvc commented Dec 17, 2019 •

edited

Loading