You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by ta...@apache.org on 2023/11/03 10:27:53 UTC

(myfaces) branch main updated: MYFACES-4636 also reverted some changes; JVM optimizes this already and it makes it just less readable

This is an automated email from the ASF dual-hosted git repository.

tandraschko pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/myfaces.git


The following commit(s) were added to refs/heads/main by this push:
     new 816d4ae3b MYFACES-4636 also reverted some changes; JVM optimizes this already and it makes it just less readable
816d4ae3b is described below

commit 816d4ae3b3d677752a33569231507a0cd7130c6b
Author: Thomas Andraschko <ta...@apache.org>
AuthorDate: Fri Nov 3 11:27:44 2023 +0100

    MYFACES-4636
    also reverted some changes; JVM optimizes this already and it makes it just less readable
---
 .../renderkit/html/util/AjaxScriptBuilder.java     | 155 ++++++++++-----------
 .../renderkit/html/util/AjaxScriptBuilderTest.java |  93 +++++++++++++
 .../apache/myfaces/test/mock/MockApplication.java  |   2 +-
 .../myfaces/test/mock/MockApplication23.java       |  51 +++++++
 4 files changed, 216 insertions(+), 85 deletions(-)

diff --git a/impl/src/main/java/org/apache/myfaces/renderkit/html/util/AjaxScriptBuilder.java b/impl/src/main/java/org/apache/myfaces/renderkit/html/util/AjaxScriptBuilder.java
index 5980da08d..d4c45627d 100644
--- a/impl/src/main/java/org/apache/myfaces/renderkit/html/util/AjaxScriptBuilder.java
+++ b/impl/src/main/java/org/apache/myfaces/renderkit/html/util/AjaxScriptBuilder.java
@@ -38,9 +38,6 @@ public class AjaxScriptBuilder
 
     private static final String AJAX_PARAM_SB = "oam.renderkit.AJAX_PARAM_SB";
 
-    private static final String QUOTE = "'";
-    private static final String BLANK = " ";
-
     private static final String AJAX_KEY_ONERROR = "onerror";
     private static final String AJAX_KEY_ONEVENT = "onevent";
     private static final String AJAX_KEY_DELAY = "delay";
@@ -49,30 +46,22 @@ public class AjaxScriptBuilder
     private static final String AJAX_VAL_THIS = "this";
     private static final String AJAX_VAL_EVENT = "event";
 
-    private static final String COLON = ":";
-    private static final String COMMA = ",";
-
-    private static final String L_PAREN = "(";
-    private static final String R_PAREN = ")";
-
-    private static final String L_C_BRACE = "{";
-    private static final String R_C_BRACE = "}";
     public static final String AJAX_KEY_PARAMS = "params";
     public static final String AJAX_VAL_NULL = "null";
     public static final String MYFACES_AB = "myfaces.ab";
 
 
     public static void build(FacesContext context,
-            StringBuilder sb,
-            UIComponent component,
-            String sourceId,
-            String eventName,
-            String execute,
-            String render,
-            Boolean resetValues,
-            String onerror,
-            String onevent,
-            List<UIParameter> uiParams)
+                             StringBuilder sb,
+                             UIComponent component,
+                             String sourceId,
+                             String eventName,
+                             String execute,
+                             String render,
+                             Boolean resetValues,
+                             String onerror,
+                             String onevent,
+                             List<UIParameter> uiParams)
     {
         build(context,
                 sb,
@@ -88,30 +77,30 @@ public class AjaxScriptBuilder
                 null,
                 uiParams);
     }
