Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ public final class EnhancedCompositeBeanHelper {
// Cache for field lookups: Class -> FieldName -> Field
private static final ConcurrentMap<Class<?>, Map<String, Field>> FIELD_CACHE = new ConcurrentHashMap<>();

// Cache for accessible fields to avoid repeated setAccessible calls
private static final ConcurrentMap<Field, Boolean> ACCESSIBLE_FIELD_CACHE = new ConcurrentHashMap<>();

private final ConverterLookup lookup;
private final ClassLoader loader;
private final ExpressionEvaluator evaluator;
Expand Down Expand Up @@ -304,19 +301,9 @@ private Object convertProperty(
* Set field value with cached accessibility.
*/
private void setFieldValue(Object bean, Field field, Object value) throws IllegalAccessException {
Boolean isAccessible = ACCESSIBLE_FIELD_CACHE.get(field);
if (isAccessible == null) {
isAccessible = field.canAccess(bean);
if (!isAccessible) {
field.setAccessible(true);
isAccessible = true;
}
ACCESSIBLE_FIELD_CACHE.put(field, isAccessible);
} else if (!isAccessible) {
if (!field.canAccess(bean)) {
field.setAccessible(true);
ACCESSIBLE_FIELD_CACHE.put(field, true);
}

field.set(bean, value);
}

Expand All @@ -326,6 +313,5 @@ private void setFieldValue(Object bean, Field field, Object value) throws Illega
public static void clearCaches() {
METHOD_CACHE.clear();
FIELD_CACHE.clear();
ACCESSIBLE_FIELD_CACHE.clear();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,68 @@ void testCacheClearance() throws Exception {
assertEquals("testValue", bean2.getName());
}

@Test
void testFieldAccessibilityIsProperlyRestored() throws Exception {
TestBean bean = new TestBean();
PlexusConfiguration config = new XmlPlexusConfiguration("test");
config.setValue("fieldValue");

when(evaluator.evaluate("fieldValue")).thenReturn("fieldValue");

// Get the field to check its accessibility state
java.lang.reflect.Field field = TestBean.class.getDeclaredField("directField");

// Verify field is not accessible initially
boolean initialAccessibility = field.canAccess(bean);

// Set the property using the helper
helper.setProperty(bean, "directField", String.class, config);

// Verify the value was set correctly
assertEquals("fieldValue", bean.getDirectField());

// Verify field accessibility is restored to its original state
boolean finalAccessibility = field.canAccess(bean);
assertEquals(
initialAccessibility,
finalAccessibility,
"Field accessibility should be restored to its original state after setting value");
}

@Test
void testMultipleFieldAccessesDoNotLeakAccessibility() throws Exception {
// This test verifies that repeated field accesses don't leave fields in an accessible state
// which was the issue with the old caching implementation
TestBean bean1 = new TestBean();
TestBean bean2 = new TestBean();
PlexusConfiguration config = new XmlPlexusConfiguration("test");
config.setValue("value1");

when(evaluator.evaluate("value1")).thenReturn("value1");
when(evaluator.evaluate("value2")).thenReturn("value2");

java.lang.reflect.Field field = TestBean.class.getDeclaredField("directField");

// First access
helper.setProperty(bean1, "directField", String.class, config);
boolean accessibilityAfterFirst = field.canAccess(bean1);

// Second access with different bean
config.setValue("value2");
helper.setProperty(bean2, "directField", String.class, config);
boolean accessibilityAfterSecond = field.canAccess(bean2);

// Both should have the same accessibility state (not accessible)
assertEquals(
accessibilityAfterFirst,
accessibilityAfterSecond,
"Field accessibility should be consistent across multiple accesses");

// Verify values were set correctly
assertEquals("value1", bean1.getDirectField());
assertEquals("value2", bean2.getDirectField());
}

/**
* Test bean class for testing property setting.
*/
Expand Down