Skip to content

Conversation

SentryMan
Copy link
Collaborator

Fixes issue where we would try to generate proxies for classes with final methods.

resolves #895

Fixes issue where we would try to generate proxies for classes with final methods
@SentryMan SentryMan added this to the 12.0 milestone Oct 1, 2025
@SentryMan SentryMan self-assigned this Oct 1, 2025
@SentryMan SentryMan added the bug Something isn't working label Oct 1, 2025
@SentryMan SentryMan enabled auto-merge (squash) October 1, 2025 09:35
@SentryMan SentryMan requested a review from rbygrave October 1, 2025 09:36
@rbygrave
Copy link
Contributor

rbygrave commented Oct 5, 2025

Hmm, so currently when moving the test into blackbox-test-inject, it won't compile as it generates the following method:

  @Override
  protected int next(int arg0) {
    return onceProvider.get().next(arg0);
  }

... which is a protected method, so wanting the use of super here. Hmm.

@SentryMan
Copy link
Collaborator Author

I see

@SentryMan
Copy link
Collaborator Author

SentryMan commented Oct 5, 2025

Actually I don't see, because when I move it to blackbox it doesn't fail.

... which is a protected method, so wanting the use of super here. Hmm.

to borrow a phrase from my stance on social media, "I don't follow". we don't want the super, we want to call the actual implementation stored in onceProvider

@SentryMan SentryMan requested a review from rob-bygrave October 5, 2025 22:16
@rbygrave
Copy link
Contributor

rbygrave commented Oct 5, 2025

So when it's:

  @Bean
  public Random secureRandom() {
    return new SecureRandom();
  }

Then it becomes a registerLazy() [beanReader.proxyLazy()] ... and we get a Random$Lazy generated and that doesn't support the protected method.

 ...
      var factory = builder.get(RandomFactory.class);
      builder.registerLazy(() -> {
      return factory.secureRandom();
      }, Random$Lazy::new);    }

... but with SecureRandom its just a lazy [beanReader.lazy()] ... so no Random$Lazy generated

      var factory = builder.get(RandomFactory.class);
      builder.registerProvider(() -> {
      return factory.secureRandom();
          });

@SentryMan
Copy link
Collaborator Author

well, it appears the bigger problem is situations where the no-arg constructor calls the methods. I'll just an escape hatch to disable proxy generation

@rbygrave
Copy link
Contributor

rbygrave commented Oct 6, 2025

Ah, but a @Lazy without a proxy ... isn't really lazy in that it be instantiated when it's injected [which is typically early unless it is injected as Provider]? What am I missing?

@SentryMan
Copy link
Collaborator Author

SentryMan commented Oct 6, 2025

That's exactly right, it's the old behavior if you set that flag to false. In the case of random it doesn't appear that we can cleanly lazy it with a proxy.

…xception

with the inline comment:

`@Lazy` proxy does not support protected methods, instead use @lazy(useProxy = false)
@rbygrave
Copy link
Contributor

rbygrave commented Oct 6, 2025

@SentryMan review the last commit c0ecf8f .... that will generate protected methods that throw UnsupportedOperationException like:

  @Override
  protected int next(int arg0) {
    // @Lazy proxy does not support protected methods, instead use @Lazy(useProxy = false)
    throw new UnsupportedOperationException();
  }

Without this, user will instead get a compilation error which I think will confuse them so I think this might be better?

@SentryMan
Copy link
Collaborator Author

I mean in the first place the generated classes are final so the generated protected methods cannot be accessed

@rbygrave
Copy link
Contributor

rbygrave commented Oct 6, 2025

the generated protected methods cannot be accessed

Technically they are accessible by classes in the same package as the generated lazy class ... but that does require specific use of the _$Lazy type so the expectation is that isn't something people will do.

So yeah, if you are happy with that last commit we can look to merge this etc.

Copy link
Collaborator Author

@SentryMan SentryMan left a comment

Choose a reason for hiding this comment

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

Lgtm

@SentryMan
Copy link
Collaborator Author

Actually one sec

@SentryMan
Copy link
Collaborator Author

okay good with this

@rbygrave rbygrave removed the request for review from rob-bygrave October 6, 2025 08:45
@SentryMan SentryMan merged commit 6481e1a into avaje:master Oct 6, 2025
5 checks passed
@SentryMan SentryMan deleted the lazy-final branch October 6, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lazy fails to compile

2 participants