Merge pull request #3077 from pvalsecc/legend_url

WMS: Better logic to pick the legend URL

src/providers/wms/CMakeLists.txt

@@ -41,6 +41,7 @@ INCLUDE_DIRECTORIES(SYSTEM
   ${QCA_INCLUDE_DIR}
 )
 
+ADD_LIBRARY(wmsprovider_a STATIC ${WMS_SRCS} ${WMS_MOC_SRCS})
 ADD_LIBRARY(wmsprovider MODULE ${WMS_SRCS} ${WMS_MOC_SRCS})
 
 TARGET_LINK_LIBRARIES(wmsprovider
@@ -50,6 +51,11 @@ TARGET_LINK_LIBRARIES(wmsprovider
   ${GDAL_LIBRARY}  # for OGR_G_CreateGeometryFromJson()
 )
 
+TARGET_LINK_LIBRARIES(wmsprovider_a
+  qgis_core
+  ${QT_QTSCRIPT_LIBRARY}
+)
+
 INSTALL (TARGETS wmsprovider
   RUNTIME DESTINATION ${QGIS_PLUGIN_DIR}
   LIBRARY DESTINATION ${QGIS_PLUGIN_DIR})

src/providers/wms/qgswmscapabilities.cpp

@@ -905,6 +905,17 @@ void QgsWmsCapabilities::parseLayer( QDomElement const & e, QgsWmsLayerProperty&
 
         parseStyle( e1, styleProperty );
 
+        for ( int i = 0; i < layerProperty.style.size(); ++i )
+        {
+          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
+            // does it all the time.
+            layerProperty.style.remove( i );
+            break;
+          }
+        }
         layerProperty.style.push_back( styleProperty );
       }
       else if ( tagName == "MinScaleDenominator" )

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;
     }
   }
 

tests/src/providers/CMakeLists.txt

@@ -9,6 +9,7 @@ INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}
   ${CMAKE_SOURCE_DIR}/src/core/auth
   ${CMAKE_SOURCE_DIR}/src/core/geometry
   ${CMAKE_SOURCE_DIR}/src/core/raster
+  ${CMAKE_SOURCE_DIR}/src/providers/wms
 )
 INCLUDE_DIRECTORIES(SYSTEM
   ${QT_INCLUDE_DIR}
@@ -84,6 +85,14 @@ SET_TARGET_PROPERTIES(qgis_wcsprovidertest PROPERTIES
 
 ADD_QGIS_TEST(gdalprovidertest testqgsgdalprovider.cpp)
 
+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

tests/src/providers/testqgswmscapabilities.cpp

@@ -0,0 +1,70 @@
+#include <QFile>
+#include <QObject>
+#include <QtTest/QtTest>
+#include <qgswmscapabilities.h>
+#include <qgsapplication.h>
+
+/** \ingroup UnitTests
+ * This is a unit test for the WMS capabilities parser.
+ */
+class TestQgsWmsCapabilities: 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();
+    }
+
+    //runs after all tests
+    void cleanupTestCase()
+    {
+      QgsApplication::exitQgis();
+    }
+
+
+    void read()
+    {
+      QgsWmsCapabilities capabilities;
+
+      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;
+
+      QVERIFY( capabilities.parseResponse( content, config ) );
+      QCOMPARE( capabilities.supportedLayers().size(), 5 );
+      QCOMPARE( capabilities.supportedLayers()[0].name, QString( "agri_zones" ) );
+      QCOMPARE( capabilities.supportedLayers()[1].name, QString( "buildings" ) );
+      QCOMPARE( capabilities.supportedLayers()[2].name, QString( "land_surveing_parcels" ) );
+      QCOMPARE( capabilities.supportedLayers()[3].name, QString( "cadastre" ) );
+      QCOMPARE( capabilities.supportedLayers()[4].name, QString( "test" ) );
+
+      // make sure the default style is not seen twice in the child layers
+      QCOMPARE( capabilities.supportedLayers()[3].style.size(), 1 );
+      QCOMPARE( capabilities.supportedLayers()[3].style[0].name, QString( "default" ) );
+      QCOMPARE( capabilities.supportedLayers()[1].style.size(), 1 );
+      QCOMPARE( capabilities.supportedLayers()[1].style[0].name, QString( "default" ) );
+      QCOMPARE( capabilities.supportedLayers()[2].style.size(), 1 );
+      QCOMPARE( capabilities.supportedLayers()[2].style[0].name, QString( "default" ) );
+
+      // check it can read 2 styles for a layer and that the legend URL is OK
+      QCOMPARE( capabilities.supportedLayers()[0].style.size(), 2 );
+      QCOMPARE( capabilities.supportedLayers()[0].style[0].name, QString( "yt_style" ) );
+      QCOMPARE( capabilities.supportedLayers()[0].style[0].legendUrl.size(), 1 );
+      QCOMPARE( capabilities.supportedLayers()[0].style[0].legendUrl[0].onlineResource.xlinkHref,
+                QString( "http://www.example.com/yt.png" ) );
+      QCOMPARE( capabilities.supportedLayers()[0].style[1].name, QString( "fb_style" ) );
+      QCOMPARE( capabilities.supportedLayers()[0].style[1].legendUrl.size(), 1 );
+      QCOMPARE( capabilities.supportedLayers()[0].style[1].legendUrl[0].onlineResource.xlinkHref,
+                QString( "http://www.example.com/fb.png" ) );
+    }
+
+};
+
+QTEST_MAIN( TestQgsWmsCapabilities )
+#include "testqgswmscapabilities.moc"

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"