Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Forward non-intercepted methods on class proxies to target #450

Conversation

stakx
Copy link
Member

@stakx stakx commented Jul 13, 2019

This should resolve #67.

(At least as far as is possible: non-interceptable methods such as non-virtual, sealed, or private methods of course still cannot be forwarded to the proxy.)

@stakx stakx mentioned this pull request Jul 13, 2019
The (failing) test being added here specifies that if one chooses via
`IProxyGenerationHook.ShouldInterceptMethod` not to intercept a method
then invocations of that method on the proxy should be forwarded to
the target object (if present).
@stakx stakx force-pushed the forward-nonintercepted-methods-to-target branch from 1cafbc2 to 7228bf7 Compare July 13, 2019 16:51
Copy link
Member

@jonorossi jonorossi left a comment

Choose a reason for hiding this comment

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

@stakx just a couple of comments.

@stakx

This comment has been minimized.

Methods which are filtered out by `MembersCollector` (e.g. because the
hook's `ShouldInterceptMethod` returned false) are hereby made visible
to the proxy-class-with-target generator.

Some methods must still be excluded: `ForwardingMethodGenerator` does
not perform any accessibility checks, so we need to check whether the
target method is callable "from the outside". For most cases, a simple
`method.IsPublic` check suffices, however we might want to test if the
target method is callable through an interface.
@stakx stakx force-pushed the forward-nonintercepted-methods-to-target branch from 7228bf7 to 3ae08b8 Compare July 15, 2019 19:13
@stakx stakx changed the title Forward non-intercepted methods on class proxies to target WIP: Forward non-intercepted methods on class proxies to target Jul 15, 2019
@stakx
Copy link
Member Author

stakx commented Jul 15, 2019

Once again, this is going to take more work than anticipated. What is needed here is a utility method that checks accessibility of a member M (the method on the target type) to some referent type T (the proxy type). Once properly implemented, the current method.IsPublic check can be replaced with something like method.IsAccessibleTo(proxyType). There are some subtleties I'm possibly not thinking of right now, I'm pretty sure I'll discover them while writing tests.

(Also, we might be able to reimplement ProxyUtil.IsAccessible* in terms of the new accessibility check method, as the latter covers a superset of the former's functionality.)

I'll ping you once I'm done.

@jonorossi
Copy link
Member

Once again, this is going to take more work than anticipated.

Haha, as always with DP.

I'll ping you once I'm done.

👍

@jonorossi jonorossi marked this pull request as draft April 28, 2020 12:43
@jonorossi
Copy link
Member

@stakx how about this one?

@stakx
Copy link
Member Author

stakx commented Apr 28, 2020

Same here. I'll eventually get back to it.

@stakx stakx closed this Apr 28, 2020
@stakx stakx deleted the forward-nonintercepted-methods-to-target branch January 30, 2021 22:36
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.

Not proxied methods on class proxy with target should forward to the target [DYNPROXY-183]
2 participants