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

[Guidance] How to properly deserialize entity graphs coming from the server? #64

Closed
omatrot opened this issue Jun 27, 2019 · 39 comments
Closed
Labels
question Further information is requested

Comments

@omatrot
Copy link
Contributor

omatrot commented Jun 27, 2019

I see two potential problems:
1/ Entities on the server are pascal cased on the server while camel cased on the client.
2/ What about object references looping and duplicates?

I suppose the other way around there is something to do too.

@tonysneed
Copy link
Contributor

tonysneed commented Jun 27, 2019

Hi @omatrot, welcome!

Let's move these issues over to EntityFrameworkCore.Scaffolding.Handlebars, which is where the code gen takes place.

To respond to your items:

1/ Entities on the server are pascal cased on the server while camel cased on the client.

This is the default behavior of ASP.NET Core, so there should not be a problem. Have you tried it yet?

2/ What about object references looping and duplicates?

Good question. In .NET there is a setting which handles cycles in the graph. On the client, you best bet may the the Dcerialize npm package, which is compatible with Json.Net on the server.

@omatrot
Copy link
Contributor Author

omatrot commented Jun 27, 2019

Hi Tony, I'm @omatrot on Twiter :)
I'm using trackable-entities-js.
1/ I'm sending Data through SignalR and found a way to use camel casing properties like this:
return JsonConvert.SerializeObject(wholeResult, new JsonSerializerSettings { ContractResolver = new CamelCasePropertyNamesContractResolver(), ReferenceLoopHandling = ReferenceLoopHandling.Serialize, PreserveReferencesHandling = PreserveReferencesHandling.Objects });2/ I had no clue about Dcerialize, will check it out.

Thanks for the amazing work.

@tonysneed tonysneed transferred this issue from TrackableEntities/ObservableEntities.Core.Templates.TypeScript Jun 27, 2019
@tonysneed tonysneed added the question Further information is requested label Jun 27, 2019
@omatrot
Copy link
Contributor Author

omatrot commented Jun 28, 2019

Dcerialize works only with annoted properties.

//unannotated properties are not serialized or deserialized, they are totally ignored
That would be problematic right now.

I assume that entities constructors must be called for the whole tracking to work properly ?

@tonysneed
Copy link
Contributor

Dcerialize works only with annoted properties.

Can you update the hbs templates to include the annotations?

I assume that entities constructors must be called for the whole tracking to work properly ?

Yes, you need the ctor to return super.proxify(this).

@omatrot
Copy link
Contributor Author

omatrot commented Jun 28, 2019

Can you update the hbs templates to include the annotations?

I'm afraid I'll need to do that, will start by manually modifying a few entities though.

Yes, you need the ctor to return super.proxify(this).

I may be wrong, but standard JSON deserialization will not call constructors, so Dcerialize should be used as it will call them.

@tonysneed
Copy link
Contributor

tonysneed commented Jun 28, 2019

I may be wrong, but standard JSON deserialization will not call constructors, so Dcerialize should be used as it will call them.

The ctor does not need to be called during serialization. Rather it's called in the client (SPA) app to set the ITrackable properties (trackingState, modifiedProperties), which are themselves serialized.

@omatrot
Copy link
Contributor Author

omatrot commented Jun 28, 2019

I'm talking about deserialization on the client indeed.

@tonysneed
Copy link
Contributor

tonysneed commented Jun 28, 2019

You should be fine then. Have you tried and run into problems?

@omatrot
Copy link
Contributor Author

omatrot commented Jun 28, 2019

Right now I'm using standard JSON deserialization on the client (Javascript).
Change tracking is not working, despite setting tracking to true, because of the missing call to the entity constructor during deserialization.

I guess I need to use Dcerialize for the constructors to be called and change tracking to work properly

@tonysneed
Copy link
Contributor

Ah yes, I believe Dcerialize will help then.

Otherwise you would need to call the Proxify method on deserialized entities.

@omatrot
Copy link
Contributor Author

omatrot commented Jul 4, 2019

Calling the proxify method on deserialized entities fails with Class constructor TrackablEntity cannot be invoked without 'new'
So I guess I'm stuck unless I have a proxify method that use Object.create() instead?

@tonysneed
Copy link
Contributor

Can you paste the code here?

@omatrot
Copy link
Contributor Author

omatrot commented Jul 4, 2019

// this.datiEntityToEdit is a deserialized entity
const obj = ObservableEntity.proxify(
      Object.getPrototypeOf(this.datiEntityToEdit).constructor
    );

@tonysneed
Copy link
Contributor

Does the entity have a constructor?

@omatrot
Copy link
Contributor Author

omatrot commented Jul 4, 2019

Yes:

  constructor() {
    super();
    //return super.proxify(this);
  }

@tonysneed
Copy link
Contributor

Does web pack remove the constructor? What happens if you add console.log?

@tonysneed
Copy link
Contributor

Perhaps you could reproduce the issue with a small GitHub repo?

@omatrot
Copy link
Contributor Author

omatrot commented Jul 4, 2019

The constructor is called,
This is the call to super(); that throws the error.

But I think this is my fault because I have es5 instead of es2015 in .tsconfig target compiler option.
Setting 'es2015' actually leads to errors because of circular dependencies between my entities.

Didn't you had this problem with your sample ?

@tonysneed
Copy link
Contributor

You can’t use the es5 compilation option because proxies are an es2015 feature that cannot be transpiled to es5.

My sample does not perform deserialization. But I can modify it to do so.

Doesn’t dcerialize solve circular references?

@omatrot
Copy link
Contributor Author

omatrot commented Jul 4, 2019

Yes it does, but here I'm talking about circular references from the angular point of view:

WARNING in Circular dependency detected:
src\app\domain\models\Device.ts -> src\app\domain\models\Registration.ts -> src\app\domain\models\Device.ts

WARNING in Circular dependency detected:
src\app\domain\models\Fleet.ts -> src\app\domain\models\FleetDetail.ts -> src\app\domain\models\Fleet.ts

WARNING in Circular dependency detected:
src\app\domain\models\FleetDetail.ts -> src\app\domain\models\Fleet.ts -> src\app\domain\models\FleetDetail.ts

WARNING in Circular dependency detected:
src\app\domain\models\GpsBoxConfiguration.js -> src\app\domain\models\VehiculeGpsBoxInfo.ts -> src\app\domain\models\GpsBoxConfiguration.js

WARNING in Circular dependency detected:
src\app\domain\models\Registration.ts -> src\app\domain\models\Device.ts -> src\app\domain\models\Registration.ts

WARNING in Circular dependency detected:
src\app\domain\models\Vehicule.js -> src\app\domain\models\VehiculeGpsBoxInfo.ts -> src\app\domain\models\Vehicule.js

WARNING in Circular dependency detected:
src\app\domain\models\Vehicule.ts -> src\app\domain\models\VehiculeGpsBoxInfo.ts -> src\app\domain\models\Vehicule.js -> src\app\domain\models\FleetDetail.ts -> src\app\domain\models\Vehicule.ts

WARNING in Circular dependency detected:
src\app\domain\models\VehiculeGpsBoxInfo.ts -> src\app\domain\models\Vehicule.js -> src\app\domain\models\VehiculeGpsBoxInfo.ts

EDIT:

My angular module is importing several entities, and I think that this is the source of the problem:

import { Vehicule } from "../../../../../domain/models/Vehicule";
import { GpsBoxConfiguration } from "../../../../../domain/models/GpsBoxConfiguration";
import { GpsBoxType } from "../../../../../domain/models/GpsBoxType";

@tonysneed
Copy link
Contributor

Two ways to deal with the Angular warning.

  1. Set "showCircularDependencies": false in your .angular-cli.json file.
  2. Put all the entities into a single .ts file. You should be able to do this in .tsconfig.app.json.

@omatrot
Copy link
Contributor Author

omatrot commented Jul 5, 2019

I've pulled all the entities in one single ts file and it solved the warning during compilation.
I'm also back to using ES6 target.

This is going to sound crazy, but everyting is fine until I simply add code using Dcerialize (in my case DeserializeArray). With this code that is not even called yet, the angular module complains that one of my entity cannot be used before being instantiated.

If I'm using ES5 target, the angular module is OK.

EDIT:
This seems to be a bug in Angular.

