remove_tag unable to remove a parent ResourceView class without intermediate save

[alchzh]

What is the problem? (Here is where you provide a complete Traceback.)

Given a resource tagged with BaseTag and ChildTag inheriting from BaseTag, a component cannot remove BaseTag class using resource.remove_tag(BaseTag) without first calling resource.remove_tag(ChildTag) and then await resource.save()

If you've discovered it, what is the root cause of the problem?

• Calling only resource.remove_tag(BaseTag) will leave BaseTag on the resource because it's the base class of ChildTag
• Calling both resource.remove_tag(BaseTag) and resource.remove_tag(ChildTag) will cause a KeyError when the resource modify diffs are committed because set.remove() is called with a tag that is already removed in the same update cycle.

What are the steps to reproduce the issue?

Minimal example:

class BaseTag(ResourceView):
    ...

class ChildTag(BaseTag):
    ...


class RemoveOnlyBase(Modifier):
    targets = tuple()

    async def modify(self, resource: Resource, config = None):
        resource.remove_tag(BaseTag)

class RemoveBoth(Modifier):
    targets = tuple()

    async def modify(self, resource: Resource, config = None):
        resource.remove_tag(ChildTag)
        resource.remove_tag(BaseTag)

class RemoveBothWithSave(Modifier):
    targets = tuple()

    async def modify(self, resource: Resource, config = None):
        resource.remove_tag(ChildTag)
        await resource.save()
        resource.remove_tag(BaseTag)

async def main(ofrak_context: OFRAKContext):
    root_resource = await ofrak_context.create_root_resource("root", b"")

    child_resource = await root_resource.create_child((BaseTag, ChildTag))
    await child_resource.run(RemoveOnlyBase)
    print(child_resource.get_tags()) # prints {BaseTag, ChildTag}

    child_resource = await root_resource.create_child((BaseTag, ChildTag))
    await child_resource.run(RemoveBothWithSave)
    print(child_resource.get_tags()) # prints set(), successfully removed BaseTag

    child_resource = await root_resource.create_child((BaseTag, ChildTag))
    await child_resource.run(RemoveBoth)
    print(child_resource.get_tags()) # crashes with KeyError

$ python3 remove_tag_bug.py 
Using OFRAK Community License.
{BaseTag, ChildTag}
set()
It took 0.002 seconds to run the OFRAK script
Traceback (most recent call last):
  File "remove_tag_bug.py", line 53, in <module>
    ofrak.run(main)
  File "/tmp/rbs-ofrak/ofrak/ofrak_core/ofrak/ofrak_context.py", line 206, in run
    asyncio.get_event_loop().run_until_complete(self.run_async(func, *args))
  File "/usr/local/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/tmp/rbs-ofrak/ofrak/ofrak_core/ofrak/ofrak_context.py", line 199, in run_async
    await func(ofrak_context, *args)
  File "remove_tag_bug.py", line 46, in main
    await child_resource.run(RemoveBoth)
  File "/tmp/rbs-ofrak/ofrak/ofrak_core/ofrak/resource.py", line 388, in run
    component_result = await self._job_service.run_component(
  File "/tmp/rbs-ofrak/ofrak/ofrak_core/ofrak/service/job_service.py", line 199, in run_component
    raise result
  File "/tmp/rbs-ofrak/ofrak/ofrak_core/ofrak/service/job_service.py", line 133, in _run_component
    result = await component.run(
  File "/tmp/rbs-ofrak/ofrak/ofrak_core/ofrak/component/abstract.py", line 164, in run
    await self._save_resources(
  File "/tmp/rbs-ofrak/ofrak/ofrak_core/ofrak/component/abstract.py", line 202, in _save_resources
    await save_resources(
  File "/tmp/rbs-ofrak/ofrak/ofrak_core/ofrak/resource.py", line 1567, in save_resources
    await resource_service.update_many(diffs)
  File "/tmp/rbs-ofrak/ofrak/ofrak_core/ofrak/service/resource_service.py", line 903, in update_many
    return [self._update(resource_diff) for resource_diff in resource_diffs]
  File "/tmp/rbs-ofrak/ofrak/ofrak_core/ofrak/service/resource_service.py", line 903, in <listcomp>
    return [self._update(resource_diff) for resource_diff in resource_diffs]
  File "/tmp/rbs-ofrak/ofrak/ofrak_core/ofrak/service/resource_service.py", line 917, in _update
    self._remove_resource_tag_from_index(tag_removed, resource_node, set(current_tags))
  File "/tmp/rbs-ofrak/ofrak/ofrak_core/ofrak/service/resource_service.py", line 691, in _remove_resource_tag_from_index
    self._tag_indexes[_tag].remove(resource)
KeyError: <ofrak.service.resource_service.ResourceNode object at 0x7f397adfb940>

How would you implement this fix?

I'm unsure of the intended behavior in these cases. It seems unintuitive that the RemoveOnlyBase modifier doesn't actually remove the BaseTag tag and RemoveBoth crashes. If requiring the pattern in RemoveBothWithSave is intended behavior, this should be clarified in the docs and descriptive exception raised instead of the cryptic KeyError.

Replacing self._tag_indexes[_tag].remove(resource) with self._tag_indexes[_tag].discard(resource) will get rid of the KeyError exception, but I'm unsure if this is the correct fix since it could hide other bugs.

[rbs-jacob]

My guess is that both RemoveOnlyBase and RemoveBoth in your minimal working example are behaving incorrectly. I think it's fairly uncommon to remove tags in OFRAK scripts that we use, so I doubt that anything relies on this behavior. Probably this is just a less-used part of the resource service, and the bug(s) have been latent for a while.

As a first effort towards fixing this, we should at least switch .remove to .discard as you suggest, and see if it causes any tests to fail.

[rbs-jacob]

The PR addressing part of this issue was merged, but let's leave this issue open for now.