Skip to content

Clear descriptor caches when bean introspectors change - #435

Open
rootvector2 wants to merge 2 commits into
apache:masterfrom
rootvector2:introspector-cache-invalidation
Open

Clear descriptor caches when bean introspectors change#435
rootvector2 wants to merge 2 commits into
apache:masterfrom
rootvector2:introspector-cache-invalidation

Conversation

@rootvector2

Copy link
Copy Markdown
Contributor

addBeanIntrospector, removeBeanIntrospector and resetBeanIntrospectors mutate the introspectors list but never invalidate descriptorsCache, which getIntrospectionData fills once per bean class, so registering a SuppressPropertiesBeanIntrospector (the documented BEANUTILS-463 hardening) after a class has already been introspected is a no-op: getPropertyDescriptor returns the stale cached descriptor at its early return before it reaches the isPropertySuppressed guard, and the suppressed property stays readable and writable via populate/getProperty/setProperty. found while auditing the suppression path from #413/#431, whose own tests call clearDescriptors() right before addBeanIntrospector to work around this. clearing the descriptor caches when the introspector set changes makes the change take effect for already-cached classes; i did not reuse clearDescriptors() because it also flushes the JVM-global Introspector cache.

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

the per-class descriptorsCache is populated once and was never invalidated when addBeanIntrospector, removeBeanIntrospector or resetBeanIntrospectors changed the introspector set, so a SuppressPropertiesBeanIntrospector registered after a class was introspected had no effect.
@garydgregory garydgregory changed the title clear descriptor caches when bean introspectors change Clear descriptor caches when bean introspectors change Aug 14, 2026

@garydgregory garydgregory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @rootvector2

Thank you for the PR.

What do you think about adding tests that cover the following patterns:

  • resetBeanIntrospectors() cache invalidation. The method now calls clearDescriptorCaches(), but there is no dedicated test that warms a cache, calls resetBeanIntrospectors() ,and checks that a previously suppressed or modified property is re-evaluated.
  • Invalidation of the mappedDescriptorsCache path specifically. The existing testCustomIntrospectionSuppressedMappedProperty* tests exercise suppression of mapped properties, but they add the introspector before the first access. A test that warms a mapped descriptor, then adds and/or removes an introspector and checks the mapped descriptor is re-created would close the gap.
  • Multiple sequential adds/removes.

cover resetBeanIntrospectors() re-evaluating a warmed cache, the mappedDescriptorsCache entry being re-created rather than served stale, and multiple sequential adds/removes each taking effect. all three fail without the cache invalidation in PropertyUtilsBean.
@rootvector2

Copy link
Copy Markdown
Contributor Author

makes sense, added all three in 85eda71: testResetBeanIntrospectorsInvalidatesCache warms the cache with a suppressor and checks the property is re-evaluated after the reset; testIntrospectorChangeInvalidatesMappedDescriptorCache warms a mapped descriptor, then adds/removes an introspector and asserts with assertNotSame that the cached MappedPropertyDescriptor is re-created rather than served stale (the suppression guard alone would mask a stale entry, so the identity check is what closes that gap); testSequentialIntrospectorChangesInvalidateCache does two adds then two removes and checks the visible property set after each step. verified all three fail with the invalidation in PropertyUtilsBean reverted. full default mvn goal green, 1291 tests.

@garydgregory

Copy link
Copy Markdown
Member

@rootvector2 Please rebase on git master to pick up a fix to the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants