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/02/23 14:05:16 UTC

[myfaces] branch main updated: MYFACES-4560 refactored

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 6b473c8d0 MYFACES-4560 refactored
6b473c8d0 is described below

commit 6b473c8d0c57e127ecbde858af6eaf02f9877ab4
Author: Thomas Andraschko <ta...@apache.org>
AuthorDate: Thu Feb 23 15:05:09 2023 +0100

    MYFACES-4560 refactored
---
 .../core/api/shared/SelectItemsIterator.java       | 36 +++-----------
 .../myfaces/core/api/shared/SelectItemsUtil.java   | 24 ++++------
 .../apache/myfaces/core/api/shared/VarUtils.java   | 56 ++++++++++++++++++++++
 .../renderkit/html/util/SelectItemsUtils.java      | 24 +++-------
 4 files changed, 79 insertions(+), 61 deletions(-)

diff --git a/api/src/main/java/org/apache/myfaces/core/api/shared/SelectItemsIterator.java b/api/src/main/java/org/apache/myfaces/core/api/shared/SelectItemsIterator.java
index 1a61df7ad..5a76e3e3f 100644
--- a/api/src/main/java/org/apache/myfaces/core/api/shared/SelectItemsIterator.java
+++ b/api/src/main/java/org/apache/myfaces/core/api/shared/SelectItemsIterator.java
@@ -206,49 +206,27 @@ public class SelectItemsIterator implements Iterator<SelectItem>
         if (_nestedItems != null)
         {
             Object item = _nestedItems.next();
-            
 
             // check new params of SelectItems (since 2.0): itemValue, itemLabel, itemDescription,...
             // Note that according to the spec UISelectItems does not provide Getter and Setter
             // methods for this values, so we have to use the attribute map
             Map<String, Object> attributeMap = _currentUISelectItems.getAttributes();
 
-            // write the current item into the request map under the key listed in var, if available
-            boolean wroteRequestMapVarValue = false;
-            Object oldRequestMapVarValue = null;
             String var = (String) attributeMap.get(SelectItemsUtil.ATTR_VAR);
-            if (var != null && !var.isEmpty())
-            {
-                // save the current value of the key listed in var from the request map
-                oldRequestMapVarValue = _facesContext.getExternalContext().getRequestMap().put(var, item);
-                wroteRequestMapVarValue = true;
-            }
 
-            if (!(item instanceof SelectItem))
-            {
-                _currentValue = item;
-                item = SelectItemsUtil.createSelectItem(_currentUISelectItems, item, SelectItem::new);
-            }
-            else
+            return VarUtils.executeInScope(_facesContext, var, item, () ->
             {
-                item = SelectItemsUtil.updateSelectItem(_currentUISelectItems, (SelectItem) item);
-            }
-
-            // remove the value with the key from var from the request map, if previously written
-            if (wroteRequestMapVarValue)
-            {
-                // If there was a previous value stored with the key from var in the request map, restore it
-                if (oldRequestMapVarValue != null)
+                if (item instanceof SelectItem)
                 {
-                    _facesContext.getExternalContext().getRequestMap().put(var, oldRequestMapVarValue);
+                    _currentValue = null;
+                    return SelectItemsUtil.updateSelectItem(_currentUISelectItems, (SelectItem) item);
                 }
                 else
                 {
-                    _facesContext.getExternalContext().getRequestMap().remove(var);
+                    _currentValue = item;
+                    return SelectItemsUtil.createSelectItem(_currentUISelectItems, item, SelectItem::new);
                 }
-            }
-
-            return (SelectItem) item;
+            });
         }
         throw new NoSuchElementException();
     }
diff --git a/api/src/main/java/org/apache/myfaces/core/api/shared/SelectItemsUtil.java b/api/src/main/java/org/apache/myfaces/core/api/shared/SelectItemsUtil.java
index 03018a8e8..08bb2c949 100644
--- a/api/src/main/java/org/apache/myfaces/core/api/shared/SelectItemsUtil.java
+++ b/api/src/main/java/org/apache/myfaces/core/api/shared/SelectItemsUtil.java
@@ -187,19 +187,13 @@ public class SelectItemsUtil
  
         CollectionUtils.forEach(values, value ->
         {
-            Object oldValue = context.getExternalContext().getRequestMap().put(var, value);
-            
-            callback.accept(
-                    createSelectItem(component, getItemValue(attributes, value), supplier));
-
-            if (oldValue != null)
-            {
-                context.getExternalContext().getRequestMap().put(var, oldValue);
-            }
-            else
+            VarUtils.executeInScope(context, var, value, () ->
             {
-                context.getExternalContext().getRequestMap().remove(var);
-            }
+                callback.accept(
+                        createSelectItem(component, getItemValue(attributes, value), supplier));
+
+                return null;
+            });
         });
     }
     
