diff --git a/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java b/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java index 36c653f07..e9f98d5d2 100644 --- a/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java +++ b/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java @@ -136,6 +136,7 @@ public PropertyUtilsBean() { */ public void addBeanIntrospector(final BeanIntrospector introspector) { introspectors.add(Objects.requireNonNull(introspector, "introspector")); + clearDescriptorCaches(); } /** @@ -148,6 +149,15 @@ public void clearDescriptors() { Introspector.flushCaches(); } + /** + * Discards the memoized introspection results after the registered {@link BeanIntrospector} set has changed. Unlike {@link #clearDescriptors()} this leaves + * the JVM-global {@link Introspector} cache untouched. + */ + private void clearDescriptorCaches() { + descriptorsCache.clear(); + mappedDescriptorsCache.clear(); + } + /** *

* Copy property values from the "origin" bean to the "destination" bean for all cases where the property names are the same (even though the actual getter @@ -1222,7 +1232,11 @@ public boolean isWriteable(Object bean, String name) { * @since 1.9 */ public boolean removeBeanIntrospector(final BeanIntrospector introspector) { - return introspectors.remove(introspector); + final boolean removed = introspectors.remove(introspector); + if (removed) { + clearDescriptorCaches(); + } + return removed; } /** @@ -1236,6 +1250,7 @@ public final void resetBeanIntrospectors() { introspectors.add(DefaultBeanIntrospector.INSTANCE); introspectors.add(SuppressPropertiesBeanIntrospector.SUPPRESS_CLASS); introspectors.add(SuppressPropertiesBeanIntrospector.SUPPRESS_DECLARING_CLASS); + clearDescriptorCaches(); } /** diff --git a/src/test/java/org/apache/commons/beanutils2/PropertyUtilsTest.java b/src/test/java/org/apache/commons/beanutils2/PropertyUtilsTest.java index 8e10a0ecf..0d1753d13 100644 --- a/src/test/java/org/apache/commons/beanutils2/PropertyUtilsTest.java +++ b/src/test/java/org/apache/commons/beanutils2/PropertyUtilsTest.java @@ -21,6 +21,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -346,6 +347,104 @@ void testCustomIntrospectionSuppressedMappedPropertyNullEntry() throws Exception assertNotNull(pub.getPropertyDescriptor(bean, "stringProperty"), "A null suppressed entry must not hide unrelated properties"); } + /** + * Registering a {@link SuppressPropertiesBeanIntrospector} must take effect for classes already introspected. The per-class descriptor cache was populated + * before the introspector was added and was never invalidated, so a property suppressed for hardening stayed readable and writable when its class had been + * introspected earlier (the common case with the shared {@code PropertyUtils} singleton). + */ + @Test + void testAddBeanIntrospectorInvalidatesCache() throws Exception { + final PropertyUtilsBean pub = new PropertyUtilsBean(); + + // Warm the descriptor cache for TestBean before the suppression is configured. + assertNotNull(pub.getPropertyDescriptor(bean, "stringProperty"), "Property should be visible before suppression"); + + final SuppressPropertiesBeanIntrospector suppressor = new SuppressPropertiesBeanIntrospector(Arrays.asList("stringProperty")); + pub.addBeanIntrospector(suppressor); + + assertNull(pub.getPropertyDescriptor(bean, "stringProperty"), "Suppressed property should have no descriptor after the introspector is added"); + assertThrows(NoSuchMethodException.class, () -> pub.getProperty(bean, "stringProperty"), "Suppressed property must not be readable"); + assertThrows(NoSuchMethodException.class, () -> pub.setProperty(bean, "stringProperty", "changed"), "Suppressed property must not be writable"); + + // Removing the introspector must likewise invalidate the cache so the property becomes visible again. + pub.removeBeanIntrospector(suppressor); + assertNotNull(pub.getPropertyDescriptor(bean, "stringProperty"), "Property should be visible again after the introspector is removed"); + } + + /** + * {@code resetBeanIntrospectors()} must invalidate the descriptor caches so that introspection results produced by a previously registered custom + * introspector are re-evaluated against the restored default set. + */ + @Test + void testResetBeanIntrospectorsInvalidatesCache() throws Exception { + final PropertyUtilsBean pub = new PropertyUtilsBean(); + pub.addBeanIntrospector(new SuppressPropertiesBeanIntrospector(Arrays.asList("stringProperty"))); + + // Warm the descriptor cache with the suppression in effect. + assertNull(pub.getPropertyDescriptor(bean, "stringProperty"), "Suppressed property should have no descriptor"); + + pub.resetBeanIntrospectors(); + + assertNotNull(pub.getPropertyDescriptor(bean, "stringProperty"), "Property should be re-evaluated and visible after the reset"); + assertEquals(bean.getStringProperty(), pub.getProperty(bean, "stringProperty"), "Property should be readable after the reset"); + } + + /** + * Changing the introspector set must also invalidate the mapped descriptor cache. The suppression guard in {@code getPropertyDescriptor} already hides a + * suppressed mapped property, so this checks the cached {@link MappedPropertyDescriptor} itself is dropped and re-created rather than served stale. + */ + @Test + void testIntrospectorChangeInvalidatesMappedDescriptorCache() throws Exception { + final PropertyUtilsBean pub = new PropertyUtilsBean(); + + // Warm the mapped descriptor cache for TestBean. + final PropertyDescriptor first = pub.getPropertyDescriptor(bean, "mappedProperty"); + assertNotNull(first, "Mapped property should be visible before suppression"); + + final SuppressPropertiesBeanIntrospector suppressor = new SuppressPropertiesBeanIntrospector(Arrays.asList("mappedProperty")); + pub.addBeanIntrospector(suppressor); + + assertNull(pub.getPropertyDescriptor(bean, "mappedProperty"), "Suppressed mapped property should have no descriptor"); + assertThrows(NoSuchMethodException.class, () -> pub.getProperty(bean, "mappedProperty(First Key)"), "Suppressed mapped property must not be readable"); + + pub.removeBeanIntrospector(suppressor); + + final PropertyDescriptor second = pub.getPropertyDescriptor(bean, "mappedProperty"); + assertNotNull(second, "Mapped property should be visible again after the introspector is removed"); + assertNotSame(first, second, "Mapped descriptor must be re-created, not served from the stale cache"); + assertEquals("First Value", pub.getProperty(bean, "mappedProperty(First Key)"), "Mapped property should be readable again"); + } + + /** + * Each add and remove in a sequence of introspector changes must invalidate the caches, so the visible property set always reflects the currently + * registered introspectors. + */ + @Test + void testSequentialIntrospectorChangesInvalidateCache() throws Exception { + final PropertyUtilsBean pub = new PropertyUtilsBean(); + final SuppressPropertiesBeanIntrospector suppressString = new SuppressPropertiesBeanIntrospector(Arrays.asList("stringProperty")); + final SuppressPropertiesBeanIntrospector suppressInt = new SuppressPropertiesBeanIntrospector(Arrays.asList("intProperty")); + + assertNotNull(pub.getPropertyDescriptor(bean, "stringProperty"), "stringProperty should be visible initially"); + assertNotNull(pub.getPropertyDescriptor(bean, "intProperty"), "intProperty should be visible initially"); + + pub.addBeanIntrospector(suppressString); + assertNull(pub.getPropertyDescriptor(bean, "stringProperty"), "stringProperty should be suppressed after the first add"); + assertNotNull(pub.getPropertyDescriptor(bean, "intProperty"), "intProperty should be unaffected by the first add"); + + pub.addBeanIntrospector(suppressInt); + assertNull(pub.getPropertyDescriptor(bean, "stringProperty"), "stringProperty should stay suppressed after the second add"); + assertNull(pub.getPropertyDescriptor(bean, "intProperty"), "intProperty should be suppressed after the second add"); + + pub.removeBeanIntrospector(suppressString); + assertNotNull(pub.getPropertyDescriptor(bean, "stringProperty"), "stringProperty should be visible again after its introspector is removed"); + assertNull(pub.getPropertyDescriptor(bean, "intProperty"), "intProperty should stay suppressed after the unrelated remove"); + + pub.removeBeanIntrospector(suppressInt); + assertNotNull(pub.getPropertyDescriptor(bean, "stringProperty"), "stringProperty should stay visible after the last remove"); + assertNotNull(pub.getPropertyDescriptor(bean, "intProperty"), "intProperty should be visible again after its introspector is removed"); + } + /** * Test the describe() method. */