-    
+
     public static void build(FacesContext context,
-            StringBuilder sb,
-            UIComponent component,
-            String sourceId,
-            String eventName,
-            Collection<String> executeList,
-            Collection<String> renderList,
-            String delay,
-            boolean resetValues,
-            String onerror,
-            String onevent,
-            Collection<ClientBehaviorContext.Parameter> params)
+                             StringBuilder sb,
+                             UIComponent component,
+                             String sourceId,
+                             String eventName,
+                             Collection<String> executeList,
+                             Collection<String> renderList,
+                             String delay,
+                             boolean resetValues,
+                             String onerror,
+                             String onevent,
+                             Collection<ClientBehaviorContext.Parameter> params)
     {
         String execute = null;
         if (executeList != null && !executeList.isEmpty())
         {
-            execute = String.join(BLANK, executeList);
+            execute = String.join(" ", executeList);
         }
-            
+
         String render = null;
         if (renderList != null && !renderList.isEmpty())
         {
-            render = String.join(BLANK, renderList);
+            render = String.join(" ", renderList);
         }
 
         build(context,
@@ -128,27 +117,28 @@ public class AjaxScriptBuilder
                 params,
                 null);
     }
-    
+
     public static void build(FacesContext context,
-            StringBuilder sb,
-            UIComponent component,
-            String sourceId,
-            String eventName,
-            String execute,
-            String render,
-            String delay,
-            String resetValues,
-            String onerror,
-            String onevent,
-            Collection<ClientBehaviorContext.Parameter> params,
-            List<UIParameter> uiParams)
+                             StringBuilder sb,
+                             UIComponent component,
+                             String sourceId,
+                             String eventName,
+                             String execute,
+                             String render,
+                             String delay,
+                             String resetValues,
+                             String onerror,
+                             String onevent,
+                             Collection<ClientBehaviorContext.Parameter> params,
+                             List<UIParameter> uiParams)
     {
-    // CHECKSTYLE:ON
+        // CHECKSTYLE:ON
         HtmlCommandScript commandScript = (component instanceof HtmlCommandScript)
                 ? (HtmlCommandScript) component
                 : null;
-   
-        sb.append(MYFACES_AB +L_PAREN);
+
+        sb.append(MYFACES_AB);
+        sb.append('(');
 
         if (sourceId == null)
         {
@@ -156,9 +146,9 @@ public class AjaxScriptBuilder
         }
         else
         {
-            sb.append(QUOTE);
+            sb.append('\'');
             sb.append(sourceId);
-            sb.append(QUOTE);
+            sb.append('\'');
 
             if (!sourceId.trim().equals(component.getClientId(context)))
             {
@@ -181,14 +171,14 @@ public class AjaxScriptBuilder
                 }
             }
         }
-        sb.append(COMMA);
+        sb.append(',');
 
         sb.append(commandScript == null ? AJAX_VAL_EVENT : AJAX_VAL_NULL);
-        sb.append(COMMA+QUOTE);
+        sb.append(",'");
 
         sb.append(eventName);
-        sb.append(QUOTE+COMMA);
-        
+        sb.append("',");
+
         SearchExpressionHandler seHandler = null;
         SearchExpressionContext seContext = null;
         if (StringUtils.isNotBlank(execute) || StringUtils.isNotBlank(render))
@@ -200,14 +190,14 @@ public class AjaxScriptBuilder
         }
 
         appendIds(sb, execute, seHandler, seContext);
-        sb.append(COMMA);
+        sb.append(',');
 
         appendIds(sb, render, seHandler, seContext);
-        
+
         if (onevent != null || onerror != null || delay != null || resetValues != null
                 || (params != null && !params.isEmpty()) || (uiParams != null && !uiParams.isEmpty()))
         {
-            sb.append(COMMA+L_C_BRACE);
+            sb.append(",{");
             if (onevent != null)
             {
                 appendProperty(sb, AJAX_KEY_ONEVENT, onevent, false);
@@ -228,7 +218,7 @@ public class AjaxScriptBuilder
             if ((params != null && !params.isEmpty()) || (uiParams != null && !uiParams.isEmpty()))
             {
                 StringBuilder paramsBuilder = SharedStringBuilder.get(context, AJAX_PARAM_SB, 60);
-                paramsBuilder.append(L_C_BRACE);
+                paramsBuilder.append('{');
                 if (params != null && !params.isEmpty())
                 {
                     if (params instanceof RandomAccess)
@@ -257,22 +247,21 @@ public class AjaxScriptBuilder
                         appendProperty(paramsBuilder, param.getName(), param.getValue(), true);
                     }
                 }
-                paramsBuilder.append(R_C_BRACE);
-                sb.append(AJAX_KEY_PARAMS+COLON);
-                sb.append(paramsBuilder);
+                paramsBuilder.append('}');
+                appendProperty(sb, AJAX_KEY_PARAMS, paramsBuilder, false);
             }
 
-            sb.append(R_C_BRACE);
+            sb.append('}');
         }
 
-        sb.append(R_PAREN);
+        sb.append(')');
     }
-    
+
     private static void appendIds(StringBuilder sb, String expressions,
-            SearchExpressionHandler handler, SearchExpressionContext searchExpressionContext)
+                                  SearchExpressionHandler handler, SearchExpressionContext searchExpressionContext)
     {
-        sb.append(QUOTE);
-        
+        sb.append('\'');
+
         if (StringUtils.isNotBlank(expressions))
         {
             List<String> clientIds =
@@ -284,18 +273,18 @@ public class AjaxScriptBuilder
                 {
                     if (i > 0)
                     {
-                        sb.append(BLANK);
+                        sb.append(" ");
                     }
                     sb.append(clientIds.get(i));
                 }
             }
         }
-        
-        sb.append(QUOTE);
+
+        sb.append('\'');
     }
 
 
-    public static void appendProperty(StringBuilder builder, 
+    public static void appendProperty(StringBuilder builder,
                                       String name,
                                       Object value,
                                       boolean quoteValue)
@@ -306,26 +295,24 @@ public class AjaxScriptBuilder
         }
 
         char lastChar = builder.charAt(builder.length() - 1);
-        if (!COMMA.equals(String.valueOf(lastChar)) && !L_C_BRACE.equals(String.valueOf(lastChar)))
+        if (',' != lastChar && '{' != lastChar)
         {
-            builder.append(COMMA);
+            builder.append(',');
         }
 
-        builder.append(QUOTE);
+        builder.append('\'');
         builder.append(name);
