search

qgis / QGIS-Enhancement-Proposals

QEP's (QGIS Enhancement Proposals) are used in the process of creating and discussing new enhancements for QGIS
116 stars 37 forks source link

QEP 74: QGIS server code refactoring for QGIS 3.0 #74

Open rldhont opened 7 years ago

rldhont commented 7 years ago

QGIS Enhancement 74: QGIS server code refactoring for QGIS 3.0

Date 2016/09/27

Author René-Luc D'HONT (@rldhont)

Contact rldhont at gmail dot com

maintainers @rldhont, @dmarteau

Version QGIS 3.0

Summary

The QGIS 3.0 version is the right time to refactor the QGIS Server code. A part of the QGIS Server code that needs a refactoring is the way Services has been added.

A better way to define Services in QGIS Server would be to add them as plugins like the layer providers. With this architecture, Services will be self declared and stored and could be written in C++ and Python.

Dependencies

Some part of the current QGIS Server code has to be cleaned up:

Services provider are dynamically loaded modules. Services may register for one or more services.

A Service module provide QGS_ServerProvider_Init() C entry point, this entry point return a QgsServiceModule instance:

class QgsServerProviderModule {

    /** Ask provider to register all provided services */
    void registerSelf( QgsServerProviderRegistry* );

    /** Finalize/cleanup module  */
    void unRegisterSelf( QgsServerServiceRegistry* )
};

Service registry

The server provider registry act as a manager for modules and services:

class QgsServerProviderRegistry {

    /* Module management */
    /* ... */

    /** Retrieve a service by its name
    */
    QgsServerService* getService( const char* );

    /** Register a service by its name 

        This method is intended to  be called by modules for registering
        services. A module may register multiple services.
        The registry gain ownership of services. 
    */ 
    void registerService( const char* name, QgsServerService* );
};

Service

class QgsServerService {

    /** Anything informational */

    /* ... */

    /** Return allowed methods 

        Return flags indicating if method like GET or POST are allowed
    */
    bool allowMethod( QGServerMethodType ); 

    /** Execute a request
        @param request a QgsServerRequest instance
        @param response: a QgsResponse instance
        @return a result that could eventually be mapped to an HTTP response code 
     */ 
    int executeRequest( const QgsServerRequest& request, QGsServerResponse& response );
};

Requests

The request object encapsulate the input request

class QgsServerRequest {

    /** request url */
    const QString& url();

    /** url parameters */
    const QMap<QString, QString>& params();

    /** request header */
    const QMap<QString, QString>&  header();

    /** post data */
    const QByteArray* data();
};

Response

A wrapper around I/O far handling the response

class QgsServerResponse {

    /** Set header entry 
     */
    void setHeader( const QString& key, const QString value );

    /** Write a chunk of data 
    */
    qint64 write(const char * data, qint64 maxSize);

    qint64 write(const char * data);

