From a983e27b3ad93a1219083831bc546801180d98eb Mon Sep 17 00:00:00 2001 From: pizzi80 Date: Sat, 17 Dec 2022 18:46:25 +0100 Subject: [PATCH] BUG inside restoreState: Object[].length is 2! Generics Cosmetics Signed-off-by: pizzi80 --- .../tag/composite/BehaviorHolderWrapper.java | 2 +- .../faces/component/StateHolderSaver.java | 13 +- .../jakarta/faces/component/UIComponent.java | 2 +- .../faces/component/UIComponentBase.java | 138 ++++++++---------- .../java/jakarta/faces/render/RenderKit.java | 4 +- .../com/sun/faces/mock/MockRenderKit.java | 17 +-- 6 files changed, 77 insertions(+), 99 deletions(-) diff --git a/impl/src/main/java/com/sun/faces/facelets/tag/composite/BehaviorHolderWrapper.java b/impl/src/main/java/com/sun/faces/facelets/tag/composite/BehaviorHolderWrapper.java index a0f89fe76d..4dc817e297 100644 --- a/impl/src/main/java/com/sun/faces/facelets/tag/composite/BehaviorHolderWrapper.java +++ b/impl/src/main/java/com/sun/faces/facelets/tag/composite/BehaviorHolderWrapper.java @@ -413,7 +413,7 @@ protected FacesListener[] getFacesListeners(Class clazz) { } @Override - protected Renderer getRenderer(FacesContext context) { + protected Renderer getRenderer(FacesContext context) { return null; } diff --git a/impl/src/main/java/jakarta/faces/component/StateHolderSaver.java b/impl/src/main/java/jakarta/faces/component/StateHolderSaver.java index f30aa39f47..ee07d34db9 100644 --- a/impl/src/main/java/jakarta/faces/component/StateHolderSaver.java +++ b/impl/src/main/java/jakarta/faces/component/StateHolderSaver.java @@ -17,6 +17,7 @@ package jakarta.faces.component; import java.io.Serializable; +import java.lang.reflect.InvocationTargetException; import jakarta.faces.context.FacesContext; @@ -100,7 +101,7 @@ public StateHolderSaver(FacesContext context, Object toSave) { public Object restore(FacesContext context) throws IllegalStateException { Object result = null; - Class toRestoreClass; + Class toRestoreClass; // if the Object to save implemented Serializable but not // StateHolder @@ -124,11 +125,11 @@ public Object restore(FacesContext context) throws IllegalStateException { if (null != toRestoreClass) { try { - result = toRestoreClass.newInstance(); - } catch (InstantiationException e) { + result = toRestoreClass.getDeclaredConstructor().newInstance(); + } catch (InstantiationException | IllegalAccessException | NoSuchMethodException e) { throw new IllegalStateException(e); - } catch (IllegalAccessException a) { - throw new IllegalStateException(a); + } catch (InvocationTargetException e) { + throw new RuntimeException(e); } } @@ -141,7 +142,7 @@ public Object restore(FacesContext context) throws IllegalStateException { return result; } - private static Class loadClass(String name, Object fallbackClass) throws ClassNotFoundException { + private static Class loadClass(String name, Object fallbackClass) throws ClassNotFoundException { ClassLoader loader = Thread.currentThread().getContextClassLoader(); if (loader == null) { loader = fallbackClass.getClass().getClassLoader(); diff --git a/impl/src/main/java/jakarta/faces/component/UIComponent.java b/impl/src/main/java/jakarta/faces/component/UIComponent.java index 6697ac0608..d60f03ff9e 100644 --- a/impl/src/main/java/jakarta/faces/component/UIComponent.java +++ b/impl/src/main/java/jakarta/faces/component/UIComponent.java @@ -1981,7 +1981,7 @@ public void processEvent(ComponentSystemEvent event) throws AbortProcessingExcep * @param context {@link FacesContext} for the current request * @return the renderer, or null. */ - protected abstract Renderer getRenderer(FacesContext context); + protected abstract Renderer getRenderer(FacesContext context); // --------------------------------------------------------- Package Private diff --git a/impl/src/main/java/jakarta/faces/component/UIComponentBase.java b/impl/src/main/java/jakarta/faces/component/UIComponentBase.java index 80ef190176..a8115ff32f 100644 --- a/impl/src/main/java/jakarta/faces/component/UIComponentBase.java +++ b/impl/src/main/java/jakarta/faces/component/UIComponentBase.java @@ -41,20 +41,8 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.util.AbstractCollection; -import java.util.AbstractMap; -import java.util.AbstractSet; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.ListIterator; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.NoSuchElementException; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.logging.Level; @@ -73,7 +61,6 @@ import jakarta.faces.event.ComponentSystemEventListener; import jakarta.faces.event.FacesEvent; import jakarta.faces.event.FacesListener; -import jakarta.faces.event.PhaseId; import jakarta.faces.event.PostAddToViewEvent; import jakarta.faces.event.PostValidateEvent; import jakarta.faces.event.PreRemoveFromViewEvent; @@ -102,7 +89,7 @@ public abstract class UIComponentBase extends UIComponent { // -------------------------------------------------------------- Attributes - private static Logger LOGGER = Logger.getLogger("jakarta.faces.component", "jakarta.faces.LogStrings"); + private static final Logger LOGGER = Logger.getLogger("jakarta.faces.component", "jakarta.faces.LogStrings"); private static final String ADDED = UIComponentBase.class.getName() + ".ADDED"; @@ -131,7 +118,7 @@ public abstract class UIComponentBase extends UIComponent { * An EMPTY_OBJECT_ARRAY argument list to be passed to reflection methods. *

*/ - private static final Object EMPTY_OBJECT_ARRAY[] = new Object[0]; + private static final Object[] EMPTY_OBJECT_ARRAY = new Object[0]; /** *

@@ -201,7 +188,7 @@ public Map getPassThroughAttributes(boolean create) { Map passThroughAttributes = (Map) this.getStateHelper().get(PropertyKeys.passThroughAttributes); if (passThroughAttributes == null && create) { - passThroughAttributes = new PassThroughAttributesMap<>(); + passThroughAttributes = new PassThroughAttributesMap(); getStateHelper().put(PropertyKeys.passThroughAttributes, passThroughAttributes); } @@ -239,7 +226,7 @@ public String getClientId(FacesContext context) { } // Allow the renderer to convert the clientId - Renderer renderer = getRenderer(context); + Renderer renderer = getRenderer(context); if (renderer != null) { clientId = renderer.convertClientId(context, clientId); } @@ -263,7 +250,7 @@ public void setId(String id) { // if the current ID is not null, and the passed // argument is the same, no need to validate it // as it has already been validated. - if (this.id == null || !this.id.equals(id)) { + if (!Objects.equals(this.id, id)) { validateId(id); this.id = id; } @@ -306,7 +293,7 @@ public void setParent(UIComponent parent) { @Override public boolean isRendered() { - return Boolean.valueOf(getStateHelper().eval(PropertyKeys.rendered, TRUE).toString()); + return Boolean.parseBoolean(getStateHelper().eval(PropertyKeys.rendered, TRUE).toString()); } @Override @@ -495,7 +482,7 @@ public void decode(FacesContext context) { String rendererType = getRendererType(); if (rendererType != null) { - Renderer renderer = getRenderer(context); + Renderer renderer = (Renderer) getRenderer(context); if (renderer != null) { renderer.decode(context, this); } else { @@ -1038,9 +1025,9 @@ protected FacesContext getFacesContext() { } @Override - protected Renderer getRenderer(FacesContext context) { + protected Renderer getRenderer(FacesContext context) { - Renderer renderer = null; + Renderer renderer = null; String rendererType = getRendererType(); if (rendererType != null) { @@ -1194,25 +1181,19 @@ public Object saveState(FacesContext context) { @Override public void restoreState(FacesContext context, Object state) { - - if (context == null) { - throw new NullPointerException(); - } - - if (state == null) { - return; - } + Objects.requireNonNull(context); // NPE + if (state == null) return; // skip restore Object[] values = (Object[]) state; - if (values[0] != null) { + if ( values.length > 0 && values[0] != null) { if (listeners == null) { listeners = new AttachedObjectListHolder<>(); } listeners.restoreState(context, values[0]); } - if (values[1] != null) { + if (values.length > 1 && values[1] != null) { Map, List> restoredListeners = restoreSystemEventListeners(context, values[1]); if (listenersByEventClass != null) { listenersByEventClass.putAll(restoredListeners); @@ -1221,15 +1202,15 @@ public void restoreState(FacesContext context, Object state) { } } - if (values[2] != null) { + if (values.length > 2 && values[2] != null) { behaviors = restoreBehaviorsState(context, values[2]); } - if (values[3] != null) { + if (values.length > 3 && values[3] != null) { bindings = restoreBindingsState(context, values[3]); } - if (values[4] != null) { + if (values.length > 4 && values[4] != null) { getStateHelper().restoreState(context, values[4]); } @@ -1293,7 +1274,7 @@ public static Object saveAttachedState(FacesContext context, Object attachedObje } Object result; - Class mapOrCollectionClass = attachedObject.getClass(); + Class mapOrCollectionClass = attachedObject.getClass(); boolean newWillSucceed = true; // first, test for newability of the class. try { @@ -1307,7 +1288,7 @@ public static Object saveAttachedState(FacesContext context, Object attachedObje } if (newWillSucceed && attachedObject instanceof Collection) { - Collection attachedCollection = (Collection) attachedObject; + Collection attachedCollection = (Collection) attachedObject; List resultList = new ArrayList<>(attachedCollection.size() + 1); resultList.add(new StateHolderSaver(context, mapOrCollectionClass)); for (Object item : attachedCollection) { @@ -1375,16 +1356,18 @@ public static Object restoreAttachedState(FacesContext context, Object stateObj) if (stateObj instanceof List) { List stateList = (List) stateObj; StateHolderSaver collectionSaver = stateList.get(0); - Class mapOrCollection = (Class) collectionSaver.restore(context); + Class mapOrCollection = (Class) collectionSaver.restore(context); if (Collection.class.isAssignableFrom(mapOrCollection)) { Collection retCollection = null; try { - retCollection = (Collection) mapOrCollection.newInstance(); + retCollection = (Collection) mapOrCollection.getDeclaredConstructor().newInstance(); } catch (InstantiationException | IllegalAccessException e) { if (LOGGER.isLoggable(Level.SEVERE)) { LOGGER.log(Level.SEVERE, e.toString(), e); } throw new IllegalStateException("Unknown object type"); + } catch (InvocationTargetException | NoSuchMethodException e) { + throw new RuntimeException(e); } for (int i = 1, len = stateList.size(); i < len; i++) { try { @@ -1401,12 +1384,14 @@ public static Object restoreAttachedState(FacesContext context, Object stateObj) // If we were doing assertions: assert(mapOrList.isAssignableFrom(Map.class)); Map retMap = null; try { - retMap = (Map) mapOrCollection.newInstance(); + retMap = (Map) mapOrCollection.getDeclaredConstructor().newInstance(); } catch (InstantiationException | IllegalAccessException e) { if (LOGGER.isLoggable(Level.SEVERE)) { LOGGER.log(Level.SEVERE, e.toString(), e); } throw new IllegalStateException("Unknown object type"); + } catch (InvocationTargetException | NoSuchMethodException e) { + throw new RuntimeException(e); } for (int i = 1, len = stateList.size(); i < len; i += 2) { try { @@ -1435,9 +1420,9 @@ private static Map restoreBindingsState(FacesContext co if (state == null) { return null; } - Object values[] = (Object[]) state; - String names[] = (String[]) values[0]; - Object states[] = (Object[]) values[1]; + Object[] values = (Object[]) state; + String[] names = (String[]) values[0]; + Object[] states = (Object[]) values[1]; Map bindings = new HashMap<>(names.length); for (int i = 0; i < names.length; i++) { bindings.put(names[i], (ValueExpression) restoreAttachedState(context, states[i])); @@ -1452,8 +1437,8 @@ private Object saveBindingsState(FacesContext context) { return null; } - Object values[] = new Object[2]; - values[0] = bindings.keySet().toArray(new String[bindings.size()]); + Object[] values = new Object[2]; + values[0] = bindings.keySet().toArray(new String[0]); Object[] bindingValues = bindings.values().toArray(); for (int i = 0; i < bindingValues.length; i++) { @@ -1473,7 +1458,7 @@ private Object saveSystemEventListeners(FacesContext ctx) { } int size = listenersByEventClass.size(); - Object listeners[][] = new Object[size][2]; + Object[][] listeners = new Object[size][2]; int idx = 0; boolean savedState = false; for (Entry, List> e : listenersByEventClass.entrySet()) { @@ -1491,20 +1476,19 @@ private Object saveSystemEventListeners(FacesContext ctx) { } - private Map, List> restoreSystemEventListeners(FacesContext ctx, Object state) { + private static Map, List> restoreSystemEventListeners(FacesContext ctx, Object state) { if (state == null) { return null; } Object[][] listeners = (Object[][]) state; - Map, List> m = new HashMap<>(listeners.length, 1.0f); - for (int i = 0, len = listeners.length; i < len; i++) { - Object[] source = listeners[i]; - m.put((Class) source[0], (List) restoreAttachedState(ctx, source[1])); + Map, List> map = new HashMap<>(listeners.length, 1.0f); + for (Object[] source : listeners) { + map.put((Class) source[0], (List) restoreAttachedState(ctx, source[1])); } - return m; + return map; } Map getDescriptorMap() { @@ -1690,7 +1674,7 @@ private void assertClientBehaviorHolder() { * @return true if component implements {@link ClientBehaviorHolder} interface. */ private boolean isClientBehaviorHolder() { - return ClientBehaviorHolder.class.isInstance(this); + return this instanceof ClientBehaviorHolder; } /** @@ -1722,7 +1706,7 @@ private Object saveBehaviorsState(FacesContext context) { attachedBehaviors[i++] = attachedEventBehaviors; } if (stateWritten) { - state = new Object[] { behaviors.keySet().toArray(new String[behaviors.size()]), attachedBehaviors }; + state = new Object[] { behaviors.keySet().toArray(new String[0]), attachedBehaviors }; } } return state; @@ -1749,8 +1733,8 @@ private BehaviorsMap restoreBehaviorsState(FacesContext context, Object state) { for (int i = 0; i < attachedBehaviors.length; i++) { Object[] attachedEventBehaviors = (Object[]) attachedBehaviors[i]; ArrayList eventBehaviors = new ArrayList<>(attachedBehaviors.length); - for (int j = 0; j < attachedEventBehaviors.length; j++) { - eventBehaviors.add((ClientBehavior) restoreAttachedState(context, attachedEventBehaviors[j])); + for (Object attachedEventBehavior : attachedEventBehaviors) { + eventBehaviors.add((ClientBehavior) restoreAttachedState(context, attachedEventBehavior)); } modifiableMap.put(names[i], eventBehaviors); @@ -1855,7 +1839,7 @@ private static void disconnectFromView(FacesContext context, Application applica private final static Object[] EMPTY_ARRAY = new Object[0]; // Empty iterator for short circuiting operations - private final static Iterator EMPTY_ITERATOR = new Iterator() { + private final static Iterator EMPTY_ITERATOR = new Iterator<>() { @Override public void remove() { @@ -2069,19 +2053,19 @@ public Object remove(Object keyObj) { @Override public int size() { - Map attributes = getAttributes(); + Map attributes = getAttributes(); return attributes != null ? attributes.size() : 0; } @Override public boolean isEmpty() { - Map attributes = getAttributes(); + Map attributes = getAttributes(); return attributes == null || attributes.isEmpty(); } @Override public boolean containsValue(java.lang.Object value) { - Map attributes = getAttributes(); + Map attributes = getAttributes(); return attributes != null && attributes.containsValue(value); } @@ -2201,11 +2185,11 @@ private void writeObject(ObjectOutputStream out) throws IOException { } private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { - // noinspection unchecked - Class clazz = (Class) in.readObject(); + Class clazz = (Class) in.readObject(); try { - component = (UIComponent) clazz.newInstance(); - } catch (InstantiationException | IllegalAccessException e) { + component = (UIComponent) clazz.getDeclaredConstructor().newInstance(); + } catch (InstantiationException | IllegalAccessException | NoSuchMethodException | + InvocationTargetException e) { throw new RuntimeException(e); } component.restoreState(FacesContext.getCurrentInstance(), in.readObject()); @@ -2295,8 +2279,7 @@ public void clear() { if (n < 1) { return; } - for (int i = 0; i < n; i++) { - UIComponent child = get(i); + for (UIComponent child : this) { if (isNotRenderingResponse(context)) { removeFromDescendantMarkIdCache(component, child); } @@ -2340,14 +2323,10 @@ public boolean remove(Object elementObj) { if (isNotRenderingResponse(context)) { removeFromDescendantMarkIdCache(component, element); } - if (super.indexOf(element) != -1) { + if (super.contains(element)) { element.setParent(null); } - if (super.remove(element)) { - return true; - } else { - return false; - } + return super.remove(element); } @Override @@ -2718,7 +2697,7 @@ public boolean removeAll(Collection c) { @Override public boolean retainAll(Collection c) { boolean result = false; - Iterator v = iterator(); + Iterator> v = iterator(); while (v.hasNext()) { if (!c.contains(v.next())) { v.remove(); @@ -2866,7 +2845,7 @@ public boolean contains(Object o) { } @Override - public boolean containsAll(Collection c) { + public boolean containsAll(Collection c) { for (Object item : c) { if (!map.containsKey(item)) { return false; @@ -2910,7 +2889,7 @@ public boolean removeAll(Collection c) { @Override public boolean retainAll(Collection c) { boolean result = false; - Iterator v = iterator(); + Iterator v = iterator(); while (v.hasNext()) { if (!c.contains(v.next())) { v.remove(); @@ -3061,7 +3040,7 @@ private Map> getModifiableMap() { } } - private static class PassThroughAttributesMap extends ConcurrentHashMap implements Serializable { + private static class PassThroughAttributesMap extends ConcurrentHashMap implements Serializable { private static final long serialVersionUID = 4230540513272170861L; @@ -3115,7 +3094,7 @@ private void populateDescriptorsMapIfNecessary() { // We did not find the property descriptor map so we are now going to load it. - PropertyDescriptor propertyDescriptors[] = getPropertyDescriptors(); + PropertyDescriptor[] propertyDescriptors = getPropertyDescriptors(); if (propertyDescriptors != null) { propertyDescriptorMap = new HashMap<>(propertyDescriptors.length, 1.0f); for (PropertyDescriptor propertyDescriptor : propertyDescriptors) { @@ -3150,8 +3129,7 @@ private PropertyDescriptor[] getPropertyDescriptors() { } private String addParentId(FacesContext context, String parentId, String childId) { - return new StringBuilder(parentId.length() + 1 + childId.length()).append(parentId).append(UINamingContainer.getSeparatorChar(context)).append(childId) - .toString(); + return parentId + UINamingContainer.getSeparatorChar(context) + childId; } private String getParentId(FacesContext context, UIComponent parent) { diff --git a/impl/src/main/java/jakarta/faces/render/RenderKit.java b/impl/src/main/java/jakarta/faces/render/RenderKit.java index 6f2e677e41..58da648586 100644 --- a/impl/src/main/java/jakarta/faces/render/RenderKit.java +++ b/impl/src/main/java/jakarta/faces/render/RenderKit.java @@ -64,7 +64,7 @@ public abstract class RenderKit { * * @throws NullPointerException if family or rendererType or renderer is null */ - public abstract void addRenderer(String family, String rendererType, Renderer renderer); + public abstract void addRenderer(String family, String rendererType, Renderer renderer); /** *

@@ -79,7 +79,7 @@ public abstract class RenderKit { * * @return the {@link Renderer} instance */ - public abstract Renderer getRenderer(String family, String rendererType); + public abstract Renderer getRenderer(String family, String rendererType); /** *

diff --git a/impl/src/test/java/com/sun/faces/mock/MockRenderKit.java b/impl/src/test/java/com/sun/faces/mock/MockRenderKit.java index 37b2491d41..c749184598 100644 --- a/impl/src/test/java/com/sun/faces/mock/MockRenderKit.java +++ b/impl/src/test/java/com/sun/faces/mock/MockRenderKit.java @@ -53,12 +53,11 @@ public MockRenderKit() { responseStateManager = new MockResponseStateManager(); } - private Map renderers = new HashMap(); - private ResponseStateManager responseStateManager = null; + private final Map> renderers = new HashMap<>(); + private final ResponseStateManager responseStateManager; @Override - public void addRenderer(String family, String rendererType, - Renderer renderer) { + public void addRenderer(String family, String rendererType, Renderer renderer) { if ((family == null) || (rendererType == null) || (renderer == null)) { throw new NullPointerException(); } @@ -66,11 +65,11 @@ public void addRenderer(String family, String rendererType, } @Override - public Renderer getRenderer(String family, String rendererType) { + public Renderer getRenderer(String family, String rendererType) { if ((family == null) || (rendererType == null)) { throw new NullPointerException(); } - return ((Renderer) renderers.get(family + "|" + rendererType)); + return (Renderer) renderers.get(family + "|" + rendererType); } @Override @@ -116,7 +115,7 @@ public ResponseStateManager getResponseStateManager() { return responseStateManager; } - class TestRenderer extends Renderer { + static class TestRenderer extends Renderer { private boolean rendersChildren = false; @@ -142,11 +141,11 @@ public void decode(FacesContext context, UIComponent component) { // System.err.println("decode(" + clientId + ")"); // Decode incoming request parameters - Map params = context.getExternalContext().getRequestParameterMap(); + Map params = context.getExternalContext().getRequestParameterMap(); if (params.containsKey(clientId)) { // System.err.println(" '" + input.currentValue(context) + // "' --> '" + params.get(clientId) + "'"); - input.setSubmittedValue((String) params.get(clientId)); + input.setSubmittedValue(params.get(clientId)); } }