@@ -237,7 +231,7 @@ public class SelectItemsUtil
             }
             else
             {
-                Object itemValue = _convertOrCoerceValue(context, uiComponent, value, item, converter);
+                Object itemValue = convertOrCoerceValue(context, uiComponent, value, item, converter);
                 if (value == itemValue || value.equals(itemValue))
                 {
                     return true;
@@ -276,7 +270,7 @@ public class SelectItemsUtil
             }
             else if (item.isNoSelectionOption())
             {
-                Object itemValue = _convertOrCoerceValue(context, uiComponent, value, item, converter);
+                Object itemValue = convertOrCoerceValue(context, uiComponent, value, item, converter);
                 if (value == itemValue || value.equals(itemValue))
                 {
                     return true;
@@ -290,7 +284,7 @@ public class SelectItemsUtil
      * If converter is available and selectItem.value is String uses getAsObject,
      * otherwise uses EL type coertion and return result.
      */
-    private static Object _convertOrCoerceValue(FacesContext facesContext,
+    private static Object convertOrCoerceValue(FacesContext facesContext,
             UIComponent uiComponent, Object value, SelectItem selectItem, Converter converter)
     {
         Object itemValue = selectItem.getValue();
diff --git a/api/src/main/java/org/apache/myfaces/core/api/shared/VarUtils.java b/api/src/main/java/org/apache/myfaces/core/api/shared/VarUtils.java
new file mode 100644
index 000000000..b585f9070
--- /dev/null
+++ b/api/src/main/java/org/apache/myfaces/core/api/shared/VarUtils.java
@@ -0,0 +1,56 @@
+/*
+ * 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.core.api.shared;
+
+import jakarta.faces.context.FacesContext;
+import java.util.Map;
+import java.util.function.Supplier;
+
+public class VarUtils
+{
+    public static <T> T executeInScope(FacesContext context, String var, Object value, Supplier<T> callback)
+    {
+        if (var == null || var.isBlank())
+        {
+            return callback.get();
+        }
+
+        Map<String, Object> requestMap = context.getExternalContext().getRequestMap();
+
+        // save the current value of the key listed in var from the request map
+        Object oldValue = requestMap.remove(var);
+        try
+        {
+            // write the current item into the request map under the key listed in var, if available
+            requestMap.put(var, value);
+
+            return callback.get();
+        }
+        finally
+        {
+            // remove the value with the key from var from the request map, if previously written
+            requestMap.remove(var);
+            if (oldValue != null)
+            {
+                // If there was a previous value stored with the key from var in the request map, restore it
+                requestMap.put(var, oldValue);
+            }
+        }
+    }
+}
diff --git a/impl/src/main/java/org/apache/myfaces/renderkit/html/util/SelectItemsUtils.java b/impl/src/main/java/org/apache/myfaces/renderkit/html/util/SelectItemsUtils.java
index 8c0e7be39..02a0d7f61 100644
--- a/impl/src/main/java/org/apache/myfaces/renderkit/html/util/SelectItemsUtils.java
+++ b/impl/src/main/java/org/apache/myfaces/renderkit/html/util/SelectItemsUtils.java
@@ -120,8 +120,7 @@ public class SelectItemsUtils
             else
             {
                 String itemStrValue = org.apache.myfaces.core.api.shared.SharedRendererUtils
-                        .getConvertedStringValue(context, component, converter,
-                                selectItem);
+                        .getConvertedStringValue(context, component, converter, selectItem);
                 boolean selected = lookupSet.contains(itemStrValue); 
                 //TODO/FIX: we always compare the String values, better fill lookupSet with Strings 
                 //only when useSubmittedValue==true, else use the real item value Objects
@@ -183,7 +182,6 @@ public class SelectItemsUtils
                 }
 
                 String labelClass = null;
-
                 if (componentDisabled || disabled)
                 {
                     labelClass = (String) component.getAttributes().get(ComponentAttrs.DISABLED_CLASS_ATTR);
@@ -198,23 +196,15 @@ public class SelectItemsUtils
                 }
 
                 boolean escape = AttributeUtils.getBooleanAttribute(component, ComponentAttrs.ESCAPE_ATTR, false);
-                //default is to escape
-                //In Faces 1.2, when a SelectItem is created by default 
-                //selectItem.isEscape() returns true (this property
-                //is not available on Faces 1.1).
-                //so, if we found a escape property on the component
-                //set to true, escape every item, but if not
-                //check if isEscape() = true first.
+                // default is to escape
+                // In Faces 1.2, when a SelectItem is created, by default selectItem.isEscape() returns true
+                //(this propertyis not available on Faces 1.1).
+                // so, if we found a escape property on the component set to true, escape every item,
+                // but if not check if isEscape() = true first.
                 if (escape || selectItem.isEscape())
                 {
                     String label = selectItem.getLabel();
-
-                    if(label == null)
-                    {
-                        label = "";
-                    }
-
-                    writer.writeText(label, null);
+                    writer.writeText(label == null ? "" : label, null);
                 }
                 else
                 {