Return instances of model class

[jthomerson]

Is your feature request related to a problem? Please describe.
I'm frustrated that I interact with the model class everywhere to create and write data, but then when I read data (get/query/scan/etc), I get raw JS objects back instead of instances of the model class.

Describe the solution you'd like
I would like store.get('some-key').exec() to return the actual instance of the model. The type signature says it does, but it doesn't ... it actually returns a raw JS object with the same fields as the model class.

Describe alternatives you've considered
Every model that I define will have to have a public constructor(o: ModelType) that takes in the raw object and "hydrates" the model object. That's super repetitive, and (to me) defeats the purpose of the "ORM" (I know this isn't technically an ORM, but it's filling that need in a DynamoDB world).

Related: see #220

[michaelwittwer]

@jthomerson thanks for reporting this issue. This is not the first time we get this request, as you realized yourself. So I hope to be able to kickoff a first code change to get this implementation started, will keep you posted.

[jthomerson]

@michaelwittwer any update on this? Alternatively, do you have any guidance on where / how you would implement it? It's the biggest nitpick I have with this library, which otherwise suits my needs very well. I could try contributing the feature, but I'm sure you have in mind a specific interface / way that it would work.

[mariobittencourt]

Hi, I wonder if you have any news on this topic. I wish I could help with the implementation but it is not the case at the moment.
I make use of setters and getters and I need to actually access the other functionalities the real class would have.
Regards,

[jthomerson]

Same question as @mariobittencourt. I've built dozens of classes with DE this past week, and this has bitten me so many times; I'm constantly copy/pastaing a bunch of ugly static methods to convert DB records into instances.

Additionally, as I built one today, I thought of a related usecase - it needs to support not just methods like store.query().foo().exec() returning real instances, but also some kind of helper so that if we're operating on a DynamoDB stream, we can send the stream object to DE and get an instance back. The problem right now is that when I get the stream object, it has the names used inside of @Property({ name: 'foo' }), whereas the DB record I get back from DE's store.query(), store.get(), etc is the private name of the fields. For example:

class Foo {

   @Property({ name: 'bar' })
   private _bar: string;

}

• DE query / get / etc will return { _bar: 'abc' }
• DDB stream will return { bar: 'abc' }

[mariobittencourt]

Hi @michaelwittwer and @simonmumenthaler , if possible provide a feedback so I can understand if I should wait or move and try to look for alternatives, what would be sad given how interesting and easy this package behaved.
Regards,

[simonmumenthaler]

Hi @jthomerson @mariobittencourt

see lab/#278-model-instance

I basically just return Object.assign(new modelConstructor(), model) where model is the plain data object (the one returned until now).

This way the values are defined directly on a new instance or through a setter if present.

Would this work for you or do you might have another (better?) approach?

[mariobittencourt]

Hi @simonmumenthaler thank you for the reply. I will try to use locally.
If looks fine as it does the job. I left a comment in your branch when having private constructors for the object we are trying to reconstruct.

[jthomerson]

@simonmumenthaler that's basically what I've been doing for my models, so yes, I think it generally works. As @mariobittencourt mentioned, it would be nice to be able to use private constructors, but right now that doesn't seem possible because when I try to create the store, it doesn't work when my constructor is private. So, that may be a separate issue.

One other edge case that I've come across is where I need to do something async immediately after construction. I've been using a static factory method for this - clients who need to construct an object do so through the static factory method. To be able to accomplish that with Dynamo-Easy, perhaps one of these ideas would help:

• Add an annotation like @OnAfterConstruction that can be applied to a [possibly async] method. In the DE code, after it instantiates the object, if there's a method with this annotation, it's automatically called; if the method is async, it awaits the promise.
• Or, add an annotation like @RecordConstructor or something - it could be applied to a static method on the model. When DE loads a record from the DB, it will by default use the class constructor, but if it finds a static method with the @RecordConstructor annotation, DE uses that instead of the constructor.

