WMS: Better logic to pick the legend URL

QGIS had two problems: 1) It was using the specified legend URL only if its mime type was matching the layer's mime type. There is no reason for that. 2) When QGIS was using the default layer (empty string), it was not even trying to find out in what style to pick the legend URL. (cherry-picked from 69bed21)
qgis · Jul 20, 2016 · ca96702 · ca96702
1 parent b81a07a
commit ca96702
Show file tree

Hide file tree

Showing 4 changed files with 126 additions and 17 deletions.
diff --git a/src/providers/wms/qgswmscapabilities.cpp b/src/providers/wms/qgswmscapabilities.cpp
@@ -907,7 +907,7 @@ void QgsWmsCapabilities::parseLayer( QDomElement const & e, QgsWmsLayerProperty&
 
         for ( int i = 0; i < layerProperty.style.size(); ++i )
         {
-          if ( layerProperty.style[i].name == styleProperty.name )
+          if ( layerProperty.style.at( i ).name == styleProperty.name )
           {
             // override inherited parent's style if it has the same name
             // according to the WMS spec, it should not happen, but Mapserver

diff --git a/src/providers/wms/qgswmsprovider.cpp b/src/providers/wms/qgswmsprovider.cpp
@@ -215,6 +215,36 @@ QString QgsWmsProvider::getTileUrl() const
   }
 }
 
+static bool isValidLegend( const QgsWmsLegendUrlProperty &l )
+{
+  return l.format.startsWith( "image/" );
+}
+
+/**
+ * Picks a usable legend URL for a given style.
+ */
+static QString pickLegend( const QgsWmsStyleProperty &s )
+{
+  QString url;
+  for ( int k = 0; k < s.legendUrl.size() && url.isEmpty(); k++ )
+  {
+    const QgsWmsLegendUrlProperty &l = s.legendUrl[k];
+    if ( isValidLegend( l ) )
+    {
+      url = l.onlineResource.xlinkHref;
+    }
+  }
+  return url;
+}
+
+static const QgsWmsStyleProperty *searchStyle( const QVector<QgsWmsStyleProperty>& styles, const QString& name )
+{
+  Q_FOREACH ( const QgsWmsStyleProperty &s, styles )
+    if ( s.name == name )
+      return &s;
+  return nullptr;
+}
+
 QString QgsWmsProvider::getLegendGraphicUrl() const
 {
   QString url;
@@ -223,25 +253,31 @@ QString QgsWmsProvider::getLegendGraphicUrl() const
   {
     const QgsWmsLayerProperty &l = mCaps.mLayersSupported[i];
 
-    if ( l.name != mSettings.mActiveSubLayers[0] )
-      continue;
-
-    for ( int j = 0; j < l.style.size() && url.isEmpty(); j++ )
+    if ( l.name == mSettings.mActiveSubLayers[0] )
     {
-      const QgsWmsStyleProperty &s = l.style[j];
-
-      if ( s.name != mSettings.mActiveSubStyles[0] )
-        continue;
-
-      for ( int k = 0; k < s.legendUrl.size() && url.isEmpty(); k++ )
+      if ( !mSettings.mActiveSubStyles[0].isEmpty() && mSettings.mActiveSubStyles[0] != "default" )
       {
-        const QgsWmsLegendUrlProperty &l = s.legendUrl[k];
-
-        if ( l.format != mSettings.mImageMimeType )
-          continue;
-
-        url = l.onlineResource.xlinkHref;
+        const QgsWmsStyleProperty *s = searchStyle( l.style, mSettings.mActiveSubStyles[0] );
+        if ( s )
+          url = pickLegend( *s );
       }
+      else
+      {
+        // QGIS wants the default style, but GetCapabilities doesn't give us a
+        // way to know what is the default style. So we look for the onlineResource
+        // only if there is a single style available or if there is a style called "default".
+        if ( l.style.size() == 1 )
+        {
+          url = pickLegend( l.style[0] );
+        }
+        else
+        {
+          const QgsWmsStyleProperty *s = searchStyle( l.style, "default" );
+          if ( s )
+            url = pickLegend( *s );
+        }
+      }
+      break;
     }
   }
 

diff --git a/tests/src/providers/CMakeLists.txt b/tests/src/providers/CMakeLists.txt
@@ -89,6 +89,10 @@ ADD_QGIS_TEST(wmscapabilititestest
               testqgswmscapabilities.cpp)
 TARGET_LINK_LIBRARIES(qgis_wmscapabilititestest wmsprovider_a)
 
+ADD_QGIS_TEST(wmsprovidertest
+              testqgswmsprovider.cpp)
+TARGET_LINK_LIBRARIES(qgis_wmsprovidertest wmsprovider_a)
+
 #############################################################
 # WCS public servers test:
 # No need to test on all platforms

diff --git a/tests/src/providers/testqgswmsprovider.cpp b/tests/src/providers/testqgswmsprovider.cpp
@@ -0,0 +1,69 @@
+#include <QFile>
+#include <QObject>
+#include <QtTest/QtTest>
+#include <qgswmsprovider.h>
+#include <qgsapplication.h>
+
+/** \ingroup UnitTests
+ * This is a unit test for the WMS provider.
+ */
+class TestQgsWmsProvider: public QObject
+{
+    Q_OBJECT
+  private slots:
+
+    void initTestCase()
+    {
+      // init QGIS's paths - true means that all path will be inited from prefix
+      QgsApplication::init();
+      QgsApplication::initQgis();
+
+      QFile file( QString( TEST_DATA_DIR ) + "/provider/GetCapabilities.xml" );
+      QVERIFY( file.open( QIODevice::ReadOnly | QIODevice::Text ) );
+      const QByteArray content = file.readAll();
+      QVERIFY( content.size() > 0 );
+      const QgsWmsParserSettings config;
+
+      mCapabilities = new QgsWmsCapabilities();
+      QVERIFY( mCapabilities->parseResponse( content, config ) );
+    }
+
+    //runs after all tests
+    void cleanupTestCase()
+    {
+      delete mCapabilities;
+      QgsApplication::exitQgis();
+    }
+
+    void legendGraphicsWithStyle()
+    {
+      QgsWmsProvider provider( "http://localhost:8380/mapserv?xxx&layers=agri_zones&styles=fb_style&format=image/jpg", mCapabilities );
+      QCOMPARE( provider.getLegendGraphicUrl(), QString( "http://www.example.com/fb.png?" ) );
+    }
+
+    void legendGraphicsWithSecondStyle()
+    {
+      QgsWmsProvider provider( "http://localhost:8380/mapserv?xxx&layers=agri_zones&styles=yt_style&format=image/jpg", mCapabilities );
+      QCOMPARE( provider.getLegendGraphicUrl(), QString( "http://www.example.com/yt.png?" ) );
+    }
+
+    void legendGraphicsWithoutStyleWithDefault()
+    {
+      QgsWmsProvider provider( "http://localhost:8380/mapserv?xxx&layers=buildings&styles=&format=image/jpg", mCapabilities );
+      //only one style, can guess default => use it
+      QCOMPARE( provider.getLegendGraphicUrl(), QString( "http://www.example.com/buildings.png?" ) );
+    }
+
+    void legendGraphicsWithoutStyleWithoutDefault()
+    {
+      QgsWmsProvider provider( "http://localhost:8380/mapserv?xxx&layers=agri_zones&styles=&format=image/jpg", mCapabilities );
+      //two style, cannot guess default => use the WMS GetLegendGraphics
+      QCOMPARE( provider.getLegendGraphicUrl(), QString( "http://localhost:8380/mapserv?" ) );
+    }
+
+  private:
+    QgsWmsCapabilities* mCapabilities;
+};
+
+QTEST_MAIN( TestQgsWmsProvider )
+#include "testqgswmsprovider.moc"