-        builder.append(QUOTE);
-        
-        builder.append(COLON);
+        builder.append("':");
 
         if (value == null)
         {
-            builder.append(QUOTE+QUOTE);
+            builder.append("''");
         }
         else if (quoteValue)
         {
-            builder.append(QUOTE);
+            builder.append('\'');
             builder.append(value);
-            builder.append(QUOTE);
+            builder.append('\'');
         }
         else
         {
diff --git a/impl/src/test/java/org/apache/myfaces/renderkit/html/util/AjaxScriptBuilderTest.java b/impl/src/test/java/org/apache/myfaces/renderkit/html/util/AjaxScriptBuilderTest.java
new file mode 100644
index 000000000..735878f29
--- /dev/null
+++ b/impl/src/test/java/org/apache/myfaces/renderkit/html/util/AjaxScriptBuilderTest.java
@@ -0,0 +1,93 @@
+/*
+ * Copyright 2007 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.myfaces.renderkit.html.util;
+
+import jakarta.faces.FactoryFinder;
+import jakarta.faces.component.UIComponentMock;
+import jakarta.faces.component.behavior.ClientBehaviorContext;
+import jakarta.faces.context.FacesContext;
+import org.apache.myfaces.component.search.*;
+import org.apache.myfaces.core.api.shared.lang.Assert;
+import org.apache.myfaces.test.base.junit.AbstractJsfTestCase;
+import org.apache.myfaces.test.mock.MockApplication;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+public class AjaxScriptBuilderTest extends AbstractJsfTestCase
+{
+    @BeforeEach
+    @Override
+    public void setUp() throws Exception
+    {
+        super.setUp();
+
+        FactoryFinder.setFactory(FactoryFinder.SEARCH_EXPRESSION_CONTEXT_FACTORY,
+                SearchExpressionContextFactoryImpl.class.getName());
+
+        MockApplication mockApplication = (MockApplication) FacesContext.getCurrentInstance().getApplication();
+        mockApplication.setSearchExpressionHandler(new SearchExpressionHandlerImpl());
+
+        CompositeSearchKeywordResolver baseResolver = new CompositeSearchKeywordResolver();
+        baseResolver.add(new ThisSearchKeywordResolver());
+        baseResolver.add(new ParentSearchKeywordResolver());
+        baseResolver.add(new ChildSearchKeywordResolver());
+        baseResolver.add(new CompositeComponentParentSearchKeywordResolver());
+        baseResolver.add(new FormSearchKeywordResolver());
+        baseResolver.add(new NamingContainerSearchKeywordResolver());
+        baseResolver.add(new NextSearchKeywordResolver());
+        baseResolver.add(new NoneSearchKeywordResolver());
+        baseResolver.add(new PreviousSearchKeywordResolver());
+        baseResolver.add(new RootSearchKeywordResolver());
+        baseResolver.add(new IdSearchKeywordResolver());
+        baseResolver.add(new AllSearchKeywordResolver());
+        mockApplication.setSearchKeywordResolver(baseResolver);
+    }
+
+    @Test
+    public void test()
+    {
+        UIComponentMock component = new UIComponentMock();
+        component.setId("test");
+
+        List<ClientBehaviorContext.Parameter> params = new ArrayList<>();
+        params.add(new ClientBehaviorContext.Parameter("var1", "NEW VALUE"));
+
+        StringBuilder sb = new StringBuilder();
+        AjaxScriptBuilder.build(FacesContext.getCurrentInstance(),
+                sb,
+                component,
+                "j_id_i",
+                "click",
+                Arrays.asList("@this"),
+                null,
+                null,
+                false,
+                null,
+                "testJs",
+                params);
+
+        String script = sb.toString();
+
+        Assertions.assertFalse(script.contains("':testJsparams:{"));
+        Assertions.assertTrue(script.contains("':testJs,'params':{"));
+        Assertions.assertEquals("myfaces.ab('j_id_i',event,'click','test','',{'onevent':testJs,'params':{'var1':'NEW VALUE'}})", script);
+    }
+}
diff --git a/test/src/main/java/org/apache/myfaces/test/mock/MockApplication.java b/test/src/main/java/org/apache/myfaces/test/mock/MockApplication.java
index ce915a2ff..ca4a201f3 100644
--- a/test/src/main/java/org/apache/myfaces/test/mock/MockApplication.java
+++ b/test/src/main/java/org/apache/myfaces/test/mock/MockApplication.java
@@ -27,7 +27,7 @@ package org.apache.myfaces.test.mock;
  * @since 1.0.0
  *
  */
-public class MockApplication extends MockApplication22
+public class MockApplication extends MockApplication23
 {
  
 
diff --git a/test/src/main/java/org/apache/myfaces/test/mock/MockApplication23.java b/test/src/main/java/org/apache/myfaces/test/mock/MockApplication23.java
new file mode 100644
index 000000000..ada12e1ae
--- /dev/null
+++ b/test/src/main/java/org/apache/myfaces/test/mock/MockApplication23.java
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.myfaces.test.mock;
+
+import jakarta.faces.component.search.SearchExpressionHandler;
+import jakarta.faces.component.search.SearchKeywordResolver;
+
+public class MockApplication23 extends MockApplication22
+{
+    private SearchExpressionHandler searchExpressionHandler;
+    private SearchKeywordResolver searchKeywordResolver;
+
+    @Override
+    public SearchExpressionHandler getSearchExpressionHandler()
+    {
+        return searchExpressionHandler;
+    }
+
+    @Override
+    public void setSearchExpressionHandler(SearchExpressionHandler searchExpressionHandler)
+    {
+        this.searchExpressionHandler = searchExpressionHandler;
+    }
+
+    @Override
+    public SearchKeywordResolver getSearchKeywordResolver()
+    {
+        return searchKeywordResolver;
+    }
+
+    public void setSearchKeywordResolver(SearchKeywordResolver searchKeywordResolver)
+    {
+        this.searchKeywordResolver = searchKeywordResolver;
+    }
+}