-
Notifications
You must be signed in to change notification settings - Fork 874
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
Added support for JSF 4.0 namespaces #6160
Added support for JSF 4.0 namespaces #6160
Conversation
Will be glad to give it a spin, once a built version is available. I hope this solved taglib 4.0 files issue as well. |
The maven code might need some improvement. I'm not 100% sure if it respects the local maven settings (proxy, central repository override for company environments). |
NB should offer access to its configured maven or at least the settings for it, no? |
@morvael Indeed, but I'm not 100% sure I got the correct API.
|
@morvael Can you be a little bit more specific what you're missing? |
re taglib I have my own taglib registering a converter. It works as long as the declaration is set to:
It stops working when switched to
Even though it's just like described in https://jakarta.ee/xml/ns/jakartaee/web-facelettaglibrary_4_0.xsd Tag turns red and attributes are no longer recognizable, and the xhtml file using the tag is highlighted with red icon. |
There's also this logged in the IDE log, might not be because of your change, but since it mentions something Jsf related I thought I'll include it: Not sure if this is related to this one following it: |
@morvael May I ask you to provide an example project? Would be easier for me to figure out what's going on with that. |
I'll try to make one. Give me some time. |
So, going with maven project then, at least there wizard can be completed. |
Lol, default maven web project does not compile as it pulls so old war plugin that it crashes without reflective access to some tree map comparator :) Switching project to use JDK11. |
I'm not the right person to be commenting on much of the JSF stuff, but this statement stands out. There may be other reasons for preferring the approach here, but if the right thing to do is to require Java 11 then that's the thing to do. |
I slightly agree, but from my understanding this won't be fixed soon (see #4904). Actually I think to use the matching JSF implementation does make much more sense, as things like attribute completion suggestions or helper text relies on that. From my - limit - understanding I'd prefer to remove that bundled JSF implementation at all, but currently it's quite tightly coupled with the module. |
That's a known issue which is - afaik - fixed master. My branch is not rebased to the latest master branch. To get it work with JDK 17 just update |
ddb8fbf
to
69d750b
Compare
Crazy, after I made this project both 2.2 and 4.0 taglib declarations stopped to be recognized (both in sample project and my core project). But the project works once deployed. |
We have a few modules that require Java 11 - there are ways and means to work around this prior to all tests being fixed - if that is the right approach for this. |
I have upgraded plugin versions and switched it to use JDK_17, so here is latest version that works in vanilla NB 18 (so without taglib declaration using version 4.0): Wildfly seems to be pretty forgiving, and accepts old namespaces etc - it's just NB that isn't developer friendly. |
69d750b
to
6d1e186
Compare
@morvael Just pushed some fixes. Custom taglib should work now with
I don't think that's fair. If you want to improve the situation get behind the code and start hacking ;) |
Tbh I already recognized that as well, but haven't had the time to fix it.
So the idea would be:
3.) I have not tested actively. |
c787881
to
629cc68
Compare
Pushed some more adjustment which
Just wanted to note: That this patchset might not be the silver bullet to support |
629cc68
to
ae0859a
Compare
@asbachb sorry, but I have to further things: I still see jakarta composite libraries offered to me by CC. I looked into this an indeed they are generated independently of the Align `LibraryUtils#getCompositeLibraryURL` and `LibraryUtils#getAllCompositeLibraryNamespaces`# This patch file was generated by NetBeans IDE
# It uses platform neutral UTF-8 encoding and \n newlines.
--- a/enterprise/web.jsfapi/src/org/netbeans/modules/web/jsfapi/spi/LibraryUtils.java
+++ b/enterprise/web.jsfapi/src/org/netbeans/modules/web/jsfapi/spi/LibraryUtils.java
@@ -71,10 +71,14 @@
}
}
- public static Set<String> getAllCompositeLibraryNamespaces(String libraryName) {
+ public static Set<String> getAllCompositeLibraryNamespaces(String libraryName, JsfVersion jsfVersion) {
Set<String> namespaces = new LinkedHashSet<>();
- namespaces.add(COMPOSITE_LIBRARY_JAKARTA_NS + "/" + libraryName);
- namespaces.add(COMPOSITE_LIBRARY_JCP_NS + "/" + libraryName);
+ if (jsfVersion.isAtLeast(JsfVersion.JSF_4_0)) {
+ namespaces.add(COMPOSITE_LIBRARY_JAKARTA_NS + "/" + libraryName);
+ }
+ if(jsfVersion.isAtLeast(JsfVersion.JSF_2_2)) {
+ namespaces.add(COMPOSITE_LIBRARY_JCP_NS + "/" + libraryName);
+ }
namespaces.add(COMPOSITE_LIBRARY_SUN_NS + "/" + libraryName);
return namespaces;
}
--- a/enterprise/web.jsfapi/nbproject/org-netbeans-modules-web-jsfapi.sig
+++ b/enterprise/web.jsfapi/nbproject/org-netbeans-modules-web-jsfapi.sig
@@ -279,7 +279,7 @@
meth public static java.lang.String getCompositeLibraryURL(java.lang.String,org.netbeans.modules.web.jsfapi.api.JsfVersion)
meth public static java.util.Map<java.lang.String,org.netbeans.modules.web.jsfapi.api.Library> getDeclaredLibraries(org.netbeans.modules.html.editor.lib.api.HtmlParsingResult)
meth public static java.util.Map<org.netbeans.modules.web.jsfapi.api.Library,java.lang.String> importLibrary(javax.swing.text.Document,java.util.Map<org.netbeans.modules.web.jsfapi.api.Library,java.lang.String>)
-meth public static java.util.Set<java.lang.String> getAllCompositeLibraryNamespaces(java.lang.String)
+meth public static java.util.Set<java.lang.String> getAllCompositeLibraryNamespaces(java.lang.String,org.netbeans.modules.web.jsfapi.api.JsfVersion)
meth public static org.netbeans.api.project.Project[] getOpenedJSFProjects()
supr java.lang.Object I also noticed, that the order of the namespaces did not seem consistent. I think the problem is in Suggested adjustment to `JsfNamespaceComparator.java`--- a/enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/facelets/JsfNamespaceComparator.java
+++ b/enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/facelets/JsfNamespaceComparator.java
@@ -37,7 +37,12 @@
@Override
public int compare(String namespace1, String namespace2) {
- return rate(namespace1).compareTo(rate(namespace2));
+ int prefixResult = rate(namespace1).compareTo(rate(namespace2));
+ if(prefixResult != 0) {
+ return prefixResult;
+ } else {
+ return namespace1.compareTo(namespace2);
+ }
}
private Integer rate(String namespace) { |
Think I got it. |
ae0859a
to
8def2b9
Compare
@matthiasblaesing Applied suggested fixes. |
Before: After: The critical parts:
|
@asbachb as I feared, in offline mode this was problematic. I removed the The first issue is, that the check if the maven artifact is present is not correct/incomplete. The maven resolver gives back a path, but the path point to a non-existing file. I got this exception: Exception
I suggest to adjust the contract of the provider to require the provider to only return valid paths. Suggested fix--- a/enterprise/web.jsfapi/src/org/netbeans/modules/web/jsfapi/spi/JsfReferenceImplementationProvider.java
+++ b/enterprise/web.jsfapi/src/org/netbeans/modules/web/jsfapi/spi/JsfReferenceImplementationProvider.java
@@ -27,5 +27,11 @@
*/
public interface JsfReferenceImplementationProvider {
+ /**
+ * Determine the path to the JSF reference implementation JAR.
+ *
+ * @param jsfVersion
+ * @return path to the JAR or {@code null} if not found
+ */
Path artifactPathFor(JsfVersion jsfVersion);
}
--- a/enterprise/maven.j2ee/src/org/netbeans/modules/maven/j2ee/MavenJsfReferenceImplementationProvider.java
+++ b/enterprise/maven.j2ee/src/org/netbeans/modules/maven/j2ee/MavenJsfReferenceImplementationProvider.java
@@ -18,11 +18,12 @@
*/
package org.netbeans.modules.maven.j2ee;
+import java.nio.file.Files;
import java.nio.file.Path;
-import java.nio.file.Paths;
import java.util.Collections;
import java.util.EnumMap;
import java.util.Map;
+import java.util.Optional;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.repository.ArtifactRepository;
import org.apache.maven.artifact.resolver.ArtifactNotFoundException;
@@ -82,6 +83,10 @@
return null;
}
- return Paths.get(jsfRIArtifact.getFile().toURI());
+ return Optional.ofNullable(jsfRIArtifact)
+ .map(artifact -> artifact.getFile())
+ .map(file -> file.toPath())
+ .filter(file -> Files.exists(file))
+ .orElse(null);
}
}
The second issue is that the schemas in the reference implementation JARs are not optimal. They contain this: <xsd:import namespace="http://www.w3.org/XML/1998/namespace"
schemaLocation="http://www.w3.org/2001/xml.xsd"/> The schema parse will then try to load the XSD from network. This is a privacy and performance problem. NetBeans has a solution for this: We carry the schemas and can use the NetBeans Exception
Suggested fix# This patch file was generated by NetBeans IDE
# It uses platform neutral UTF-8 encoding and \n newlines.
--- a/enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/facelets/mojarra/ConfigManager.java
+++ b/enterprise/web.jsf.editor/src/org/netbeans/modules/web/jsf/editor/facelets/mojarra/ConfigManager.java
@@ -68,6 +68,8 @@
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
+import java.io.Reader;
+import java.io.StringWriter;
import java.lang.annotation.Annotation;
import java.lang.ref.WeakReference;
import java.net.MalformedURLException;
@@ -120,9 +122,13 @@
import javax.xml.transform.stream.StreamSource;
import javax.xml.validation.Schema;
import javax.xml.validation.SchemaFactory;
+import org.netbeans.api.xml.services.UserCatalog;
import org.netbeans.modules.web.jsf.editor.facelets.DefaultFaceletLibraries;
import org.netbeans.modules.web.jsfapi.api.JsfNamespaces;
+import org.openide.util.Exceptions;
import org.w3c.dom.*;
+import org.w3c.dom.ls.LSInput;
+import org.w3c.dom.ls.LSResourceResolver;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;
import org.xml.sax.SAXParseException;
@@ -1180,6 +1186,31 @@
WeakReference<Schema> schema = SCHEMA_CACHE.get(id);
if (schema == null || schema.get() == null) {
SchemaFactory schemaFactory = SchemaFactory.newDefaultInstance();
+ schemaFactory.setResourceResolver(new LSResourceResolver() {
+ @Override
+ public LSInput resolveResource(String type, String namespaceURI, String publicId, String systemId, String baseURI) {
+ try {
+ InputSource is = UserCatalog.getDefault().
+ getEntityResolver().
+ resolveEntity(publicId, systemId);
+ if (is != null) {
+ return new LSInputFromInputSource(is);
+ }
+ } catch (SAXException | IOException ex) {
+ LOGGER.log(
+ Level.FINE,
+ "Failed to resolve namespaceURI: {}, publicId: {}, systemId: {}, baseURI: {}",
+ new Object[] {
+ namespaceURI,
+ publicId,
+ systemId,
+ baseURI
+ }
+ );
+ }
+ return null;
+ }
+ });
schema = new WeakReference<>(schemaFactory.newSchema(jsfRIClassLoader.getResource(schemaResourceName)));
SCHEMA_CACHE.put(id, schema);
@@ -1345,4 +1376,96 @@
} // END URITask
+ /**
+ * Helperclass to supply the SchemaFactory with XSDs from the NB catalog
+ */
+ private static class LSInputFromInputSource implements LSInput {
+
+ private final InputSource is;
+
+ public LSInputFromInputSource(InputSource is) {
+ this.is = is;
+ }
+
+ @Override
+ public Reader getCharacterStream() {
+ return is.getCharacterStream();
+ }
+
+ @Override
+ public void setCharacterStream(Reader characterStream) {
+ }
+
+ @Override
+ public InputStream getByteStream() {
+ return is.getByteStream();
+ }
+
+ @Override
+ public void setByteStream(InputStream byteStream) {
+ }
+
+ @Override
+ public String getStringData() {
+ try (Reader r = getCharacterStream()) {
+ if (r == null) {
+ return null;
+ }
+ StringWriter sw = new StringWriter();
+ getCharacterStream().transferTo(sw);
+ return sw.toString();
+ } catch (IOException ex) {
+ throw new RuntimeException(ex);
+ }
+ }
+
+ @Override
+ public void setStringData(String stringData) {
+ }
+
+ @Override
+ public String getSystemId() {
+ return is.getSystemId();
+ }
+
+ @Override
+ public void setSystemId(String systemId) {
+ }
+
+ @Override
+ public String getPublicId() {
+ return is.getPublicId();
+ }
+
+ @Override
+ public void setPublicId(String publicId) {
+ }
+
+ @Override
+ public String getBaseURI() {
+ return "";
+ }
+
+ @Override
+ public void setBaseURI(String baseURI) {
+ }
+
+ @Override
+ public String getEncoding() {
+ return is.getEncoding();
+ }
+
+ @Override
+ public void setEncoding(String encoding) {
+ }
+
+ @Override
+ public boolean getCertifiedText() {
+ return false;
+ }
+
+ @Override
+ public void setCertifiedText(boolean certifiedText) {
+ }
+ }
} |
8def2b9
to
67cc090
Compare
@matthiasblaesing applied fixes (slightly adjusted stream expressions) Note: Based on the Stack it seems that this exception is not coming from the standard taglibs, but from primefaces: |
Looks sane to me, thanks. I'll try to have another look at this tomorrow, but if noone objects (@pepness, @juneau001, @jGauravGupta), and I don't hit another road block, I'll merge on sunday. |
@asbachb small merge conflict to resolve if you have time (without github ui please) |
Previously NetBeans bundles the JSF reference implementation Mojarra in order to provide several functionality (like namespaces) based on the parsed taglib xml files from that jar. This is somehow problematic as: 1) The dependency needs to be updated on every new JSF release 2) NetBeans needs to fulfil the expectations of Mojarra This leads to the problem that we currently cannot update and bundle the latest Mojarra version as Java 11 is required. This change wants to get less dependant on that bundled JSF implementation by resolving the reference implementation using maven. Instead of reading the taglib xml files from that bundled jar NetBeans now gets in from the local maven repository or downloads it from maven central. If this is not possible we fallback to the bundled version. This change also improves the situation when autosuggestion is invoked in the namespace part of JSF xhtml documents by ordering the suggested namespace with the one on top which is most likely to be used. This should fix apache#6069 apache#5470 apache#5470 apache#4338
67cc090
to
14fee17
Compare
please let me know when and how to test this promising release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be still problem be lurking, but I think, that the potential benefit trumps potential problems. Thank you for working through this and staying with it @asbachb.
Lets get this in.
I'm gonna buy you some coffees for that @asbachb, many thanks! |
JSF 4.0 now operational in NB 19 ! |
I think the best would be to open up a new issue. I'm currently blocked with personal stuff so I won't be able to provide any more fixes for now.
11 Sept 2023 21:25:13 Louis Collet ***@***.***>:
…
JSF 4.0 now operational in NB 19 !
Many thanks to @asbachb[https://github.com/asbachb]
a little detail : its does not detect the xmlns which are not used in the .xhtm file
Have a nice day
Louis
—
Reply to this email directly, view it on GitHub[#6160 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AALKAEHDO7JINGJQLL7MSLLXZ5QRRANCNFSM6AAAAAA2BJDWDI].
You are receiving this because you were mentioned.
[Tracking image][https://github.com/notifications/beacon/AALKAEGY6TNCR3BGJM6IZ23XZ5QRRA5CNFSM6AAAAAA2BJDWDKWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTGGB6JY.gif]
|
I think this might be the cause for issues: #7449, #7436, #7354. I can't reproduce the issues, but my gut feeling is, that maven embedder loading causes class loading with the wrong classloader. My idea would be to pull out the places that trigger potential class loading: Lines 334 to 361 in b332b29
out into Would you mind having a look? |
Previously NetBeans bundles the JSF reference implementation Mojarra in order to provide several functionality
(like namespaces) based on the parsed taglib xml files from that jar. This is somehow problematic as:
This leads to the problem that we currently cannot update and bundle the latest Mojarra version as
Java 11 is required.
This change wants to get less dependant on that bundled JSF implementation by resolving the reference implementation
using maven. Instead of reading the taglib xml files from that bundled jar NetBeans now gets in from the local maven
repository or downloads it from maven central. If this is not possible we fallback to the bundled version.
This change also improves the situation when autosuggestion is invoked in the namespace part of JSF xhtml documents
by ordering the suggested namespace with the one on top which is most likely to be used.