MS RFC: Layer Plug-in Architecture

Frank Warmerdam fwarmerdam at GMAIL.COM
Mon Aug 22 15:47:16 EDT 2005


On 8/22/05, Jani Averbach <javerbach at extendthereach.com> wrote:
> After that all layer operations will happen through this
> vtable which is cached in the ``layerObj`` struct.

Jani, 


I am assuming there will still be quite a few generic layer operations
not going through the vtable, as now, right?  The vtable is only for
backend-format specific operations. 

>         2.1.1. InitItemInfo
>                ::
> 
>                  int (*fpLayerInitItemInfo)(layerObj *layer);

I don't think it is a convention to use hungarian prefix in generic
MapServer code, is it?  I would suggest just calling the functions
in the vtable something like "LayerIinitItemInfo" instead of prefixing
with fp.  If we were to use a prefix, I would encourage pfn (pointer
to function) to match GDAL/OGR and other hungarian usage. 

I would also like to suggest that the functions like msLayerInitItemInfo()
that call into the vtable ought to do some reasonable default action (like
nothing) if the function pointers are NULL.  So layers only need to implement
(ie. override) those functions that make sense. 

>    2.2.  New functions and/or fields for vtable
> 
>          2.2.1. CloseConnection
> 
>                 This function is used to actually close the
>                 connection, in case that layer implements some kind of
>                 connection pooling by its own.  If layer doesn't use
>                 any connection pooling, this function should be
>                 implemented as no-op. Caller should first call layer's
>                 "close" function, and finally at the very end
> ``CloseConnection``.
> 
>                 The signature of function is
>                 ::
> 
>                    int (*fpLayerCloseConnection)(layerObj *layer);
> 
> 
>                 And the main place where this function will be called,
>                 is ``mapfile.c: 4822``
>                 ::
> 
>                    void msCloseConnections(mapObj *map)
> 
>                 This function is needed because e.g. ``POSTGIS`` is
>                 implementing this usage pattern at the moment
>                 ``maplayer.c:599``
>                 ::
> 
>                     void msLayerClose(layerObj *layer)
>                     ...
>                    /*
>                     * Due to connection sharing, we need to close the
> results
>                     * and free the cursor, but not close the connection.
>                     */
>                     msPOSTGISLayerResultClose(layer);

I am confused about what the close connection method is
for.  Isn't the existing msCloseConnections() logic just left over
from the "old" connection pooling mechanism?  I think we ought
to take the old connection pool stuff out as part of this overhaul.  

>          2.2.2. GetConnectionTypeName
> 
>                 This function is used to get MAP-file name for this
>                 layer/connectiontype.  It is used when map file is
>                 written back (``mapfile.c: 2856``), actual strings will
>                 be: ``OGR``, ``POSTGIS`` and so on.

Will we, or will we not still have "connectiontype" entry in
the layerObj?  I think we would, and that the connection type
name can be derived from that the same way it is now.  

>          2.2.3. SetTimeFilter
> 
>                 This function is used to create a time filter for
>                 layer. At the moment we have three special cases
>                 (``maplayer.c: 1635``):
> 
>                 1. ``POSTGIS`` with it's own function
>                 2. Layers with backticks delimited expressions
>                 3. Layers without backticks
> 
>                 The idea is provide a generic helper function,
>                 ::
> 
>                    int makeTimeFilter(layerObj *lp,
>                                       const char *timestring,
>                                       const char *timefield,
>                                       const bool bBackTicks)
> 
>                 And the actual layer's ``SetTimeFilter`` could use the
>                 above, or implement something totally different as
>                 ``POSTGIS`` is doing at the moment.
> 
>                 The signature for layer's vtable function is
>                 ::
> 
>                    int (*fpLayerSetTimeFilter)(layerObj *lp,
>                                                const char *timestring,
>                                                const char *timefield);

Aren't there other filter translation issues a well?  In an 
ideal world, layers ought to take care of transforming filters
into their native format but it seems like this is a big piece
to chew off.  I'm not clear why time is a special case of 
filter translation. 
 
>    2.3. Things to fix or extra functions to add to the vtable
> 
>         I would like to see following items fixed, but I don't know
>         which way to go.
> 
>         2.3.1. FLTApplyFilterToLayer (mapogcfilter.c: 1084)
> 
>                Could we create two helper functions, one for "SQL"
>                case and the another for "non-SQL" case, and set all
>                layers, except ``POSTGIS`` and ``ORACLESPATIAL`` to call
>                directly this "non-SQL" version of this helper function
>                (else-branch of if).  ``ORACLESPATIAL`` and ``POSTGIS`` could
>                use "SQL" version of the helper function (actual
>                if-branch) if the conditions for this are met. This way
>                we could remove all layer specific logic from this
>                function, and make this function's logic actually generic.

Ahh, this addresses my earlier question I see.  I would note that 
OGR now uses SQL filter translation too.  
 
>         2.3.2. layerObj->items allocation
> 
>                There is layer (SDE) specific code in ``msLayerWhichItems``
>                (``maplayer.c: 880``) when initial items allocation is
>                done.
>                ::
> 
>                   /* TODO: it would be nice to move this into the SDE
>                    * code itself, feels wrong here...
>                    */
>                   if(layer->connectiontype == MS_SDE) {
>                   ...
> 
>                One solution would be to provide a layer specific
>                function
>                ::
> 
>                   int (*fpCreateItems)(layerObj *)
> 
>                which does the allocation and set numitems to correct
>                value.

I don't understand what CreateItems does that is distinct from 
the InitItemInfo function.  Could you clarify?  

>         2.3.3. msCheckConnection (``mapfile.c: 4779``)
> 
>                This api looks to be deprecated. It is called only from
>                ``msMYGISLayerOpen``.  Is it hard to fix GIS to use
>                mappool?  If this isn't feasible thing to do, we could
>                remove all other cases and just leave GIS inside this
>                function.

Is the MyGIS interface still in significant use?  My understanding 
is that it is no longer supported by Attila.  Perhaps it is time to 
remove MyGIS support since it seems hard to maintain. 

> * To find best place to initialize layer's vtable. One possible place
>   could be ``msLayerOpen``. The problem with that is that if we need
>   before opening the layer something layer-specific information, it
>   won't be available.

I had assumed that the vtable would be initialized at the point
where the layerObj is established (when parsing the mapfile for
instance).  However deferring it till the first LayerOpen might work ok.
Perhaps all the msLayer* functions that can call into the vtable can
call a function to initialize the vtable if it isn't already setup when 
they are called.  
 
> * I also like to note that this proposal won't remove all layer
>   specific code from MapServer e.g. ``WMF``, ``WMS`` and ``GRATICULE``
>   are used as special cases from place to place.

Are there any special issues with the raster query layer support
which is handled via the layer API? 
 
I would also like to suggest that as part of this process a "howto" 
document for implementing new layers be prepared.  I would be prepared
to take a first pass at that.  

Finally, you don't seem to have mentioned anything about custom 
layers that are dynamically loaded in this draft.  Are you planning to 
add that? 

Best regards,
-- 
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, warmerdam at pobox.com
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent



More information about the mapserver-dev mailing list