@tonysneed
Copy link
Contributor

Sounds like your were able to get past the circular dependencies issue by specifying a single .ts output file. This is the same issue as the Angular bug you mentioned.

The other issue seems like it is specific to Dcerialize, so maybe open an issue there? It seems to have to do with deserializing entities with constructors.

@omatrot
Copy link
Contributor Author

omatrot commented Jul 5, 2019

Unfortunately, issues are not enabled in the Dcerialize repo :(

@tonysneed
Copy link
Contributor

Maybe check these out? https://stackoverflow.com/a/22245105

@omatrot
Copy link
Contributor Author

omatrot commented Jul 9, 2019

I may be wrong but all are creating simple JSON objects with only properties.
Maybe this one could do if it understands JSON.NET references

@omatrot
Copy link
Contributor Author

omatrot commented Jul 10, 2019

Ok I've been able to reproduce the problem on your trackable entities sample simply by adding Typescript decorators on entities.

I've ben using serializr to successfully deserialize entities. I just need to handle JSON.NET refs by myself and deal with arrays that are not supported right now.
It works because there is an API to mimic the decorators to tell how (de)serialization should go.

I'll report here as soon as I have a working example with my entities.

@tonysneed
Copy link
Contributor

serialzr looks promising. Regarding cycles, you might want to consider creating separate entities for serialization that don’t have cyclical references, using something like AutoMapper (see https://medium.com/ps-its-huuti/how-to-get-started-with-automapper-and-asp-net-core-2-ecac60ef523f). It sounds like Json.Net’s ref format isn’t interoperable.

@omatrot
Copy link
Contributor Author

omatrot commented Jul 17, 2019

serializr has its own ref format that is not suitable to my needs.
I'm afraid I'm stuck, unless I have a way to turn dcerializer decorators into something that works with angular (like the corresponding API that serializ provides).

@tonysneed
Copy link
Contributor

Closing this for now, since best practice is to avoid sending entities over the wire that have cycles and use auto-mapper to handle the data transfer.

@omatrot
Copy link
Contributor Author

omatrot commented Jul 18, 2019

Just to be clear, this as nothing to do with circular dependencies. It's more about types being used before being declared.
I forked dcerialize to try to provide an API to mimic the decorators, as serializr does.

@tonysneed
Copy link
Contributor

Oh yes, was thinking of another issue. 😄 You mentioned JSON refs, but those would go away without JSON.NET preserving references to handle cycles.

I imagine using POJO's (Plain Old JavaScript Objects) as DTO's (Data Transfer Objects) would omit a ctor and solve your serialization issue?

@tonysneed tonysneed reopened this Jul 18, 2019
@omatrot
Copy link
Contributor Author

omatrot commented Jul 18, 2019

Why not try it with your tackable entities javascript sample augmented with a Web API sending the initial data and receiving the modified version ?

@tonysneed
Copy link
Contributor

Good idea to update the sample with an API. Let me see if I can find some time to do that.

@omatrot
Copy link
Contributor Author

omatrot commented Aug 21, 2019

Hi @tonysneed.
Did you had any time to look at it ?

@tonysneed
Copy link
Contributor

Haven’t had time for this. @lelong37 Does the URF sample use trackable entities?

@omatrot
Copy link
Contributor Author

omatrot commented Sep 4, 2019

Hi @tonysneed.
I have been in touch with the creator of dcerialize (which is a french guy like me).
He told me to set emitDecoratorMetadata to false and see how it goes. I tried in a fork of your trackable entities js sample, and it seems to work.

I'll try that in my project and get back to you with results.

@tonysneed
Copy link
Contributor

That sounds like great news! If all goes well, why don’t you submit a pull request?

@omatrot
Copy link
Contributor Author

omatrot commented Sep 4, 2019

This works because decorator metata are mandatory for dependency injection in angular <= 7.5.
They are not anymore starting with Angular 8.
I guess I'll close the issue, because I can upgrade to Angular 8.

@omatrot omatrot closed this as completed Sep 4, 2019
@omatrot omatrot changed the title [Guidance] How to properly desierialize entity graphs coming from the server? [Guidance] How to properly deserialize entity graphs coming from the server? Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants