SierraSoftworks / Iridium

A high performance MongoDB ORM for Node.js
http://sierrasoftworks.github.io/Iridium/
Other
570 stars 25 forks source link

Idea/Feature: Update Behavior Decorators #116

Open CatGuardian opened 6 years ago

CatGuardian commented 6 years ago

Hi Ben (is it ok to call you that?),

I just had a brilliant idea that I think will take Iridium to the next level and really enhance the functionality.

The idea is provide a way to specify, easily, the behavior of how a specific property is updated when the save() or update() or anytime the built in dif algorithm is ran to create the $changes object.

For example, let's say I have a property named timeLeft which is a number type. I'm assuming the default dif algorithm will utilize the $set operator of the $changes object to simply replace what the database currently has with what value timeLeft has at the time of running the dif algorithm. But what if I really want to utilize the $inc operator to increment whatever value the database currently has with the result of (_original.timeLeft - _modified) is. Then I would have to override the onSave and write custom code every time I wanted to do something different than the default. And if I had to do this a lot of places in my objects then that would start to get unnecessarily repetitive. (Also I'm not even sure if onSave is the only place where the default dif algorithm is run, and if it ran elsewhere then I wouldn't be able to easily ensure I use the $inc everywhere)

Imagine if I could just use a decorator to do this!

// Signature is for example only, not really a thought out recommendation
@Property(Number, ..., { useInc: true } );
timeLeft

And through the power of decorators the dif algorithm will see that I wanted to use $inc and that the property the dif algorithm is looking at is typeof 'number' so it would know how to utilize the $inc operator like I mentioned with the (_original.timeLeft - _modified)

The $inc is only one example! The bigger picture is a whole bunch of defined updateOptions which could even behave differently depending on the type it applies to. Like maybe we have an add update option that when applied to an array only adds new elements, never removes any by utilizing the appropriate update operator; but when used on a string it will add the modified value to the end of the string in the current database (if that is an update operator).

MongoDB Update Operator Reference: link

You get the point.

We could extend the current @Property decorator or we could make a new decorator to add this behavior.

What are your thoughts? I think this is something that is missing and definitely needed because from talking to people about MongoDB they are scared to go to it because of the ACID concerns as well as other data overwriting concerns like data being overwritten when they don't want it to be. The built in dif algorithm which updates only the portions that changed does a good job of helping these concerns but I think it can be smarter!

notheotherben commented 6 years ago

Hey @CatGuardian, yeah Ben is perfectly fine by me.

Very cool idea and definitely something I'd be willing to add. As a matter of fact, the diff algorithm does have support for using atomic number updates ($inc) but it's currently a global option and not one that is exposed for configuration outside Iridium's internal API. The reason it isn't enabled by default is that sometimes $inc isn't what someone is looking for with a numeric property, so I disabled it and left it at that.

The idea of having some control over exactly how diffs are performed, however, should go a long way towards alleviating that concern and potentially even supporting new use cases.

If I had to pick a decorator, I'd say a new one would be best - currently @Property is tightly coupled to the schema and changing its API would be non-trivial (likely a breaking change) while adding a new decorator would almost certainly be doable within a minor version update or even within the alpha.

As I'm sure you've seen, the way Iridium handles these kinds of things is adding a static field onto your extends Instance classes which specifies the overrides for specific properties, so it would make perfect sense to add a new diffHandlers: { [property: string]: DiffHandler<any> } static property and a decorator to populate it.

As for how the DiffHandler would work, you could probably have something along these lines:

interface DiffHandler<T> {
  (newValue: T, oldValue: T, ctx: DiffContext<T>): void;
}

interface DiffContext<T> {
  set(value: T): void;
  unset(): void;
  inc(value: number): void;
  mul(value: number): void;
  push(...values: T[]): void;
  pull(...values: T[]): void;
  child(path: keyof[T]): DiffContext<T[path]>;
}

That would allow you to define a diff handler for $inc like this:

(oldValue: number, newValue: number, ctx: DiffContext) => ctx.inc(newValue - oldValue)

Or if you wanted to have a custom object that did something odd, perhaps you would do this:

interface CustomObject {
  value: number;
}

(oldValue: CustomObject, newValue: CustomObject, ctx: DiffContext<CustomObject>) => ctx.child("value").mul(newValue/oldValue)

So the full example might look something like this:

interface MyModelDoc {
  _id?: string;
  upvotes: number;
}

class MyModel extends Iridium.Instance<MyModelDoc, MyModel> {
  @Iridium.ObjectID
  public _id: string;

  @Iridium.Property(Number)
  @Iridium.Diff((oldValue, newValue, ctx) => ctx.inc(newValue - oldValue))
  public upvotes: number;
}

Of course, one has to keep in mind that this doesn't solve all ACID concerns, specifically there is no cross-document transactional safety (although the upcoming support for retrying operations with a transaction ID should assist with that, and there are some good patterns which avoid the issues that a lack of native ACID compliance introduce).

From a usability perspective though, I think it's a great idea and it'll open up the option of relying on Instance.save() for more complex workflows rather than needing to fall back on manually providing your diff object (which you can do via Instance.save(diff)) or using Model.update().

CatGuardian commented 6 years ago

Cool. I'm glad you think so.

It isn't anything I need right now. I was just doing some thinking ahead for my current project and I found use cases where something like this might be needed.

So I wouldn't reprioritize what you have been working on to focus on this. And if it becomes time when I need it, then I'd be willing to implement it at that time.