On Fri, 21 Jun 2019, Sven Barth via fpc-pascal wrote:
Hello together!
I've played around a bit with the SQLDBRESTBridge and encountered some
troubles which I wanted to check up here on the list whether those are
indeed bugs or not before reporting them to Mantis.
For the tests I've used FPC 3.0.4 and Lazarus 2.0 with the
SQLDBRESTBridge and the Lazarus examples being from trunk on Friday 14th
June.
As server I used the restmodule demo in
$lazarus/components/sqldbrest/demo/restmodule and changed it to use
SQLite3 instead of PostgreSQL.
For the client I used the jsonclient demo in
$lazarus/components/sqldbrest/demo/jsonclient.
I created the database using the expenses-sqlite.sql script in
$fpc/packages/fcl-web/examples/restbridge/expenses-sqlite.sql (and no I
didn't execute expenses-data.sql at first which will be important
further down ;) ).
I started the server and tried to access localhost:7331/REST/metadata (I
changed the port from 8080), but this resulted in a 404 error which was
displayed as an empty page (maybe something isn't correctly working
here?). Using localhost:7331/metadata resulted in a "Not found" exception.
This I fixed. I had noticed the same, but didn't have time to fix.
It is fixed now.
So I started to research what went wrong and found various points:
- the metadata was coupled with the rdoConnectionInURL flag in
TSQLDBRestDispatcher.DoRegisterRoutes instead of rdoExposeMetadata (see
patch exposematadata.patch)
This is not quite correct. The rdoConnectionInURL check is also necessary:
Both connection/metadata and /metadata must work.
connection/metadata is handled by HandleRequest (FindSpecialResource)
So a special registration for metadata must be handled here.
But there was indeed a bug in the code, I fixed it properly.
- then it turned out that the routes are only registered once the module
was loaded and thus they would never be found so I adjusted
TSQLDBRestModule.HandleRequest to call the router again (this of course
required to change the BasePath of the TSQLDBDispatcher in the example
to 'REST' instead of '') (see patch reroute.patch) - this point I'm
definitely not sure whether it is the right solution (but in the end it
works)
This is not the correct solution.
The module itself handles the routes: the module is
registered e.g. under /REST and then the request is dispatched to the
router, and the dispatcher will take care of the rest.
This means of course that when you use a restmodule, the dispatcher
'RegisterRoutes' must not be called at all, and this can be achieved by
setting Active to False.
Maybe the meaning of active is a bit obtuse. I put a comment in when I made
it, but if that goes unnoticed...
In general, I added the rest module only for completeness. I think the
simple datamodule approach (as demoed in restbridge demo) is the better way:
this keeps the schema in memory. for cases where you want to control the
connections through the REST api, this is needed anyway.
The REst Module has the additional disadvantage that you must have an
initial /REST/ or whatever part in your URL. With the dispatcher on a
datamodule, you can skip this if so desired...
- then the problem was that the module's route ('REST/*/') took
precedence over the routes added by the TSQLDBRestDispatcher, so I
implemented a sorting of the routes so that more specific ones are
encountered first (see patch sort-routes.patch) - don't know whether
this is the right approach either, but this *is* a problem
No, see above.
- then the fact that TSQLDBRestModule is by default set to wkOneShot
lead to the routes being registered again or more precisely the old
routes being left over and pointing to a freed TSQLDBRestDispatcher
instance; thus I added a call to UnRegisterRoutes to
TSQLDBRestDispatcher.Destroy (see unregister-routes.patch)
Well, since they are not supposed to be registered in the first place...
With these changes the server worked, but then I encountered two
problems in the client:
- if the database table is empty an empty JSON object ('{}') is returned
resulting in the client complaining about the missing metadata (after
this I executed the expenses-data.sql script)
Indeed.
I fixed that, an error on my end, probably got in after some refactoring :(
- when changing from one resource to another I got an exception that the
operation is not possible on an active dataset; I solved this for me by
closing the dataset before executing LoadFromStream (see patch
jsonclient.patch)
Applied, thanks !
And an additional note regarding the expenses-sqlite.sql script. The
intention appears to be that the ID columns are auto increment. This
will however *only* work if the type of the columns is "integer primary
key". "bigint primary key" won't work here. See also:
https://www.sqlite.org/autoinc.html
Thus as with patch expenses-sqlite.sql I updat the SQL script for that
database.
Yes, the autoincrement in sqlite is rather weird, it seems I forgot to adapt
the script for this :(
I applied your patch, thank you !
If these are indeed bugs I'll either commit the patches as is (e.g. the
one for the SQLite script is rather surely a bug) or open a bug report
for the more complex ones.
Well, I have applied patches where necessary.
I haven't tested more yet, so I don't know whether there are any further
problems. :)
Well, I yesterday created a demo for TMS Software's TMS Web core using the
SQLDB Rest bridge as a backend, so I'm pretty sure it works 99%.
They will include a component that can be dropped on a web form which
will allow you to enter SQL etc.
With your patches, we got closer to 100%, so many thanks for that !! :-)
Michael.
_______________________________________________
fpc-pascal maillist - fpc-pascal@lists.freepascal.org
https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
