mORMot Open Source

trx

Member

Registered: 2015-08-30

Posts: 30

A bug in TSQLPropInfoRTTIDynArray.SetValue

Hi,

I was trying out mORMot's DDD features and have noticed a bug which i believe is in the implementation of TSQLPropInfoRTTIDynArray.SetValue.
For example, using a class like the following (with TSQLRestServerDB and SQLite3):

  TTestSubObjectName = type RawUTF8;
  TTestSubObject = class(TSynPersistent)
  private
    FName: TTestSubObjectName;
  published
    property Name : TTestSubObjectName read FName write FName;
  end;
  TTestSubObjectObjArray = array of TTestSubObject;

  TTestObject = class(TSynAutoCreateFields)
  private
    FSubObjects: TTestSubObjectObjArray;
  published
    property SubObjects : TTestSubObjectObjArray read FSubObjects write FSubObjects;
  end;
  TTestObjectObjArray = array of TTestObject;	

  TJSONSerializer.RegisterObjArrayForJSON([
    TypeInfo(TTestSubObjectObjArray), TTestSubObject,
    TypeInfo(TTestObjectObjArray), TTestObject
  ]);	
	
  // Persistence object.
  TSQLTestObject = class(TSQLRecord)
  protected
    FSubObjects: TTestSubObjectObjArray;
  published
    property SubObjects : TTestSubObjectObjArray read FSubObjects write FSubObjects;
  end;

   
If you create one test object with several subobjects and save (commit) it, everything works as expected, when you select and get the object again, the subobjects are there.
The problem arises when you create, add and commit several such test objects at once. If you select and get them, you will notice that there are no subobjects.

I did some digging around and I see that if you commit one object, its SubObjects value ends up in the database as TEXT whereas if you commit multiple objects, their SubObjects value ends up as BLOB.
I think this is because batch insert uses prepared statements and explicitly sets the field type to BLOB because SubObjects property is a DynArray. Single insert on the other hand saves everything as text.

When retrieving the objects from the DB, the implementation of TSQLPropInfoRTTIDynArray.SetValue expects JSON text.
But because the field was saved as a BLOB, it gets a base64 encoded value instead which does not get handled properly.

procedure TSQLPropInfoRTTIDynArray.SetValue(Instance: TObject; Value: PUTF8Char; wasString: boolean);
var tmp: TSynTempBuffer;
    da: TDynArray;
begin
  GetDynArray(Instance,da);
  if Value=nil then
    da.Clear else
    try
      // fObjArray is not nil, so it will attempt jump to da.LoadFromJSON which does not expect a base64 encoded input.
      if (fObjArray=nil) and Base64MagicCheckAndDecode(Value,tmp) then
        da.LoadFrom(tmp.buf) else 
        da.LoadFromJSON(tmp.Init(Value));
    finally
      tmp.Done;
    end;
end;

The following quick fix solves the problem:

procedure TSQLPropInfoRTTIDynArray.SetValue(Instance: TObject; Value: PUTF8Char; wasString: boolean);
var
  tmp: TSynTempBuffer;
  da: TDynArray;
begin
  GetDynArray(Instance,da);
  if Value = nil then
    da.Clear else
    try
      if not Base64MagicCheckAndDecode(Value,tmp) then
        tmp.Init(Value);
      if fObjArray = nil then
        da.LoadFrom(tmp.Buf) else
        da.LoadFromJSON(tmp.buf)
    finally
      tmp.Done;
    end;
end;

The above problem has another side effect.
For example, you commit one object and after that you commit two in a batch.
You then retrieve all inserted objects, and see that the first object is correct, but the other two are not.
Their SubObjects field has the value of the first object. This happens because when they get filled, the same record (TSQLRecordFill.fTableMap.Dest) is reused.
So the first object gets filled properly, the other two attempt to TSQLPropInfoRTTIDynArray.SetValue but fail, so they end up with the first one's data.

This problem does not occur with eg. TSQLRestServerFullMemory.
I have created a quick test application which reproduces the problem : http://txlab.org/objArrayPublishedPropTest.zip

Thanks,
Svetozar Belic.

Last edited by trx (2018-01-21 17:28:47)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,655

Website

Re: A bug in TSQLPropInfoRTTIDynArray.SetValue

Shouldn't the batch problem be fixed instead?

TSQLPropInfoRTTIDynArray.GetJSONValues() calls W.AddDynArrayJSONAsString() in this case, and therefore write the content as text, not as base-64.
Then it should be written as JSON text in the DB, not as blob.

But there was indeed a problem with external DB.
Please check https://synopse.info/fossil/info/e7d106ff7a

Now your test program works with no problem.

trx

Member

Registered: 2015-08-30

Posts: 30

Re: A bug in TSQLPropInfoRTTIDynArray.SetValue

When commiting the batch, in TSQLRestServerDB.InternalBatchStop that T*DynArray field is determined to be ftBlob at line 2008 (latest master branch) :

Types[f] := Props.Fields.List[prop].SQLDBFieldType;

Later in that same method, a statement is prepared and field values are bound using the determined types:

          for f := 0 to valuesCount-1 do begin
            if GetBit(ValuesNull[0],f) then
              fStatement^.BindNull(f+1) else
              case Types[prop] of
              ftInt64:
                fStatement^.Bind(f+1,GetInt64(pointer(Values[f])));
              ftDouble, ftCurrency:
                fStatement^.Bind(f+1,GetExtended(pointer(Values[f])));
              ftDate, ftUTF8:
                fStatement^.Bind(f+1,Values[f]);
              ftBlob:
                fStatement^.BindBlob(f+1,Values[f]);
              end;

This field's data (a json string) is therefore saved as a ftBlob. Later when you retrieve the row in unit SynSQLite3, TSQLRequest.FieldsToJSON the field is a ftBlob which gets encoded to base64. When this base64 encoded value ends up in TSQLPropInfoRTTIDynArray.SetValue, it is not decoded first and fails to be processed.

I am not sure where this should be fixed, you are more familiar with the code. Correcting this in TSQLPropInfoRTTIDynArray.SetValue seems the like cleanest approach to me because I don't see why it wouldn't accept base64 and be a T*ObjArray.

Thanks ab,

Svetozar Belic.

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,655

Website

Re: A bug in TSQLPropInfoRTTIDynArray.SetValue

Properly setting SQLDBFieldType is what I fixed in my latest commit.

Please check my previous post.

trx

Member

Registered: 2015-08-30

Posts: 30

Re: A bug in TSQLPropInfoRTTIDynArray.SetValue

Yeah, I see you fixed the source of the problem.

Thank you!

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,655

Website

Re: A bug in TSQLPropInfoRTTIDynArray.SetValue

So maybe you could close the pull request on github?

trx

Member

Registered: 2015-08-30

Posts: 30

Re: A bug in TSQLPropInfoRTTIDynArray.SetValue

Yeap, I forgot to do that, done.