a scenario that before save hook may not cover

[clark0x]

@bajtos I'm trying migrate my codes to use the new hook, and I found a scenario that I can't drop the old beforeCreate hook: before creating a model, some related models need to be created or executed, but the creating model must be valid at first.

e.g., for a room booking system, we need to lock the room resource before create a booking:

Booking.observe('before save', function (ctx, next) {
    var booking = ctx.instance;
    booking.room(function (err, room) {
      room.lock(booking.time, next);
    });
});

Currently, the before save hook is called before checking if model is valid. If booking is invalid, room.lock() has already been executed, then we have to unlock the room.

The expect behavior is:

1. checking if creating booking data is valid
2. if booking data is valid, try lock the room
3. if success to lock the room, create booking, then send emails, blah blah...

What do you think?

[bajtos]

Hi @clarkorz, thank you for describing your use case in detail.

The before save hook is intended for case where you don't need to distinguish between different write operations (create, update, prototype.save, updateOrCreate, findOrCreate). Especially with findOrCreate, the hook may be executed even when the create operation is not performed at all, see your patch #419.

I am also wondering how does your application handle the case when the creation of booking fails (e.g. due to database error) just after the related models were already updated?

Setting those concerns aside, you can run the validation yourself in the hook and bail out on validation error.

Booking.observe('before save', function (ctx, next) {
  if (!ctx.instance) {
   // partial update, skip this hook
   return next();
  }

  // TODO: detect `.prototype.save()` and possibly other cases
  // when this is not an update. Or not?

  var booking = ctx.instance;
  booking.isValid(function(valid) {
    if (!valid) {
      return next(new booking.constructor.app.loopback.ValidationError(booking));
    }

    // business as usual
    booking.room(function (err, room) {
      room.lock(booking.time, next);
    });
  });
});

[bajtos]

In #545 (comment), I am proposing to add a hook that will allow you to clean up resources after save failed, either because a validation error or some other error.

I am wondering if this would be a good solution for you?

Booking.observe('before save', function (ctx, next) {
    var booking = ctx.instance;
    booking.room(function (err, room) {
      room.lock(booking.time, next);
    });
});

Booking.observer('save error', function(ctx, next) {
    var booking = ctx.instance;
    booking.room(function (err, room) {
      room.unlock(booking.time, next);
    });
});

BTW I am not sure that hooks are the right tool for this job. IIUC, you want to lock the booked room only on CREATE, not on UPDATEs. In which case it's probably better to implement a custom remote method instead of using hooks.

Booking.create = function(data, cb) {
  var self = this;

  self.isValid(function(valid) {
    if (!valid) return cb(new loopback.ValidationError(self));

    booking.room(function (err, room) {
      if (err) return cb(err);

      room.lock(booking.time, function(err) {
        if (err) return cb(err);

        Booking.base.create.call(self, data, function(err, created) {
          if (err) {
            // unlock the room
          }
          cb(err, created);
        });
      });;
    });
  });
};

[bajtos]

We can extend the create method to accept options.validate allowing the caller to skip the validation, in which case you would write

Booking.base.create.call(self, data, { validate: false }, function(err, created) {
  // ...
});

[bajtos]

Based on the discussion in strongloop/loopback#1226, we are thinking about adding a new hook called "persist" that can be used as a replacement for "afterValidate". See #559.

I am closing this issue, let's move the discussion to 559.

[bajtos]

@clarkorz FWIW, the new "persist" has been landed on master recently.