Opened 8 years ago

Last modified 6 years ago

#287 new enhancement

support nested layers in WMSCapabilitiesStore

Reported by: bartvde Owned by: ahocevar
Priority: major Milestone: 1.2
Component: GeoExt.data.WMSCapabilitiesStore Version: trunk
Keywords: Cc:
State: Needs more work

Description

This came up in ticket:272 we would need support for nested layers in the WMSCapabilitiesStore so that the WMSCapabilitiesLoader can use a store.

Attachments (2)

ticket287.patch (4.6 KB) - added by bartvde 7 years ago.
ticket287_ie6tests.patch.gz (35.3 KB) - added by bartvde 7 years ago.
patch to fix up tests in IE6

Download all attachments as: .zip

Change History (16)

comment:1 Changed 8 years ago by bartvde

{{ Note, however, that at some point before the next release, we should change the WMSCapabilitiesLoader under to hood to use a WMSCapabilitiesStore, which should have a "path" property on the LayerRecords. }}}

comment:2 Changed 7 years ago by bartvde

  • Owner set to bartvde
  • Status changed from new to assigned

Changed 7 years ago by bartvde

comment:3 Changed 7 years ago by bartvde

Tests pass in FF 3.6 and IE6. Please review.

Btw, the second patch is to get the tests to work properly in IE6, they were already broken in IE6 before this change. Also some speedup for the IE test by not using string concatenation.

comment:4 Changed 7 years ago by bartvde

  • State changed from None to Review

Second patch is gzipped since it was too large.

Changed 7 years ago by bartvde

patch to fix up tests in IE6

comment:5 Changed 7 years ago by pgiraud

I may be completely wrong but, do we really need to get the complete path? Perhaps, only a reference to the parent layer is enough, kind of an id of the record. However I do not understand were the 'path' attribute is actually used.

comment:6 Changed 7 years ago by bartvde

Hi Pierre, it's because it might actually be a path that contains non-physical layers, such as WMS layers with only a title and no name.

comment:7 Changed 7 years ago by bartvde

ticket287_ie6tests.patch.gz has been committed see ticket:336

comment:8 Changed 7 years ago by ahocevar

To potential reviewers: please also review #314

comment:9 Changed 7 years ago by ahocevar

  • Owner changed from bartvde to ahocevar
  • Status changed from assigned to new

reviewing now

comment:10 Changed 7 years ago by ahocevar

  • State changed from Review to Needs more work

Using the layer's title in the path makes this approach very fragile, because it is easy to change layer titles any time. If you would fall back from name to tile, you would get a solid path: names for physical layers, and titles for containers.

Also, with a huge list of layers (I hear there are WMS installations with thousands of layers), the approach of walking through the nested layers every time seems a bit expensive. It would probably make sense to add another object to OpenLayers.Format.WMSCapabilities: an object keyed by layer name, with the name/title of the parent as value. Because when the layers are processed in OpenLayers, you have a parent anyway.

Should we bump this to 1.1?

comment:11 Changed 7 years ago by ahocevar

Looking at #314, I think Pierre's suggestion to just store the parent makes sense, because what you need there is just the parent. You could store the layer's name in the parent field, and use the title for non-physical layers.

comment:12 Changed 7 years ago by bartvde

  • Milestone changed from 1.0 to 1.1

Okay, you mean a hybrid approach Andreas? If the parent is a physical layer, just store the name. Otherwise parent can still be an array of titles (or an xpath type of string)? Since a layer can be way down in the hierarchy:

<Layer><Title>Level 1</Title>
    <Layer><Title>Level 2</Title>
        <Layer><Title>Level 3</Title>
            <Layer><Name>physical_layer</Name><Title>first physical layer</Title></Layer>
       </Layer>
    </Layer>
</Layer>

Let's bump this to 1.1.

comment:13 Changed 7 years ago by ahocevar

Good point Bart. Well, in that case I think it will be better to store the path. But use the layer's name rather than it's title if we are dealing with a physical layer.

comment:14 Changed 6 years ago by ahocevar

  • Milestone changed from 1.1 to 1.2

Batch move of tickets to finish the 1.1 milestone.

Note: See TracTickets for help on using tickets.