Closed DellanX closed 11 months ago
dpimonov
commented
1 year ago Hi @DellanX,
I need your entity configs to fully understand an issue. Without looking at the configs it is hard to restore the whole serialization process from payloads. Thanks in advance.
DellanX
commented
1 year ago I'm struggling to be able to come up with a replication of this on the backend, as I don't know how to create polymorphic relationships there.
Here is pretty much the following class structure:
classDiagram
class Occasionable{
occasions: Occasion[]
occurrences: Occurrence[]
occurrences: Override[]
}
class User{
}
class Calendar{
}
class Occasion{
start: Date;
end: Date;
}
class Occurrence{
}
class Override{
}
Occurrence --|>Occasion
Override--|>Occasion
User--|>Occasionable
Calendar--|>Occasionable
Occasion--|>Occasionable
Occasionable-->Occasion
Occasionable-->Occurrence
Occasionable-->Overrideoccasionable.ts
import type Nullable from '@core/services/v1/data/types/nullable';
import type CalendarFeed from './calendar-feed';
import type ICalendar from './icalendar';
import { Occasion } from './occasion';
import { Model } from '@core/services/v1/data/models/model';
import { Property } from '@dipscope/type-manager';
import { Override, Occurrence } from './occurrence';
export abstract class Occasionable extends Model implements ICalendar {
@Property(Array, [() => Occasion])
public occasions: Nullable<Occasion[]>;
/**
* The more processed form of occasions.
* Occasions are able to repeat, Occurrences are each instance
*/
@Property(Array, [() => Occurrence])
public occurrences: Nullable<Occurrence[]>;
@Property(Array, [() => Override])
public overrides: Nullable<Override[]>;
/**
* @deprecated use calendar feedable instead
*/
public 'calendar-feeds': CalendarFeed[];
}user.ts
import { Property, Type } from '@dipscope/type-manager';
import { JsonApiResource } from '@dipscope/json-api-entity-provider';
import Model from './model';
import type IUser from '../interfaces/iuser';
import UserSession from './user-session';
import type Nullable from '../types/nullable';
import { Occasion } from '@planner/services/v1/data/models/occurrence';
import { Calendar } from '@planner/services/v1/data/models/calendar';
import { CalendarFeed } from '@planner/services/v1/data/models/calendar-feed';
import { Occurrence, Override } from '@planner/services/v1/data';
import type { CalendarFeedable } from '@planner/services/v1/data/models/calendar-feedable';
@Type()
@JsonApiResource({ type: 'users' })
export class User extends Model implements IUser, CalendarFeedable {
public type = 'users';
@Property()
public email!: string;
@Property(Array, [() => Calendar])
public calendars!: Calendar[];
@Property(Array, [() => CalendarFeed])
public 'calendar-feeds': CalendarFeed[];
@Property(Array, [() => Occasion])
public occasions: Nullable<Occasion[]>;
@Property(Array, [() => Occurrence])
public occurrences: Nullable<Occurrence[]>;
@Property(Array, [() => Override])
public overrides: Nullable<Override[]>;
@Property(Array, [UserSession])
public sessions!: UserSession[];
}
export default User;
export function isUser(x: object): x is User {
return (x as User).email ? true : false;
}calendar.ts
import { SharedResource } from '@grants/services/v1/data/models/shared-resource';
import { Property, Type } from '@dipscope/type-manager';
import { JsonApiResource } from '@dipscope/json-api-entity-provider';
import { Occasion } from './occurrence';
import type Nullable from '@core/services/v1/data/types/nullable';
import CalendarFeed from './calendar-feed';
import type { CalendarFeedable } from './calendar-feedable';
import { Occurrence, Override } from './occurrence';
@Type()
@JsonApiResource({ type: 'calendars' })
export class Calendar extends SharedResource implements CalendarFeedable {
public type = 'calendars';
@Property(Array, [() => CalendarFeed])
public 'calendar-feeds': CalendarFeed[];
@Property(Array, [() => Occasion])
public occasions!: Occasion[];
@Property(Array, [() => Occurrence])
public occurrences!: Occurrence[];
@Property(Array, [() => Override])
public overrides: Nullable<Override[]>;
}
export function isCalendar(x: object): x is Calendar {
return (x as Calendar).type === 'calendars';
}
export default Calendar;
occasion.ts
import { User } from '@core/services/v1/data/models/user';
import { SharedResource } from '@grants/services/v1/data/models/shared-resource';
import type EventStatus from './event-status';
import type { UserOwnedResource } from '@grants/services/v1/data/models/user-owned-resource';
import type Nullable from '@core/services/v1/data/types/nullable';
import { Property } from '@dipscope/type-manager';
import type { IdType } from '@core/services/v1/data/interfaces/iidentifier';
import CalendarFeed from './calendar-feed';
import { Occasionable } from './occasionable';
import { Occurrence, Override } from './occurrence';
export enum Confirmation {
Confirmed = 'confirmed',
Tentative = 'tentative',
Declined = 'declined',
None = 'none',
}
export abstract class Occasion extends SharedResource implements UserOwnedResource, Occasionable {
@Property()
public title!: string;
@Property()
public description!: string;
@Property(Date)
public start!: Date;
@Property(Date)
public end?: Nullable<Date>;
@Property(Boolean)
public allDay!: boolean;
@Property()
public rrule?: string;
@Property(String)
public status!: EventStatus;
@Property(Array, [Date])
public exdates?: Date[];
@Property()
public calendarType?: IdType;
@Property()
public calendarId?: IdType;
@Property()
public occurrenceId: Nullable<IdType>;
@Property()
public userId!: IdType;
@Property(Array, [() => CalendarFeed])
'calendar-feeds': CalendarFeed[];
@Property(Array, [() => Occasion])
public occasions: Nullable<Occasion[]>;
@Property(() => Occasionable)
public calendar?: Occasionable;
@Property(() => User)
public user!: User;
@Property(Array, [() => Occurrence])
public occurrences!: Occurrence[];
@Property(Array, [() => Override])
public overrides: Nullable<Override[]>;
}occurrence.ts
import { Type } from '@dipscope/type-manager';
import { Occasion } from './occasion';
import { JsonApiResource } from '@dipscope/json-api-entity-provider';
export type OverrideMetadata = {
recurrenceId?: Date | null;
start?: Date | null;
end?: Date | null;
};
@Type()
@JsonApiResource({ type: 'occurrences' })
export class Occurrence extends Occasion {
public type = 'occurrences';
}
@Type()
@JsonApiResource({ type: 'overrides' })
export class Override extends Occasion {
public type = 'overrides';
}
export function isOverride(x: object): x is Override{
return (x as Override).type === 'overrides';
}The Create Event view
const event = new Occurrence();
// OMITING Attributes
event.calendar = calendar;
event.user = user;
occurrenceStore.add(event);
I was able to get this kinda working by adding JsonApiResource to the Occasionable and Occasion classes; however, the type was still incorrect.
I figure that maybe by declaring a discriminant/discriminator I may be able to make another step; however, I haven't had any luck with using those settings with the JsonApiEntityProvider.
A similar structure that you could use to reproduce would be to create a chat model that can also store messages. Then when storing a new message one should be able to specify if it's added to a chat or to a user.
Unfortunately I am not familiar enough with JsonApiDoNet to set up a polymorphic relationship. But what I'd imagine would be something such as:
classDiagram
class Messageable{
messages: Message[]
}
class User{
}
class Chat{
}
class Message{
content: string
}
class Reaction{
}
Reaction--|>Message
User--|>Messageable
Chat--|>Messageable
Message--|>Messageable
Messageable-->MessageHi @DellanX, thanks for detailed info!
Yep, you did it correctly with discriminant / discriminator. You have to follow this guide: https://dipscope.com/type-manager/defining-configuration-manually#configuring-usage-of-polymorphic-types. For your case seems that Discriminator is type as you declare it everywhere and Discriminant is the same as resource type. But... looks like I forgot to treat polymorphic case in adapter when reading from document object response and also in some other places. Some information might be lost during creation of serialized / deserialized entities. I am going to think how to fix that.
Can you meanwhile provide a raw JSON:API response from your server? I don't know how to configure polymorphic relations with JSON.NET either and don't have much time now to investigate. So I will try to provide a fix for things which I already see and we can check if it will work for you.
dpimonov
commented
1 year ago Hi @DellanX,
I've added required changes to make polymorphic types work. It is not exactly how I want the whole configuration behave but it is sufficient if you are not combining different kind of datasource for the same entities. I can rethink adapter part later when someone encounter a case I have in mind.
Basically you can omit Discriminator and Discriminant parts as I included this config by default when one specify JsonApiResource. So type set as Discriminator and value for a type set as Discriminant.
@Type()
@JsonApiResource({ type: 'overrides' })
export class Override extends Occasion
{
public type: string = 'overrides';
}But... to make it all work you have to also specify JsonApiResource for your base class. It does not matter what values you will set here but the serializer has to know that this class is actually a resource when serializing and deserializing objects. I've added a small test for JsonApi.Net library and it worked for me. Try changes from master if they will work for you and let me know.
DellanX
commented
12 months ago I poked around, and it seems like this did improve the polymorphic support a lot (possibly even fully).
I am adding in tests; however, have hit a snag on the backend. It seems like the /womans/:id/children and /mans/:id/children endpoints both return an empty array. Even though I can confirm that models have them as a parents. (It looks like the client code is pinging the routes correctly though).
That being said, given that all the testing I added passed on first try (excluding backend issues), I think you've covered this feature.
I've added in two test files: morphOne and morphMany
To add morph-to-one testing, I needed to modify the backend just a bit, adding a polymorphic-to-one relationship spouse. Then modified the client to consume that relationship as a toOne relationship.
All Tests passed on the first try! 😄 I'll do some fault insertion to ensure confidence.
[HasOne]
public Human? Spouse { get; set; }
public int? SpouseId { get; set; }To add morph-to-many testing, the children endpoint would work fine. But I am running into the issue with the backend not looking up any related models.
I think that the testing you added in the base spec file was adequate and ensures the clients functionality.
dpimonov
commented
11 months ago Hi DellanX, thanks for your PR. I've merged your changes as any additional tests are very helpful. I removed unused code but it is still in the repo history if will be required. With children relationship - there is a bit of miss configuration on the backend for database. Changing it will result in some test rewrite so I kept it for now. As everything works fine with polymorphic relations - i will close an issue and create a release. 🙂
DellanX
commented
11 months ago Ooof, I am on V2.2 on my system, and unfortunately polymorphic relationships still aren't working. I wonder if I am running into issues due to:
It is not exactly how I want the whole configuration behave but it is sufficient if you are not combining different kind of datasource for the same entities.
I have a few models
classDiagram
class Messageable{
messages: Message[]
}
class Occasionable{
occasions: Occasion[]
}
class SharedResource{
members: User[]
}
class User{
}
class Chat{
}
class Message{
content: string
user: User
messageable: Messageable
}
User--|>Occasionable
User--|>Messageable
Chat--|>Messageable
Chat--|>SharedResource
Messageable-->Message
Message-->Messageable
Message-->UserChat inherits from two abstract types, SharedResource and Messageable. I'm wondering if this is possibly the issue? (If I create a new message, and set the messageable to a chat, the payload still uses messageables as the Type)
In the meantime, I'll do some experiments to see if I can reproduce the issue on your codebase.
Also, if you'd like, I can definitely try to migrate your tests if you can point out what needs to happen for the backend support to be upgraded.
dpimonov
commented
11 months ago Hi DellanX,
I wonder if I am running into issues due to...My comment was more about when you have microservices and your model is duplicated in 2 of them with different set of properties and you have 2 different configurations. This is not about that case.
Chat inherits from two abstract types, SharedResource and Messageable. I'm wondering if this is possibly the issue?Yes - it might be an issue. I wonder how you were able to inherit from 2 abstract classes. How to do that in TypeScript? In any case I would rather go with composition over inheritance principle instead.
Also, if you'd like, I can definitely try to migrate your tests if you can point out what needs to happen for the backend support to be upgraded.C# does not allow multiple inheritance by design so there is no way to upgrade in case I understood everything corectly. 🙂
DellanX
commented
11 months ago So, it's pulled off by:
implementing all of the other abstract classes (using the type as if it was a C# interface)
I use the same discriminator, and re-implement the same decorators from the other types
dpimonov
commented
11 months ago Sorry, I still do not quite understand what is assigned to what and what result you expect without actual entity configs. I already see some things which for sure will go wrong with implementation you described as serialization engine will only see classes which you extend and not interfaces you implemented as they are just hints for a compiler. Any time you are going to serialize a type like implemented interface - it will simply does not work as there is not way currently in type script to browse implemented interfaces.
Can you please simply attach code snippet with your entity definitions and configurations + code sample what is created and attached to which properties? This will be much more sufficient for me as I will be able to see serialization / desesialization pipeline from this configs.
DellanX
commented
11 months ago Sure!
Right now, I am creating a message message.ts, and trying to upload it.
My chat mechanism has a sender user.ts and a receiver messageable.ts, which could either be a chat, group or user (I'll drop chat.ts, user.ts)
message.ts@Type()
@JsonApiResource({ type: 'messages' })
export class Message extends SharedResource implements UserOwnedResource {
public type = 'messages';
public aliasBy: string & keyof this = 'name';
constructor(name?: string) {
super(name ?? '')
}
// Base Properties
@Property()
public name!: string;
@Property(Boolean)
public mine!: boolean;
// Related Linkage
@Property()
public messageableType!: string;
@Property()
public messageableId?: IdType;
@Property()
public userId!: IdType;
// Related Models
@Property(() => Messageable)
public messageable!: Messageable;
@Property(() => User)
public user: Nullable<User>;
}chat.ts@Type()
@JsonApiResource({ type: 'chats' })
export class Chat extends SharedResource implements Messageable {
public type = 'chats';
constructor(name?: string) {
super(name ?? 'New Chat');
}
@Property()
unreadMessageCount!: number;
@Property(EntityCollection, [() => Message])
messages!: EntityCollection<Message>;
}user.ts@Type()
@JsonApiResource({ type: 'users' })
export class User extends Model implements Messageable, CalendarFeedable {
public type = 'users';
@Property()
public name!: string;
@Property()
public email!: string;
@Property()
public unreadMessageCount!: number;
@Property(EntityCollection, [() => Message])
public messages!: EntityCollection<Message>;
@Property(EntityCollection, [() => Chat])
public chats!: EntityCollection<Chat>;
// Omitted Extraneous fields
}shared_resource.ts@Type()
@JsonApiResource({ type: 'shared-resources' })
export abstract class SharedResource extends Model {
public aliasBy: string & keyof this = 'name';
public imageBy: string & keyof this = 'imageUrl';
constructor(name?: string) {
super();
this.name = name ?? 'New';
this.members = new EntityCollection<User>();
this.memberships = new EntityCollection<Membership>();
}
@Property()
public name!: string;
@Property(EntityCollection, [() => User])
public members: EntityCollection<User>;
@Property(EntityCollection, [() => Membership])
public memberships: EntityCollection<Membership>;
}messageable.ts@Type()
@JsonApiResource({type: 'messageables'})
export abstract class Messageable extends Model {
// Eager Loadable
@Property()
public unreadMessageCount!: number;
@Property(EntityCollection, [() => Message])
public messages!: EntityCollection<Message>;
}The philosophy here is that chat and user both implement the same interface messageable
Since they don't extend the class, I have to redefine the properties again.
That being said, I can now have a single model contain both members (from shared-resource) and messages (from messageable).
That being said, if I create a new message with messageable defined as a chat, the message still posts as:
{
"data": {
"type": "messages",
"attributes": {
"name": "test"
},
"relationships": {
"messageable": {
"data": {
"type": "messageables",
"id": "9a4a0afa-8ebb-4588-aeea-5efc65f16dcb"
}
},
"user": {
"data": {
"type": "users",
"id": "5fc169cd-d9c3-4094-98bf-c67b099e2cfa"
}
}
}
}
}Also, I have now successfully migrated all of my JsonAPI code to using this library. This library is amazing, and I'd definitely like to help in any way I can! 😊
In the meantime, I might see about studying your code to see how it works on a lower level.
dpimonov
commented
11 months ago I am glad that you like a library and hope you will have fun with a code. 😊
Thanks for detailed answer! I see what is wrong and why serialization does not work as you expected. The point is that implements keyword in TypeScript is just a hint and it outputs no code or any information like other programming languages like PHP or C# does. Because of that during serialization the only information which is available is inheritance tree. If class not directly extends Messageable - it is not a descendand of Messagable and there is no way to define that in TypeScript.
Solution 1 - Possible right now
You can extend your classes from Messageable and implement other classes as interfaces. This will look kinda similar to what you already have but may result in property and method duplications. I will not provide any code as this solution should be clear.
Solution 2 - Will not work but just as an info
What you have there is actually multiple inheritance case which is of course due to several reasons prohibit in many programming languages. However the most common solution to this problem is composition over inheritance pattern.
export class Messageable
{
public unreadMessageCount: number;
public messages: EntityCollection<Message>;
}
export class SharedResource
{
public members: EntityCollection<User>;
public memberships: EntityCollection<Membership>;
}
export class Chat implements SharedResource, Messageable
{
private messageable: Messageable;
private sharedResource: SharedResource;
public get unreadMessageCount(): number
{
return this.messageable.unreadMessageCount;
}
public set unreadMessageCount(value: number)
{
this.messageable.unreadMessageCount = value;
}
public get members(): EntityCollection<User>
{
return this.sharedResource.members;
}
public set members(value: EntityCollection<User>)
{
this.sharedResource.members = value;
}
}So basically we have 2 separate self sufficient or abstract classes Messageable and SharedResource. Chat implements them as interfaces and stores each instance as a property. Access is done through the getters and setters. When you have methods with complex logic in Messageable and SharedResource this solution helps to avoid code duplication. However serialization of such structure is not possible with TypeManager yet.
Solution 3 - Proposal to cover your case and solutions above
And as a proposal - your case gave me an idea that we can provide information about implemented classes through Type decorator for serialization system to collect this information and use later. However it requires special treatment not to spoil main serialization.
It might solve your issue without changing anything and also provide a way to serialize composite objects with interfaces. Because properties are already defined in interface classes it is not required to specify them one more time while implementing interface. There are 2 things to consider - such definition cannot be lazy and overrides should be possible. Last note is more for me.
As this part is deeply inside TypeManager - I can prototype it and you can write tests as your examples are great use case. As usual you will have a new version as soon as your tests pass. 😊
DellanX
commented
11 months ago Oh wow! This would actually explain a lot (I was running into some linting issues that could be explained by this)! I primarily use languages such as C++, C, C# and PHP and was expecting that the implemented types were tracked in its inheritance tree. Thanks for the info.
I'm going to experiment with option 2 and see how far I can get. I wanted to avoid composition, but I think for now that's my best path forward for the immediate future. (Clarification, I'm going to implement a hybrid of 1 and 2, a bit complicated, but easier than my old system)
I am definitely down to write some tests if that assists (It's what I do for my profession, the apps I build are personal projects)
dpimonov
commented
11 months ago Ok, great! I've added required changes to TypeManager repo. Here is an example with new Type option called parentTypeFns.
import { Type, Property } from '@dipscope/type-manager';
@Type()
export abstract class Entity
{
@Property(String) public id?: string;
}
@Type()
export abstract class UserStatus extends Entity
{
@Property(String) public title?: string;
}
@Type({
parentTypeFns: [UserStatus]
})
export class ActiveUserStatus extends Entity implements UserStatus
{
@Property(String) public title?: string;
@Property(Boolean) public active?: boolean;
}My simple test case passed. You can experiment with more of your complex cases. There is a folder spec/use-cases. You can create a file like complex-polymorphic-types.spec.ts as an example and put your code there. 😊
DellanX
commented
11 months ago I've added tests in the file recommended. Two cases are failing on serialization. I've commented those out as they could be false-failures. Finally I submitted a PR so you can take a peak. Still need to cleanup styling and add comments. (I mentioned in the PR on TypeManager)
DellanX
commented
11 months ago The data sends correctly! 🙂
Unfortunately, I'm still running into Descriminator/Descriminate errors. I'm going to go through and see if I can root cause. My dataset is pretty complex, so it might be that an implement type isn't configured.
Error: $: cannot define discriminant of polymorphic type. This is usually caused by invalid configurationYou should have correct types assigned to deserialize from objects back to entities. 🙂
DellanX
commented
11 months ago It Works!!!
Thank you for working on this! (Issue was in my configuration!)
Describe the bug
Ah, got a fun one now (Though could be worked around by #7) I have a model I am creating
Occurrenceand am attaching it to a 'calendar'.The complexity is that calendar comes in two types:
user-calendarandshared-calendar(User calendars just re-use the user object) So,calendaris a polymorphic type (and until now, was just an abstract class without aTypedecorator)When I try to add a new entity to the entityset, I see the following payload:
To which, it seemed to me like the JsonAPIEntityProvider wasn't able to key that the calendar is a related object. So, I went ahead and added the Type() decorator --- No change in payload Finally, I added the JsonApiResource decorator, giving me the next payload.
To Reproduce
I'll see if I can come up with a reduced example to reproduce the error later. But I have plans to be out and about running errands today.
Expected behavior
The expected payload should be the following for user-calendars
And for shared-calendars
Screenshots NA
Desktop (please complete the following information):
Additional context
Add any other context about the problem here.