Problem accessing layers by name with mapserv CGI and WMS

[dmorissette]

With v4.4.0-beta1, in a mapfile that contains "wms_name" metadata values for the
layers, the wms_name takes precedence over the real layer.name. This breaks
existing applications (or WMS services) that used to use the layer.name
parameter to layers on or off.

<long_story>
This is a side-effect of a change that was made to accomodate support for
loading map contexts via the mapserv CGI in bug 946. When a context is loaded,
MapServer's msLoadMapContext() generates new unique names for all the layers in
the context since the names in the context file come from the remote WMS servers
and are not guaranteed to be unique. However the new unique names would have
made it hard for mapserv CGI application developers to predict the layer names
that will be created and to code them in their interface, and to solve the
issue, this generation of unique names was replaced by the use of the wms_name
metadata (when present) to control layer's status. The problem is that this
change has been made global to all CGI operations and not specific to the case
where a context is loaded, causing the side-effect described above where you
cannot control the status of layers using the layer.name any more.
</long_story>

First step is to restrict this change of behavior to cases where a context is
being loaded. Assefa will do that now (in time for beta2).

Then we need to find a long term solution. One option could be to modify
msLoadmapContext() to not generate unique layer names by default and use the
names found in the context document directly even if duplicates are present.
Then an option (e.g. context_unique_layer_names) could be used to explicitly ask
for unique layer names via the CGI and via MapScript. 

That would be the option that makes most sense but unfortunately that would mean
a change of behavior for MapScript apps since they are used to and rely on the
unique generated names by default.

[assefa]

The behaviour olny applies now when a context file is passed as argument.

[pspencer@…]

For me, the most significant impact would be on the legend template generation,
as long as that works, I'm good with it :>

[dmorissette]

Paul: My thinking is that $map->loadMapContext() would require an (optional)
extra argument in 4.4 to force the generation of unique names (by default the
context names would be used):

int map.loadMapContext(string filename
                       [,boolean force_unique_layer_names=MS_FALSE]);

So you'd need some MapScript-version-specific code in your apps in order to keep
today's behavior (based on MapScript version). I hate breaking backwards
compatibility, but that seems like the most viable option.

Whadyathink?

[jlacroix]

Is it just me or using the wms_name instead of the layer name negate the fact
that we build a unique name for MapContext layers anyway?

I have not played with the context in CGI, but I think that in a CGI
environment, the user won't be able to load two context at the same time like it
is the case in MapScript. This is the main source of layer name conflict. So I
think that for CGI it is fairly safe to use the wms_name for the layer name.

Is there a reason why we can't have the unique name functionality by default in
MapScript and don't use it in CGI?

[dmorissette]

> Is it just me or using the wms_name instead of the layer name negate the fact
> that we build a unique name for MapContext layers anyway?

Yes, using wms_name in mapserv.c undoes what msLoadMapContext did and that's the
source of this bug.

> Is there a reason why we can't have the unique name functionality by default
> in MapScript and don't use it in CGI?

I think we need to be consistent between both interfaces otherwise we'll need to
document and explain the difference to users all the time. That's why I proposed
that we stop generating the unique names by default in both MapScript and the
CGI and instead offer an option to explicitly request automated generation of
unique names.

[jlacroix]

I agree for the consistency between the interface. But then, Paul, in Chameleon
is there really just the legend template generation that use getLayerByName() +
loadMapContext? My concern is that initially we made functionality because in a
MapScript application like Chameleon, we can load multiple context file and then
we end with multiple layers with the same name and it bugs the
move-up/move-down/delete layers functionality.

[dmorissette]

okay, it seems that we have a solution. Assefa confirmed that passing an extra
argument to older versions of $map->loadMapContext() won't generate any error,
so we can safely add the extra argument to the function without breaking
existing code too much. Calling $map->loadMapContext(filename, MS_TRUE) will
work consistently with PHP MapScript pre-4.4 and post-4.4. 

So here is what we should do:

1- Modify msLoadMapContext() to take an extra boolean argument to control
whether the layer names should be left intact or should be made unique:

int msLoadMapContext(mapObj *map, char *filename, int unique_layer_names)

2- Remove this new code in mapserv.c that was using wms_name. Just calling the
above with unique_layer_names=MS_FALSE in the mapserv CGI case will solve this
part of the problem.

3- Add the new optional unique_layer_names argument to map.loadMapContext() with
a default value to MS_FALSE.

[dmorissette]

I forgot #4: Same change needs to be applied to SWIG MapScript

Oh, and #5: A clear note should be included in HISTORY.TXT and in the Migration
guide in the Wiki to explain that map.loadMapContext() doesn't generate unique
names by default any more, and that an extra argument is required for the unique
names.

Assefa is going to take this from here.

[assefa]

working on this.

[assefa]

I did the changes according to comment #7 and #8.

I have created a bug 1025 for the swig version to be changed. Sean is aware of it.

[dmorissette]

*** Bug 521 has been marked as a duplicate of this bug. ***

[dmorissette]

Please ignore the "bug 521 is duplicate" comment above. Wrong bug.