Of course, those are both additional features above-and-beyond the basic default behavior that this issue is focused on - which I think is addressed by your lab branch.

Let me know if those comments don't make sense or you have questions.

[simonmumenthaler]

@mariobittencourt you can use the published prerelease version @5.7.0-pr278.1 to test this approach

[mariobittencourt]

@mariobittencourt you can use the published prerelease version @5.7.0-pr278.1 to test this approach

Hi, @simonmumenthaler it worked for me with both public and private constructors. Thank you.

@jthomerson I used this simple code to test private constructor: https://gist.github.com/mariobittencourt/8341b6d92ba4c3dc31f0859010c2a1f3

Maybe you can take a look and see if there is something different that would cause issues in your case.

[mariobittencourt]

@simonmumenthaler one question. I wonder how did you get the modelConstructor?

I have a situation where I will have some properties that are polimorphic (example, I have a collection of events and the events can be of different types).

I was planning on having something like this:

// In the class
// ...
@CollectionProperty({itemMapper: DomainEventMapper})
    private events: Array<DomainEvent>;

// the mapper
export const DomainEventMapper: MapperForType<DomainEvent, StringAttribute> = {
    toDb: propertyValue => ({ S: `${JSON.stringify(propertyValue)}`}),
    fromDb: propertyValue => {
        const model = JSON.parse(propertyValue.S);
        return Object.assign(new modelConstructor(), model);
    },
}

In your code you have the modelConstructor being injected. In my case how can I get the modelConstructor? I would add the @model in each event.
Or is there a better approach in this case?

[jthomerson]

@simonmumenthaler it doesn't seem like this ever landed in the mainline. Do you have plans to merge and release this?

Tangentially: to try to determine if it had landed, I needed to use git log extensively since I'm on 5.5.0 and you're now up to 7.1.0. Do you have any plans to have a CHANGELOG?

[michaelwittwer]

@jthomerson thanks for coming back to this. Your assumption is right, this never landed so far. We would love to merge this, but didn't have the time to verify on our side. What I could offer is releasing a new version branching of the head (v7.1.0) with the Return instances of model class merged in, so you could use that, and report back any issues or confirm that it works. Would that make sense for you? (check the link bellow for BREAKING CHANGES)

Concerning the CHANGELOG, we don't have one in the source yet, but you can find the CHANGELOG under releases.

[jthomerson]

@michaelwittwer thanks! Yes, if you're able to make a version of this feature off of 7.1.0, I can test it out this week / next week as I'm working on some new model classes for a new service I'm building.

[michaelwittwer]

@jthomerson just released a new version -> v7.2.0-pr278.1.

thanks for verifying!

[jthomerson]

Still testing, and will probably have more feedback, but wanted to pass along this initial thought: This will definitely need to be labeled as a breaking change. I upgraded and a bunch of model classes that do not have a default constructor broke. For example, if they are constructor(foo: string) { this.foo = foo.toLowerCase(); } ... they'll break.

Initial thoughts on how to overcome this is to support pluggable constructors for the model. In otherwords, similar to how you do this: defaultValueProvider for Property. Example:

construct(o: Bar): Bar {
   return Object.assign(new Bar(o.foo), o));
}

@Model({ constructor: construct })
class Bar {

   public constructor(foo: string) {
      this.foo = foo.toLowerCase();
   }

}

One other thought while I'm here and so I don't forget: the comment above from June 3rd about needing access to a mapper so you can use this on stream objects ... have you given any thought to that yet?

[jthomerson]

I don't think I have any more feedback other than what I left last above. That limitation made it where I really couldn't use this in our existing apps.

Sum-up: I think without a way of supporting non-default constructors, you'd basically have to design your model classes around Dynamo-Easy. That has a long-reaching effect: if I can't require certain fields in my constructor, then some fields on my model class have to become optional, which starts to break things. If you want any required fields, you typically require them in your constructor. So if we can't do that, then how do we build our models?

[jthomerson]

@michaelwittwer any updates on this?