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

childrenOfPath exclusion now obeys wildcard paths for exceptions. #371

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

justinlaster
Copy link

When an include predicate uses a childrenOfPath exclude statement, exceptions would not evaluate Unicorn wildcard paths. This has been updated to use a path segment comparison to ensure that any configured unicorn, wildcard paths get correctly identified as an exception.

Example CoP configuration:

			  <include name="Data" database="master" path="/sitecore/content/MyTenant/MySite/Data">
				  <exclude childrenOfPath="*">
					  <except name="Component Library"></except>
				  </exclude>
			  </include>

  • Added two new methods to the PathTool class to provide consistency with path comparisons.
  • Item path escaping rules for Sitecore wildcard items have been "inversed" in the ChildrenOfPathBasedPresetTreeExclusion class for exceptions. This was done since it was possible to define a configuration where path comparisons would previously incorrectly match against Sitecore wildcard items.

When an include predicate uses a childrenOfPath exclude statement,
exceptions would not evaluate Unicorn wildcard paths. This has been updated to
use a path segment comparison to ensure that any configured unicorn,
wildcard paths get correctly identified as an exception.

Example CoP configuration:

			  <include name="Data" database="master" path="/sitecore/content/MyTenant/MySite/Data">
				  <exclude childrenOfPath="*">
					  <except name="Component Library"></except>
				  </exclude>
			  </include>

* Added two new methods to the PathTool class to provide consistency
with path comparisons.
* Item path escaping rules for Sitecore wildcard items have been
"inversed" in the ChildrenOfPathBasedPresetTreeExclusion class for
exceptions. This was done since it was possible to define a configuration where path
comparisons would previously incorrectly match against Sitecore wildcard
items.
@cassidydotdk
Copy link
Member

Thank you for this :-)

However. Looking through what we have, would it not make more sense to hook in this <except> extension to the already existing pattern <exclude>?

<!-- 
	NAME PATTERN: Exclude items whose name matches a regex pattern
	NOTE: children of items whose name matches the pattern will also be excluded
	NOTE: regex is case-insensitive (as Sitecore names are case-insensitive)
-->
<include name="Name pattern" database="master" path="/sitecore/namepattern">
	<exclude namePattern="^__Standard values$" />
</include>

Currently, this namePattern exclude will not allow any <except> under it. But your code was tagged onto this instead of a new wildcard (that can't really do more complicated matching like a regex can), we stay truer to form.

Or am I completely overlooking something while my brain cooks away in "Sunday mode"? :D

Ideally, the test config should be expanded to this:

<!-- 
	NAME PATTERN: Exclude items whose name matches a regex pattern
	NOTE: children of items whose name matches the pattern will also be excluded
	NOTE: regex is case-insensitive (as Sitecore names are case-insensitive)
-->
<include name="Name pattern" database="master" path="/sitecore/namepattern">
	<exclude namePattern="^__Standard values$" />
	<exclude namePattern="^.*$">
                <except name="Test Include Children 1" />
                <except name="Skeleton Folder 1" includeChildren="false" />
	</exclude>
</include>

@justinlaster
Copy link
Author

justinlaster commented Nov 10, 2019

While working through the motivation that lead to this PR, I did also "discover" that namePattern statements don't have any support for except; glad to know that it's not such a strange idea after all!

But the <exclude childrenOfPath> clause already accepts and uses wildcards, while supporting except statements, allowing you to combine the two. However, the code is doing two different forms of path analysis which leads to some functionality gaps. Unless I'm missing something on Sunday morning, I think this is a straight forward fix for existing include, exclude, and except constructs :D.

I'll definitely volunteer to add except functionality in namePattern statements in a second PR! I think both additions are in order.

@cassidydotdk
Copy link
Member

Guess we'll both have to muddle through :D

But you are exactly right in that there is nothing strange in the idea. I've had lots of requests recently to expand upon these, to allow for more complex including/excluding. I've just had precious little time to give it much consideration the past couple of months. I really appreciate any contributions here.

For now, let me see if I can get a test case up and around what you have here. The include/exclude code has quite many execution paths and will send my mind on a spin if I don't have supporting test cases around it.

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

Successfully merging this pull request may close these issues.

None yet

3 participants