    /** Return the underlying QIODevice 
     */
    QIODevice& ioDevice();

Example(s)

(optional)

Affected Files

(required if applicable)

Performance Implications

(required if known at design time)

Further Considerations/Improvements

(optional)

Backwards Compatibility

(required)

Issue Tracking ID(s)

(optional)

Votes

(required)

wonder-sk commented 7 years ago

Nice!

I would suggest to have a proper qgis_server library next to qgis_core and others, with fcgi completely abstracted away and having HTTP request/reply classes to interact with. Having things in a library would:

I know there is already QGIS server library, but it is based on the current messy server internals and it should be completely redesigned. In my opinion, the library should contain:

The server binary (for use with fastcgi) should only contain implementations of HTTP request / reply abstract classes to deal with fastcgi interface and the loop to process those.

Python bindings could also provide implementation of HTTP request / reply abstract classes to support WSGI - many people will probably find it as an useful alternative to fastcgi.

rldhont commented 7 years ago

@dmarteau can't participate to this discussion how to open this issue ?

dmarteau commented 7 years ago

An HTTP request is mainly a header and a body, this fit almost any kind of protocol. I would suggest to not be too much rooted in the HTTP protocol as this would be delegated to the caller (fcgi, wsgi, AMQP, 0MQ...). IMHO, we should keep the basic functionality as the simplest one, without dealing with process loop.

rldhont commented 7 years ago

The QEP has been updated with 'Proposed solutions' @elpaso: what do you think ?

elpaso commented 7 years ago

Yes, that's more or less what I also had in mind.

It's still not completely clear to me how the following features that are available in the current implementation will be implemented, here are some random thoughts:

There are some minor details that I guess could be considered at a later stage (I just note them to not forget):

rldhont commented 7 years ago

Update QgsServerService and add QgsServerResponse class

sbrunner commented 7 years ago

@rldhont How do you imagine the access control should work with that? Is it the responsibility of the plug-ins to use the access control manager to be sure that the actions are allowed?

rldhont commented 7 years ago

Hi @sbrunner, There is for me 2 possibility that depends on the way QgsProject will be refactored:

sbrunner commented 7 years ago

I don't know how he can work by modifying the loaded project (I especially think about the WFS transactional), but if we fond a way booth looks good to me :-)

mhugo commented 7 years ago

Hi, very interesting topic here.

I fully agree that we need to decouple the protocol from the "core" of the request handling. @elpaso you mentioned you think we would still need to have access to a header and a body. Could you elaborate ?

We (Oslandia) are very interested in having a good and clean server for QGIS. I don't know yet much about this part of the code, so I will ask very naive questions :

About dependencies to qgis.gui components, @elpaso you mentioned QgsProject depends on qgis.gui, why is that ? I think we also have providers that depends on qgis.gui, which would need to be fixed in order to have a server not dependent on gui, right ?

sbrunner commented 7 years ago

what do you think of having a pure Python server ?

It will be a huge work, why do you think we need it?

I think we also have providers that depends on qgis.gui, which would need to be fixed in order to have a server not dependent on gui, right ?

+1 :-)

mhugo commented 7 years ago

It will be a huge work, why do you think we need it?

I am trying to estimate the amount of work needed. So : what makes you think it would be a huge work ? :) What is so specific in the server code ? Suppose things that are listed here are fixed, mainly, no dependency on qgis.gui anymore, no duplicated qgs project parsing code, etc. I guess most of the server code could be calls to qgis.core, no ? Or are there something special this code does ?

mhugo commented 7 years ago

And the other thing is I guess performances are needed during the rendering of tiles (for WMS) or for feature fetching (for WFS), but not really for request parameter handling ?

dmarteau commented 7 years ago

Hi @mhugo

I see a 'service' a as functional entities that deliver content based on given protocol or specification (you can think WMS, WFS... beeing implemented as a service): it means that the server 'route' the requests to a registered handlers that is dedicated to it. This is mostly how the current Qgis server actually works.

This is (and should be) different from plugins: plugins act transversally in the workflow of the request handling, they could actually manage logging to specific device, access control....

Even if you can create new request handlers using plugins (which is in fact what is done in the current implementation), plugins are acting as pipelined set of filters acting independently of the functional target of request - plugins and services address different things, they don't have the same requirements in term of api, I/O or security, whatever....

Concerning a pure python server, yes... this is appealing, but too much work in the actual state of the qgis api and project managment. Apart from that, I do not see performances beeing a real issue because most of the heavy work is already done in c++, no ?

rldhont commented 7 years ago

@mhugo The work to do is to translate WMS parameters to QGIS core components and actions and it's not as simple as it sounds. But before redo QGIS Server, it will be great to:

mhugo commented 7 years ago

@rldhont @dmarteau thanks for your answers and for helping me clarify between services and plugins :)

I am not saying it should be rewritten in Python (well ... not yet :)) I am just trying to have a first overview of the code before going into more depth and try to propose things.

elpaso commented 7 years ago

@mhugo for SERVICE in this context I mean WMS WFS* etc. The coupling with the gui is (as far as I remember) mainly for the rendering part: the layer drawing order is determined by the order of the layers in the registry, but there might be other parts of the code where GUI is coupled.

ocourtin commented 7 years ago

Hi alls,

