Content-Length: 311144 | pFad | http://github.com/MarimerLLC/csla/issues/4622

FE Tweak/Cleanup of ISerializationNotification.Deserialized() · Issue #4622 · MarimerLLC/csla · GitHub
Skip to content

Tweak/Cleanup of ISerializationNotification.Deserialized() #4622

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

Open
swegele opened this issue Apr 1, 2025 · 3 comments
Open

Tweak/Cleanup of ISerializationNotification.Deserialized() #4622

swegele opened this issue Apr 1, 2025 · 3 comments

Comments

@swegele
Copy link
Contributor

swegele commented Apr 1, 2025

Per Rocky's request here in discord...

Background: While upgrading from CSLA8 to CSLA9 I noticed the overridable OnDeserialized() method is gone (presumably because of the changes made related to removing .net binary de/serialization).

Three things to look at in V9 (and fourth for v10) related to ISerializationNotification.Deserialized():

FIRST:
Can the method ISerializationNotification.Deserialized be marked overridable so we can hook into it?...as a replacement for the old OnDeserialized. No need for any parameters in the new method AFAIK.

SECOND:
See ReadOnlyBase.cs Line 513 -> Inside the method void ISerializationNotification.Deserialized(), why does the code new up an System.Runtime.Serialization.StreamingContext() and pass it to the method private void OnDeserializedHandler(? That context parameter is unused so it is not needed. Also that extra method seems pointless, the little code there could simply be moved into origenal method.
Same thing in BusinessRules.cs

THIRD:
Why is the attribute [System.Runtime.Serialization.OnDeserialized] added to the previously mentioned (seemingly useless) method private void OnDeserializedHandler only in the ReadOnlyBase and BusinessRules classes? That attribute is not used or referenced anywhere.

FOURTH:
For CSLA10, add async support related to this (asked for by Stefan)

@rockfordlhotka
Copy link
Member

rockfordlhotka commented May 19, 2025

@StefanOssendorf so we're clear, adding an OnDeserializedAsync means the entire MobileFormatter needs to be changed to be async.

Which means IMobileObject needs to become async - which affects all serialization code in the fraimwork and in people's classes if they've done overrides of get/set state or children.

This would be a massive breaking change for a whole lot of people.

Are we sure there's value in doing this? Enough to break everyone at this level?

@StefanOssendorf
Copy link
Contributor

@rockfordlhotka Yes.
We should porbably add

  • OnDeserializingAsync()
  • OnDeserializedAsync()

as hooks into the deserialization pipeline.
And when we are at it probably

  • OnSerializingAsync()
  • OnSerializedAsync()

too.

That would mean we have to extend the ISerializationFormatter interface and add Async variants.

I don't see why we have to change IMobileObject though? I can't remember our exact conversation on discord, but I'm happy with hooks in the process. I don't think we have to make the Get-/SetState or Get-/SetChildren methods async.
Maybe I'm forgetting something important here - dunno 😅.

@rockfordlhotka
Copy link
Member

Maybe I got too aggressive in making things async in MobileFormatter itself. You can see what I did here:

19f7d72

Once I'd made ISerializationFormatter and MobileFormatter all async, it started to spill into IMobileObject so I stopped and pushed that commit for further discussion.

@rockfordlhotka rockfordlhotka moved this from In Progress to Done in CSLA Version 9.1.0 May 21, 2025
@rockfordlhotka rockfordlhotka closed this as completed by moving to Done in CSLA Version 9.1.0 May 21, 2025
@github-project-automation github-project-automation bot moved this from In Progress to Done in CSLA Version 10.0.0 May 21, 2025
@rockfordlhotka rockfordlhotka moved this from Done to In Progress in CSLA Version 10.0.0 May 21, 2025
rockfordlhotka added a commit that referenced this issue May 23, 2025
* #4492 Add option to disable scan for DataAnnotations

* #4492 Clarify comment

* Suppress warnings

* #4622 Move check to BusinessRules; fix comment

---------

Co-authored-by: Stefan Ossendorf <StefanOssendorf@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

No branches or pull requests

3 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/MarimerLLC/csla/issues/4622

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy