Useless HashSet in DefaultFaceletContext

[SuperPat45]

I think there is a mistake in /impl/src/main/java/com/sun/faces/facelets/impl/DefaultFaceletContext.java in the apply method we can see:

        private final Set<String> names = new HashSet<>();

        // ....

        @Override
        public boolean apply(FaceletContext ctx, UIComponent parent, String name)
        throws IOException {

            String testName = (name != null) ? name : "facelets._NULL_DEF_";
            if (this.names.contains(testName)) {
                return false;
            } else {
                this.names.add(testName);
                boolean found = this.target.apply(new DefaultFaceletContext(
                        (DefaultFaceletContext) ctx, this.owner), parent, name);
                this.names.remove(testName);
                return found;
            }
        }

You have a set of string for containing ui:insert name having no composition Handlers. But as the element is always removed, I think this set is therefore useless as it is used anywhere else and it may be removed.

[cmunden]

Difficult to say if this is actually useless as TemplateManager follows the Composite pattern. TemplateManager is a TemplateClient that has a TemplateClient (referenced by this.target).

As such, one could produce strange family trees where parents can be their own parents, parents can be their own children, children can be the parents of their own grand parents, etc. Without strict rules about the tree's descendants and ancestors lineage, these situations could exist.

It appears to me that the list of names is being used as a safeguard to prevent OutOfMemory and StackOverFlow exceptions that could occur as a result of infinite recursion. Or it could be used to ensure that the apply method is only executed effectively once during a complete traversal of the composite tree.

But again... it is hard to say for sure without knowing the author's original intent.

[edburns]

Please see this important message regarding community contributions to Mojarra.

https://javaee.groups.io/g/jsf-spec/message/30

Also, please consider joining that group, as that group has taken the place of the old [email protected] mailing list.

Thanks,

Ed Burns

[SuperPat45]

Ok cmunde, I don' think about recursion although a Stack OverFlow would not bother me in this case as this would prevent there is a problem