Something i want to share with you, related to QGIS Server perimeter.

As i already wrote, IMO the real -and meaningful- value of QGIS Server is the WMS service (who provide the really same symbology than in QGIS desktop, which is a rich one), and the related PDF GetPrint.

But if you want to go further, and provide WFS, SOS, WCS and so on, you will -need- a spatial database to store and manipulate (all) your datas.

As theses services are no more than a Web API in front of a spatial databases. And so there's no good and efficient way to do without.

So IMO as options, for QGIS Server in 3.0:

A) WMS 1.3, SLD, FE 1.1 and GetPrint only

B) Same than A but provide also packaging with other existing Web services and ability for them to read qgs configuration file

C) Want to go beyond WMS, and provide at the very least WFS and/or SOS and/or WCS and so imply to also think before on an embeded/tied spatial database storage.

Any reactions welcome :)

dmarteau commented 7 years ago

Hi @ocourtin

As described in the QEP. The new architecture is organized as modular services around a request management, so, potentially, any kind of services can be activated/remo ved from the core set of services that qgis server can provide. It will be up to admin/users to select the set of functionalities they want.

ocourtin commented 7 years ago

@dmarteau Thanks for your quick reply ! But my point is not about the modularity, but about the ability. :)

As to fully provide services as WFS, SOS, WCS, you need a spatial database. Or something who behave just like the same...

elpaso commented 7 years ago

@ocourtin the vector layer abstraction in QGIS core does exactly this. Every data provider has its own capabilities, of course, for example WFS-T will not be enabled if the provider does not support editing.

ocourtin commented 7 years ago

@elpaso , thanks too for your reply.

Vector layer abstraction could surely be use to do a part of the job.

But i guess, you will face sooner or later to others limitations, as, for instance, from WFS:

giohappy commented 7 years ago

Whatever the refactoring will be, please DO NOT REMOVE the current web services. Many rely on them for their services and business (even WFS @ocourtin). Every improvement is welcome but please let's mantain the current functionalities.

ocourtin commented 7 years ago

@giohappy thanks for you post, and to fully understand your point:

Do you want to keep all the existing OGC services maintains, from an end user point of view (and that could be achieved by B option) ?

Or do you want in more that to be done without any other external apps, even if packaged (so more related to C option) ?

ebelo commented 7 years ago

Dear all, it's good to refactor QGIS Server. But all the existing OGC capabilities from QGIS Server should be kept available in every future releases, maintained and improved. We speak about WMS, WFS, WCS but also the Print, etc. [http://docs.qgis.org/testing/en/docs/user_manual/working_with_ogc/ogc_server_support.html]

QGIS Server is used in many OGC service based Spatial Data Infrastructure. All these services are needed to provide a good OGC integration and cover the different use cases our users expect, with a unique point of data configuration and map/print layout definition, beeing provided by QGIS Desktop.

Camptocamp will also provide developer time to help in this effort, but there is no way to drop any OGC feature from qgis server, even temporarily. I think, there are enough companies and developers interested.

We should start to organize a code sprint (with some financial support from the qgis organization?) to debate, define and work in a more synchronous way on the new architecture.

giohappy commented 7 years ago

@ocourtin I'm not sure what you mean with "provide packaging" in B. How would you provide optional, additional, services? Do you mean through plugins (service providers) to be loaded at startup (which means at each request in case of a simple CGI) depending on some configuration mechanism? Do you mean different QGIS distributions? Please provide some more details.

rldhont commented 7 years ago

With this QEP, our goal is to keep the functionalities, simplify the code and add modularity. So to have something more maintainable, robust and expendable.

To do so, we will start with the implementation of the service register.

ocourtin commented 7 years ago

@giohappy The idea in B option was that services as WFS, SOS, WCS could be provided throught an another already existing server (as GeoServer or MapServer or whatever). But both well packaged with WMS QGIS Server. And .qgs could be used as the only configure file. So should be somehow transparent from the end user point of view.

I fully understand now from all the reactions, that it just can't be imagined. So just forget it :)

@ebelo And to be understood, my only concern, was to see if there's a way to focus first on (all) the low level stuff, allowing thereafter to have something really reliable.

As QGIS server code need a lot of love, OGC implementation are known to be painful, and can't really imagine that 3.0 could wait 2018.

I agree too, that a CodeSprint could help. Oslandia could hosts in Lyon if you/all find the place suitable.

@rldhont Yeap, understood, thanks too for your clarification !

mhugo commented 7 years ago

@rldhont If you don't mind, I've updated the ticket to add https://github.com/qgis/qgis3.0_api/issues/66 to the list of items

rldhont commented 7 years ago

@mhugo thanks

vmora commented 7 years ago

From the discussion an the linked tickets, here is what I see for the server code. It's c++/fcgi, but short enought to be re-implemented as part of a wsgi server, an apache or an module).

int main()
{
    const QgsProject project(projectFile); // not a singleton
    QgsServices services(project); // not a singleton either,
                                   // stores the const QgsProject and
                                   // reads the services part of the project,
                                   // which can include python services if support is compiled in.

    for (;;)
    {
      const QgsServerRequest request = convert_request_from_fcgi();
      const QgsServerResponse response = services.execute(request);
      convert_response_to_fcgi(response);
    }
    return SUCCESS;
}

With the constrains:

