foAutoGenerated in multiple fields

[azapater]

trafficstars

I've noticed that the foAutoGenerated field option is very linked to primary keys. It would be useful if other fields could be foAutoGenerated as well, mostly in insert situations. For example, creation timestamps are usually defined in triggers and after an insert, using ActiveRecord, the only updated value in the entity is the primary key.

It happens similarly with default values assigned in fields in the database. If a field has a default value, is marked as not null and the insert or update value is null, the database assigns that default value automatically. In this scenario, the same behaviour happens and the entity doesn't retrieve the default value assigned.

After quickly analysing the code, the method TMVCActiveRecord.ExecNonQuery has the RefreshAutoGenerated argument, but it only refreshes the primary key, not the rest of the possible autoGenerated fields which leads with an unsynced entity: the value is stored properly in the database but the entity doesn't have it.

IMO, the most accurate aproach to solve all these problems it would be to retrieve all fields after and insert or update (either if they have foAutoGenerated option assigned or not), just in case there are default values or triggers that modify any value and always asume the database is the source of truth. I know this is more resource demanding, because it implies an extra query to the database after each insert or update, but I think is the most trustful option. Maybe creating a global parameter to active or desactive this behaviour so users could prioritize performance over integrity depending on their scenarios could be a safer approach.

Another approach I have in mind that is less of a breaking change it would be to create a new method in the TMVCActiveRecord class with the name "refresh" or "refetch". This would simply update the entity with the current values in the database. It's more prone to integrity errors, because it implies the user would have to execute it manually in their code, but maybe it's easier to implement in the library.

If it's okay, I could try to analyse the code in more detail and PR a first approach for this but I would like your opinion first.

Best regards

[danieleteti]

Extending the "auto refresing" to other fields other than the PK is a good thing. However, it should be done in just one roundtrip to the db. Currently SQLite and MySQL don't support "returning" so it is necessary to concatenate 2 statement ("insert...";"select last_id") and it is ok... while for Interbase the have to rely on generators... so we have 2 call (get generator value and then do the actual insert) it is a not optimal approach. Also, for Interbase the only way to update all the updated field would be doing an actual select after the insert/update, and I really don't like it. I'm torn about it... would be sufficient to raise an exception id the engine doesnt' support the returning clause and the user uses foAutoRefresh for fields which are not PK?

[azapater]

I know what you mean...tricky decision. Maybe the aproach of raising an exception in case the engine doesn't support it is the most optimal way. In that case, it would be interesting to offer the option of using a method "refresh" or "refetch" straight in the TMVCActiveRecord objects (to make it easier to implement) . In this way, if a developer uses a non supported engine, at least they will have the option of doing it's own implementation but knowing that it implies a second query to the database.

I personally fixed this issue creating a subclass of TMVCActiveRecord with a couple tweaks and I use it just on the entities I need this auto-refresh behaviour. Pasting the code here just in case is useful for anyone. I know it implies a second query, but it gets the job done.

TAutoRefreshActiveRecord = class(TMVCActiveRecord)
  protected
    procedure OnAfterInsertOrUpdate; override;
  public
    procedure Refresh;
end;

implementation

procedure TAutoRefreshActiveRecord.OnAfterInsertOrUpdate;
begin
  inherited;
  self.Refresh;
end;

procedure TAutoRefreshActiveRecord.Refresh;
begin
  if not(GetPK.IsEmpty) then
    LoadByPK(GetPK.AsInteger);
end;