SchneiderInfosystems / FB4D

Open Source Library for connecting Firebase by Delphi VCL/FMX
https://www.schneider-infosys.ch/DelphiBlog/de/#FB4D
Apache License 2.0
181 stars 54 forks source link

Somethimes, GetSynchronous returns a array of documents, but without JSON inside #139

Closed arvanus closed 2 years ago

arvanus commented 2 years ago

Describe the bug Looks like at lines: https://github.com/SchneiderInfosystems/FB4D/blob/f9247a5e55e46fcb54ed18c517d40362b1091392/Source/FB4D.Document.pas#L238...L269 Specifically in my case https://github.com/SchneiderInfosystems/FB4D/blob/f9247a5e55e46fcb54ed18c517d40362b1091392/Source/FB4D.Document.pas#L261-L267 In the last line where you free JSONObj, you also are freeing fDocumentList[0], a solution would be to clone JSONObj at CreateFromJSONObj, not sure why this happens, because somethimes it work, sometimes dont, and at the demo it's working too At line you simply copy the JSON, that is latter freed: https://github.com/SchneiderInfosystems/FB4D/blob/f9247a5e55e46fcb54ed18c517d40362b1091392/Source/FB4D.Document.pas#L409 My (temporary) solution here was to simply clone fJSONObj := JSONObj.Clone as TJSONObject;

To Reproduce

//Console application 
    var fAuthProducao := TFirebaseAuthentication.Create(...);
    var fDatabaseProducao := TFirestoreDatabase.Create(..., fAuthProducao);
    var documents := fDatabaseProducao.GetSynchronous(['general', 'accountings'] );
    for var i:=0 to documents.Count-1 do
    begin
      var lDoc := documents.Document(i);
        writeln(lDoc.AsJSON.ToJSON); //Error 
    end;

Expected behavior Return the document, but nil found

Screenshots

Desktop (please complete the following information):

SchneiderInfosystems commented 2 years ago

It is much more complicated than I expected initially because it is called by different callers. Can you check my solution here?

https://github.com/SchneiderInfosystems/FB4D/commit/4b4e6c656d3d44ed80076b714cbcdfdeb41cb2db

With this change, the unit tests are passed, and no memory leaks occur.

arvanus commented 2 years ago

Looks like it's working in this commit Thanks