What I see as the ideal case is a QgsProject that is no singleteon anymore and is copyable: the request obtains a copy of the initial project to work with (https://github.com/qgis/qgis3.0_api/issues/8). If complete singleton removal is not possible (https://github.com/qgis/qgis3.0_api/issues/42) we would need way to save/restore the project (to/from memory).

For the plugin services mechanism, I like decoration/overload better than hooks:

class MyFilteredAndWatermarkedService(QgsWmsService):
    """a potpourri of plugins I know about"""

    def __init__(self, project):
        QgsWmsService.__init__(self, project)

    def getMap(self, request):
        request = self.__convert_proprietary_filter_params_and_access_control_to_SLD_FE(request)
        response = QgsWmsService.getMap(self, request)
        response = self__add_watermark(response)
        return response
    ...

Questions:

I don't see the need for singleton/registry anywhere here. Is there a reason to continue to use global variables ?

wonder-sk commented 7 years ago

Hi @vmora just a quick note - I think QGIS server should not be fixed to just one project (it certainly can work with multiple projects right now). In my opinion the "current" QgsProject instance (a singleton) should be only used within QGIS desktop, any code reachable from server should not refer to the singleton (ideally should stay nullptr).

elpaso commented 7 years ago

@vmora, some random notes,

QGIS Server must be able to serve different projects, the project should continue to be per-request (caching is of course needed).

Stream operators are ok.

The ability to clear/add/edit headers from within a plugin is mandatory. Use case: all authentication plugins, but there are certainly more use case where manipulation of the headers is desired.

About the fcgi , I think that there is only one blocker here: streaming services, like WFS must be able to stream content as they do now (think about a huge and slow WFS request): how do you get rid of fcgi write operations in the WFS service loop? I don't see any other option than the current implementation (or something more elegant but that works the same way).

vmora commented 7 years ago

@wonder-sk, thanks for the reminder about project beeing an argument of the request, @elpaso loading/caching const projects in services would then be simple enough (as far as "simple" goes for caching). It could even be a list of available projects (all loaded at startup) depending on the memory footprint of a QgsProject, rather than caching. And with only one projet, the argument could be optional (no proprietary param the client must care about)

@wonder-sk the singleton could be implemented just in gui, or even app... or not at all, so the instance would not even be callable from core, but that goes in https://github.com/qgis/qgis3.0_api/issues/8. I think we agree that singleton are a pain and that the core should not use them.

The ability to clear/add/edit headers from within a plugin is mandatory.

I aggree with the mendatory functionnality, but does the header need to be separate "by design" (i.e. api constrain). It seems easy enough to me to have a child/decorator class that provide those kind of functionaries, in which case the QgsServerResponse is just a typedef of ostream.

how do you get rid of fcgi write operations in the WFS service loop

This could be done with streams and the proposed response=execute(request), but with multi-threading only... so no, I agree with @rldhont initial execute(request, response):

for(;;)
{
  const QgsServerRequest request = convert_request_from_fcgi();
  QgsFcgiServerResponse response; // stream to stdout
  services.execute(request, response);
}

but still don't see the need for a return value.

ps: of course 

class QgsFcgiServerResponse: public QgsServerResponse
...
luzpaz commented 7 years ago

Please update the checkboxes of the completed 'Dependencies' for this project eg.

mhugo commented 7 years ago

@luzpaz Thanks, text updated

haubourg commented 7 years ago

@rldhont - trying to get todolists up to date here:

elpaso commented 7 years ago

@dmarteau @sbrunner @rldhont

I really like how the server is evolving, but after the refactoring we might wanto to do some further cleaning of the Python API.

Here are some thoughts about what I would like to address (in a future PR), concerning Python usage of the API:

Returned values from python QgsServer::handleRequest( urlstr ....)

I wonder if now that we have QgsServerResponse we can get rid of the QPair of QByteArray return value from the "urlstr" version of handleRequest( urlstr ....) and use from python a more intuitive pattern like:

response = server.handleRequest(url, method, data, headers) # <-- needs the headers, see below

Do we still need two versions of handleRequest?

the way the FCGI request is built (implicitly getting all it needs from the env) and the way the urlstr version of it gets all from its arguments is too different,

Maybe we can completely eliminate the "urlstr" version of QgsServer::handleRequest( urlstr ...) and use the existing oveload that passes the QgsServerRequest instance to handleRequest(request, reponse):

request = QgsServerRequest(url, method, data, headers)
response = server.handleRequest(request) 
# Or passing response as argument if we cannot really avoid that:
response = QgsServerResponse()
server.handleRequest(request, response)

Proper handling of the request headers

This is not really important, but I'd like to standardize the way request headers are passed to the QgsServerRequest and down the line, to the plugins. Headers are now available in the environment (CGI-style) but there is no way to pass them to the buffered request, that's why I'd like to see them added as an optional argument to

request = QgsServerRequest(url, method, data, headers)

Empowered QgsServerInterface

The server interface is meant to "protect" the server internals from plugin abuse, but we are in a server context, a system administrator is supposed to know what he is doing when installing and enabling a plugin. That's why we might want to give more power to the plugins. What I'm thinking of is to expose (from QgsRequestHandler) to the server interface.

    QgsServerRequest   &mRequest;
    QgsServerResponse  &mResponse;

Most proxy methods would be removed from the server interface because they would not be necessary anymore.

Looking forward to read your comments!

dmarteau commented 7 years ago

@elpaso I totally agree with you.

Some remarks:

QgsServerInterface

First of all, note that QgsServerInterface as a larger perimeter since it passed to the core services and not only to the plugins, providing access to some caching features. Thus, this object as a more central role than before.

The object that hold input/output is QgsRequestHandler. actually it is mainly a interface to the QgsServerRequest/QgsServerResponse and delegates most of the functionalities to these objects. Aside from the fact that it does some setup on request, it's main purpose is to keep some compatibility level for plugins.

As you say, this abstraction level could be removed in favor of a direct exposition of QgsServerRequest/QgsServerResponse that already hold the necessary I/O abstraction.

handleRequest

QgsServer::handleRequest( urlstr ...) method is only a convient method that call under the hood server.handleRequest(request, response) from an appropriate implementation of QgsServerRequest and QgsServerResponse. It's main purpose was to provide some simple entrypoint from python by mimic fcgi environment variable settings. But this interface is not suitable nor efficent for many other cases.

I see the main entry point to the server provided by the method handleRequest(request, response). Each embedders at to provide the correct implementation of QgsServerRequest/QgsServerResponse wich are only adapters to the I/O model used.

From this, we could have many third party wrappers (in python or c++) for the most common frameworks (wsgi, django, tornado, ...).

rldhont commented 6 years ago

It seems that the refactoring is complete!

haubourg commented 6 years ago

Woooohoooo!!! this is great news! ping @anitagraser