BUGFIX: Flush node content caches without workspace name

[dlubitz]

As we don't want to iterate over all workspaces on flushing content caches, caused by asset changes, we add addionally node cache tags without a workspace name. So we can flush all node aggregates in all workspaces with just the node aggregate id.

[bwaidelich]

Thanks, this makes a lot of sense in general!
I'm just a bit skeptical about the nullable arguments and would suggest to remove those from the public methods again.

The other suggestions we could address in a follow up

[bwaidelich]

...change this to sth like

Suggested change

	return self::forNodeAggregate(
	$node->contentRepositoryId,
	null,
	$node->aggregateId
	);
	return new self(
	'Node_'
	. self::getHashForContentRepositoryId($contentRepositoryId)
	. '_' . $nodeAggregateId->value
	);

and we could reduce the amount of repetition, by introducing some private fromParts constructor, e.g.:

return self::fromParts(self::NODE_PREFIX, self::hashed($contentRepositoryId), $nodeAggregateId->value);

that would also make it easier to change our internal pattern because I think:
<prefix>_sha1(<workspace>@<cr>)_<aggregateId> is not safe (because the NodeAggregateId could contain underscores for example – not currently, but it is being requested)

<prefix>|sha1(<workspace>)|sha1(<cr>)|<aggregateId> would make more sense to me – even though: I don't understand why we randomly(?) use sha1 for 2 of those?

[dlubitz]

The sha1 is to reduce/fix the length of the key.

is not safe (because the NodeAggregateId could contain underscores for example

There is also no need to change this, as this is just a visual delimiter, without any meaning.

[bwaidelich]

The sha1 is to reduce/fix the length of the key

It was used to avoid special characters to render the tags inconsistent – but this should no longer be an issue with our restrictive ValueObjects.
Re length:
The max length of a cache tag is 250 characters.
SHA1 hashes are 40 characters long.
workspaceName can be up to 200 characters
cr id only 16 (i.e. hashing the cr ID increases its length)

We could just hash the complete tag, but that would obviously make it impossible to read so I would not suggest to do so.
Instead I'd say (sorry, this is not related to your change, but since we talk about it):

• Reduce the length of WorkspaceName dramatically – 200 characters is crazy
• use a delimiter that can' be part of aggregate id, workspace name or cr id
• don't hash the parts at all

There is also no need to change this, as this is just a visual delimiter, without any meaning

Well, it's not just visual, imagine a node aggregate id of foo_notAWorkspace :)

[dlubitz]

Sure, but can we move this into a dedicated issue?

[bwaidelich]

Yes, sorry ;)

[mhsdesign]

created an issue because the sha1 bugged me while debugging: #5164

[bwaidelich]

@dlubitz two more little issues – I'll push a PR-PR with some suggestions

[kitsunet]

I still like bastis comment about the bool/ second WorkspaceName but otherwise wonderful thanks for sifting through this!

[kitsunet]

For some reason I seem to have ended up on an older commit 🙈 Great