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

Normalize LocalProxy by subclassing DataPortalProxy #2861 #3546

Conversation

swegele
Copy link
Contributor

@swegele swegele commented Nov 9, 2023

Fixes #2861 by making LocalProxy subclass DataPortalProxy so it is like the other 3 proxies; Also adds a virtual method called when switching appcontext back to client which allows dev to inherit from LocalProxy giving a hook to watch for salient things "coming back" to client from logical server;

Closes #3556

Resolves MarimerLLC#2861 by making LocalProxy subclass DataPortalProxy so it is like the other 3 proxies;
Also adds a virtual method called when switching appcontext back to client which allows dev to inherit from LocalProxy giving a hook to watch things salient things coming back to client from server;
- now that we are inheriting from DataPortalProxy I made OriginalApplicationContext simply point to base.ApplicationContext;
- better comments and minor fix;
use CurrentApplicationContext when checking location after DataPortal operation;
@swegele
Copy link
Contributor Author

swegele commented Nov 9, 2023

@rockfordlhotka @TheCakeMonster - this PR is a followup attempt at the things we discussed wayyyy back in #2856.

In review we have many different well defined hooks / extensibility points to tap into on the server side of the DataPortal (logical or physical). (example: OnDataPortalInvoke, OnDataPortalInvokeComplete, interceptors, etc.)

But on the client side we only have access to the virtual method ConvertResponse (by way of subclassing a proxy that inherited from DataPortalProxy). Unfortunately that didn't help with LocalProxy which was treated differently. That really left no easy practical consistent way to intercept client side actions.

So this PR attemtps to do several things:

  • Make LocalProxy inherit from DataPortalProxy so it is consistent as Rocky suggested
  • Adds a more helpful virtual method ReturnedToClientSideAndPreparedResult useable in all proxies returning a helpful DataPortalResult instead of just byte array stuff.
  • Adds a very helpful virtual method specific to LocalPortal called RestoringClientSideApplicationContext

I explained this to my wife this evening like this: "Now I more easily have the ability to consistently intercept everyone returning from overseas immediately as they get off the plane at the home airport. I can watch for important generic things. Before I only could ask the travelers to do a self-check-in when they got all the way home."

I hope all this makes sense. Let me know what you think.

Copy link
Member

@rockfordlhotka rockfordlhotka left a comment

Choose a reason for hiding this comment

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

I like the extensibility point of having a virtual method when the call returns from the logical server.

I actually think there should maybe be two such methods:

  • OnServerComplete
  • OnServerCompleteClient

The second one would really be optional, since the first one could do its own check to see if the logical execution location is now client - but I can see the convenience of having this built-in to the framework.

So what is in the current PR is basically OnServerCompleteClient, because it is only invoked if the logical execution location is now client. I think that's fine, but there should also be the OnServerComplete that is always invoked when a logical server call completes (even if the code is still loigcally on the server).

Copy link
Member

@rockfordlhotka rockfordlhotka left a comment

Choose a reason for hiding this comment

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

There is a warning about IsServerRemote being hidden - that seems wrong to me.

Copy link
Member

@rockfordlhotka rockfordlhotka left a comment

Choose a reason for hiding this comment

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

There is a warning where LocalProxy has a method with no await in an async method. Is that a valid warning? If so, we should suppress the warning.

changes per Rocky's suggestions on naming and tweaks based on compiler suggestions;  MarimerLLC#2861
@rockfordlhotka rockfordlhotka merged commit 384da1f into MarimerLLC:main Nov 13, 2023
1 check passed
@swegele swegele deleted the LocalProxy_InheritFromDataPortalProxy_2861 branch November 14, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants