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.

Sign up for GitHub

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

Feature: Added support for Faces 4 changed xmlns namespaces #6096

Closed
wants to merge 2 commits into from
Closed

Feature: Added support for Faces 4 changed xmlns namespaces #6096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e710c9
Feature: Added support for Faces 4 changed xmlns namespaces
asbachb Jun 18, 2023
1ebff96
Refactoring: Removed `isJsf22Plus()` and `Jsf30Plus()` which can be a…
asbachb Jun 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion enterprise/web.core/src/org/netbeans/modules/web/wizards/PageIterator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,11 @@ private static boolean isJSF30(WebModule wm) {
return classpath != null && classpath.findResource("jakarta/faces/flow/Flow.class") != null; //NOI18N
}

private static boolean isJSF40(WebModule wm) {
ClassPath classpath = ClassPath.getClassPath(wm.getDocumentBase(), ClassPath.COMPILE);
return classpath != null && classpath.findResource("jakarta/faces/lifecycle/ClientWindowScoped.class") != null; //NOI18N
}

public Set<DataObject> instantiate(TemplateWizard wiz) throws IOException {
// Here is the default plain behavior. Simply takes the selected
// template (you need to have included the standard second panel
Expand Down Expand Up @@ -231,7 +236,9 @@ public Set<DataObject> instantiate(TemplateWizard wiz) throws IOException {
template = templateParent.getFileObject("JSP", "xhtml"); //NOI18N
WebModule wm = WebModule.getWebModule(df.getPrimaryFile());
if (wm != null) {
if (isJSF30(wm)) {
if (isJSF40(wm)) {
wizardProps.put("isJSF40", Boolean.TRUE);
} else if (isJSF30(wm)) {
wizardProps.put("isJSF30", Boolean.TRUE);
} else if (isJSF22(wm)) {
wizardProps.put("isJSF22", Boolean.TRUE);
Expand Down
7 changes: 4 additions & 3 deletions ...rise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/InjectCompositeComponent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.netbeans.modules.parsing.spi.ParseException;
import org.netbeans.modules.web.api.webmodule.WebModule;
import org.netbeans.modules.web.common.api.WebUtils;
import org.netbeans.modules.web.jsfapi.api.JsfVersion;
import org.netbeans.modules.web.jsfapi.api.Library;
import org.netbeans.modules.web.jsfapi.spi.LibraryUtils;
import org.netbeans.spi.editor.codegen.CodeGenerator;
Expand Down Expand Up @@ -208,14 +209,14 @@ public void run() {
//but since the library has just been created by adding an xhtml file
//to the resources/xxx/ folder we need to wait until the files
//get indexed and the library is created
final String compositeLibURL = LibraryUtils.getCompositeLibraryURL(compFolder, jsfs.isJsf22Plus());
final String compositeLibURL = LibraryUtils.getCompositeLibraryURL(compFolder, jsfs.getJsfVersion().isAtLeast(JsfVersion.JSF_2_2));
Source documentSource = Source.create(document);
ParserManager.parseWhenScanFinished(Collections.singletonList(documentSource), new UserTask() { //NOI18N
@Override
public void run(ResultIterator resultIterator) throws Exception {
Library lib = jsfs.getLibrary(compositeLibURL);
if (lib != null) {
if (!LibraryUtils.importLibrary(document, lib, prefix, jsfs.isJsf22Plus())) { //XXX: fix the damned static prefix !!!
if (!LibraryUtils.importLibrary(document, lib, prefix, jsfs.getJsfVersion())) { //XXX: fix the damned static prefix !!!
logger.log(Level.WARNING, "Cannot import composite components library {0}", compositeLibURL); //NOI18N
}
} else {
Expand Down Expand Up @@ -248,7 +249,7 @@ public void run() {
((BaseDocument) templateInstanceDoc).runAtomic(new Runnable() {
@Override
public void run() {
LibraryUtils.importLibrary(templateInstanceDoc, importsMap, jsfs.isJsf22Plus());
LibraryUtils.importLibrary(templateInstanceDoc, importsMap, jsfs.getJsfVersion());
}
});
try {
Expand Down
15 changes: 8 additions & 7 deletions enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/JsfHtmlExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.netbeans.modules.web.jsf.editor.facelets.CompositeComponentLibrary;
import org.netbeans.modules.web.jsf.editor.hints.HintsRegistry;
import org.netbeans.modules.web.jsfapi.api.DefaultLibraryInfo;
import org.netbeans.modules.web.jsfapi.api.JsfVersion;
import org.netbeans.modules.web.jsfapi.api.Library;
import org.netbeans.modules.web.jsfapi.api.LibraryComponent;
import org.netbeans.modules.web.jsfapi.api.NamespaceUtils;
Expand Down Expand Up @@ -199,10 +200,10 @@ public List<CompletionItem> completeOpenTags(CompletionContext context) {
if (declaredPrefix == null) {
//undeclared prefix, try to match with default library prefix
if (lib.getDefaultPrefix() != null && lib.getDefaultPrefix().startsWith(context.getPrefix())) {
items.addAll(queryLibrary(context, lib, lib.getDefaultPrefix(), true, jsfs.isJsf22Plus()));
items.addAll(queryLibrary(context, lib, lib.getDefaultPrefix(), true, jsfs.getJsfVersion()));
}
} else {
items.addAll(queryLibrary(context, lib, declaredPrefix, false, jsfs.isJsf22Plus()));
items.addAll(queryLibrary(context, lib, declaredPrefix, false, jsfs.getJsfVersion()));
}
}
} else {
Expand All @@ -215,7 +216,7 @@ public List<CompletionItem> completeOpenTags(CompletionContext context) {
for (Library lib : librariesSet) {
if (lib.getDefaultPrefix() != null && lib.getDefaultPrefix().equals(tagNamePrefix)) {
//match
items.addAll(queryLibrary(context, lib, tagNamePrefix, true, jsfs.isJsf22Plus()));
items.addAll(queryLibrary(context, lib, tagNamePrefix, true, jsfs.getJsfVersion()));
}
}

Expand All @@ -227,7 +228,7 @@ public List<CompletionItem> completeOpenTags(CompletionContext context) {
return Collections.emptyList();
} else {
//query the library
items.addAll(queryLibrary(context, lib, tagNamePrefix, false, jsfs.isJsf22Plus()));
items.addAll(queryLibrary(context, lib, tagNamePrefix, false, jsfs.getJsfVersion()));
}
}
}
Expand All @@ -253,11 +254,11 @@ private String getUriForPrefix(String prefix, Map<String, String> namespaces) {
return null;
}

private Collection<CompletionItem> queryLibrary(CompletionContext context, Library lib, String nsPrefix, boolean undeclared, boolean isJsf22Plus) {
private Collection<CompletionItem> queryLibrary(CompletionContext context, Library lib, String nsPrefix, boolean undeclared, JsfVersion jsfVersion) {
Collection<CompletionItem> items = new ArrayList<>();
for (LibraryComponent component : lib.getComponents()) {
if (!(component instanceof AbstractFaceletsLibrary.Function)) {
items.add(JsfCompletionItem.createTag(context.getCCItemStartOffset(), component, nsPrefix, undeclared, isJsf22Plus));
items.add(JsfCompletionItem.createTag(context.getCCItemStartOffset(), component, nsPrefix, undeclared, jsfVersion));
}
}

Expand Down Expand Up @@ -353,7 +354,7 @@ public List<CompletionItem> completeAttributeValue(CompletionContext context) {
JsfAttributesCompletionHelper.completeFaceletsFromProject(context, items, ns, openTag);

// completion for sections in cases of ui:define name attribute
JsfAttributesCompletionHelper.completeSectionsOfTemplate(context, items, ns, openTag);
JsfAttributesCompletionHelper.completeSectionsOfTemplate(context, items, ns, openTag, jsfs.getJsfVersion());

// completion for java classes in <cc:attribute type="com.example.|
JsfAttributesCompletionHelper.completeJavaClasses(context, items, ns, openTag);
Expand Down
53 changes: 28 additions & 25 deletions enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/JsfSupportImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.logging.Level;
Expand All @@ -40,6 +42,7 @@
import org.netbeans.modules.web.jsf.editor.facelets.FaceletsLibrarySupport;
import org.netbeans.modules.web.jsf.editor.index.JsfIndex;
import org.netbeans.modules.web.jsfapi.api.JsfSupport;
import org.netbeans.modules.web.jsfapi.api.JsfVersion;
import org.netbeans.modules.web.jsfapi.api.Library;
import org.netbeans.modules.web.jsfapi.api.NamespaceUtils;
import org.netbeans.modules.web.jsfapi.spi.JsfSupportProvider;
Expand All @@ -58,6 +61,21 @@ public class JsfSupportImpl implements JsfSupport {

private static final Logger LOG = Logger.getLogger(JsfSupportImpl.class.getSimpleName());

private static final Map<JSFVersion, JsfVersion> JSF_VERSION_MAPPING;
static {
Map<JSFVersion, JsfVersion> map = new HashMap<>();
map.put(JSFVersion.JSF_1_0, JsfVersion.JSF_1_0);
map.put(JSFVersion.JSF_1_1, JsfVersion.JSF_1_1);
map.put(JSFVersion.JSF_1_2, JsfVersion.JSF_1_2);
map.put(JSFVersion.JSF_2_0, JsfVersion.JSF_2_0);
map.put(JSFVersion.JSF_2_1, JsfVersion.JSF_2_1);
map.put(JSFVersion.JSF_2_2, JsfVersion.JSF_2_2);
map.put(JSFVersion.JSF_2_3, JsfVersion.JSF_2_3);
map.put(JSFVersion.JSF_3_0, JsfVersion.JSF_3_0);
map.put(JSFVersion.JSF_4_0, JsfVersion.JSF_4_0);
JSF_VERSION_MAPPING = Collections.unmodifiableMap(map);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for the two version implementations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not happy about that mapping as well.

Basically JSFVersion is not part of the api module but of implementation module web.jsf. So reusing JSFVersion would have meant either to:

  • Move it to web.jsfapi
    or
  • Add web.jsf as dependency

From my perspective it makes more sense to have it as part of the api, at least the feature set which is currently implemented. On the other hand I did not want to merge both classes as it would have created an even huger patchset than already.
So basically the idea would be to eliminate JSFVersion and integrate it to JsfVersion or somewhere else.

}

public static JsfSupportImpl findFor(Source source) {
return getOwnImplementation(JsfSupportProvider.get(source));
}
Expand Down Expand Up @@ -171,22 +189,18 @@ public void propertyChange(PropertyChangeEvent evt) {
}
});

if(isJsf30Plus()){
webBeansModelJakarta = new org.netbeans.modules.jakarta.web.beans.MetaModelSupport(project).getMetaModel();
} else {
webBeansModel = new org.netbeans.modules.web.beans.MetaModelSupport(project).getMetaModel();
}

//init lookup
//TODO do it lazy so it creates the web beans model lazily once looked up
InstanceContent ic = new InstanceContent();
if(isJsf30Plus()){
if(getJsfVersion().isAtLeast(JsfVersion.JSF_3_0)){
webBeansModelJakarta = new org.netbeans.modules.jakarta.web.beans.MetaModelSupport(project).getMetaModel();
ic.add(webBeansModelJakarta);
} else {
webBeansModel = new org.netbeans.modules.web.beans.MetaModelSupport(project).getMetaModel();
ic.add(webBeansModel);
}
this.lookup = new AbstractLookup(ic);

this.lookup = new AbstractLookup(ic);
}

@Override
Expand Down Expand Up @@ -268,25 +282,14 @@ public synchronized MetadataModel<org.netbeans.modules.jakarta.web.beans.api.mod
}

@Override
public boolean isJsf22Plus() {
if (wm != null) {
JSFVersion version = JSFVersion.forWebModule(wm);
// caching is done inside the method
return version != null && version.isAtLeast(JSFVersion.JSF_2_2);
public JsfVersion getJsfVersion() {
if (wm == null) {
return JsfVersion.latest();
}
// return the latest supported one until somebody will complain about that
return true;
}

@Override
public boolean isJsf30Plus() {
if (wm != null) {
JSFVersion version = JSFVersion.forWebModule(wm);
// caching is done inside the method
return version != null && version.isAtLeast(JSFVersion.JSF_3_0);
}
// return the latest supported one until somebody will complain about that
return true;
JSFVersion jsfVersion = JSFVersion.forWebModule(wm);

return JSF_VERSION_MAPPING.getOrDefault(jsfVersion, JsfVersion.latest());
}

@Override
Expand Down
10 changes: 4 additions & 6 deletions enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/JsfUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.netbeans.modules.parsing.api.Snapshot;
import org.netbeans.modules.parsing.spi.ParseException;
import org.netbeans.modules.parsing.spi.Parser.Result;
import org.netbeans.modules.web.jsfapi.api.JsfVersion;
import org.netbeans.modules.web.jsfapi.api.LibraryInfo;

/**
Expand Down Expand Up @@ -99,12 +100,9 @@ public static Result getEmbeddedParserResult(ResultIterator resultIterator, Stri
return null;
}

public static Node getRoot(HtmlParserResult parserResult, LibraryInfo library) {
Node rootNode = parserResult.root(library.getNamespace());
if ((rootNode == null || rootNode.children().isEmpty()) && library.getLegacyNamespace() != null) {
rootNode = parserResult.root(library.getLegacyNamespace());
}
return rootNode;
public static Node getRoot(HtmlParserResult parserResult, LibraryInfo library, JsfVersion jsfVersion) {
String namespace = jsfVersion.getNamespaceUri(library.getDefaultPrefix());
return parserResult.root(namespace);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this changes the semantic. The way this is written allows using both namespaces, while the new code only allows the "right" one. I would assume, that JSF implementations will recognize also old namespaces and I think the old behavior was sane.

You then can also keep the signature to its original form.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding the old behavior only supported one namespace at a time as well.

e.g. when you create a new maven web application, add a new jsf site and add an tag which namespace is not yet present and you hit auto import you only can choose between the namespace of the used jsf version.

From my understanding if we want to provide all compatible namespaces for a jsf implementation this would be an feature which is currently not present.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I entered this into a facelets file:

<f:view
	xmlns="http://www.w3.org/1999/xhtml"
	xmlns:c="http://java.sun.com/jsp/jstl/core"
	xmlns:f="http://java.sun.com/jsf/core"
	xmlns:h="http://java.sun.com/jsf/html"
	xmlns:p="http://primefaces.org/ui"
	xmlns:ui="http://java.sun.com/jsf/facelets">
	
	<h:head>
	</h:head>
	<h:body>
            <ui:define name="|" />
        </h:body>
</f:view>

I called completion at position marked with "|". At that point JsfUtils#getRoot is called with the DefaultLibraryInfo FACELETS. In my exampled I used the legacy namespace. Because the original method checks both namespaces it finds the f:view element, with your change that would have not happenend and an empty pseudo node had been returned.

So sorry, but your change introduces a bug, you need to scan all possible namespaces for the libraries.

}

}
82 changes: 42 additions & 40 deletions ...e/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/actions/FixNamespacesAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import javax.swing.SwingUtilities;
import javax.swing.text.JTextComponent;
import org.netbeans.api.editor.EditorActionRegistration;
import org.netbeans.api.progress.ProgressUtils;
import org.netbeans.api.progress.BaseProgressUtils;
import org.netbeans.editor.BaseAction;
import static org.netbeans.editor.BaseAction.MAGIC_POSITION_RESET;
import static org.netbeans.editor.BaseAction.UNDO_MERGE_RESET;
Expand Down Expand Up @@ -83,53 +83,55 @@ public FixNamespacesAction() {

@Override
public void actionPerformed(ActionEvent evt, final JTextComponent target) {
if (target != null) {
final AtomicBoolean cancel = new AtomicBoolean();
final AtomicReference<ImportData> importData = new AtomicReference<>();
final UserTask task = new UserTask() {
@Override
public void run(ResultIterator ri) throws Exception {
Parser.Result parserResult = getHtmlParserResult(ri);
if (parserResult instanceof HtmlParserResult) {
HtmlParserResult htmlParserResult = (HtmlParserResult) parserResult;
if (cancel.get()) {
return;
}
if (target == null) {
return;
}

final ImportData data = computeNamespaces(htmlParserResult);
if (cancel.get()) {
return;
}
if (data.shouldShowNamespacesPanel) {
if (!cancel.get()) {
importData.set(data);
}
} else {
performFixNamespaces(htmlParserResult, data, data.getDefaultVariants(), isRemoveUnusedNs());
final AtomicBoolean cancel = new AtomicBoolean();
final AtomicReference<ImportData> importData = new AtomicReference<>();
final UserTask task = new UserTask() {
@Override
public void run(ResultIterator ri) throws Exception {
Parser.Result parserResult = getHtmlParserResult(ri);
if (parserResult instanceof HtmlParserResult) {
HtmlParserResult htmlParserResult = (HtmlParserResult) parserResult;
if (cancel.get()) {
return;
}

final ImportData data = computeNamespaces(htmlParserResult);
if (cancel.get()) {
return;
}
if (data.shouldShowNamespacesPanel) {
if (!cancel.get()) {
importData.set(data);
}
} else {
performFixNamespaces(htmlParserResult, data, data.getDefaultVariants(), isRemoveUnusedNs());
}
}
};
}
};

ProgressUtils.runOffEventDispatchThread(new Runnable() {
BaseProgressUtils.runOffEventDispatchThread(new Runnable() {
@Override
public void run() {
try {
ParserManager.parse(Collections.singleton(Source.create(target.getDocument())), task);
} catch (ParseException ex) {
Exceptions.printStackTrace(ex);
}
}
}, Bundle.FixNamespacesLabelLongName(), cancel, false);

if (importData.get() != null && !cancel.get()) {
SwingUtilities.invokeLater(new Runnable() {
@Override
public void run() {
try {
ParserManager.parse(Collections.singleton(Source.create(target.getDocument())), task);
} catch (ParseException ex) {
Exceptions.printStackTrace(ex);
}
showFixNamespacesDialog(target, importData.get());
}
}, Bundle.FixNamespacesLabelLongName(), cancel, false);

if (importData.get() != null && !cancel.get()) {
SwingUtilities.invokeLater(new Runnable() {
@Override
public void run() {
showFixNamespacesDialog(target, importData.get());
}
});
}
});
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...eb.jsf.editor/src/org/netbeans/modules/web/jsf/editor/actions/FixNamespacesPerformer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ private void removeUnusedNamespaces() {

private void includeMissingNamespaces() {
for (VariantItem variant : selections) {
LibraryUtils.importLibrary(baseDocument, variant.getLibrary(), variant.getPrefix(), importData.isJsf22);
LibraryUtils.importLibrary(baseDocument, variant.getLibrary(), variant.getPrefix(), importData.jsfVersion);
}
}
}
3 changes: 2 additions & 1 deletion enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/actions/ImportData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.List;
import java.util.Objects;
import org.netbeans.modules.html.editor.lib.api.elements.Attribute;
import org.netbeans.modules.web.jsfapi.api.JsfVersion;
import org.netbeans.modules.web.jsfapi.api.Library;

/**
Expand All @@ -31,7 +32,7 @@
public class ImportData {

public volatile boolean shouldShowNamespacesPanel;
public volatile boolean isJsf22;
public volatile JsfVersion jsfVersion;

private final List<DataItem> dataItems = new ArrayList<>();
private final List<DataItem> dataItemsToReplace = new ArrayList<>();
Expand Down
Loading