Skip to content

Commit

Permalink
Redo [BUGFIX][QGIS Server] Joins was not reloaded if the layer is in …
Browse files Browse the repository at this point in the history 
…cache

With the commit f6aad8b, the QgsMapLayerRegistry signal `layersWillBeRemoved` is always emit. This imply that the vector layer join buffer is empty and not reloaded if the layer is in cache.

To fix it, the QgsServerProjectParser has to have the same method as QgsVectorLayerJoinBuffer::readXml.

This commit fixed #15522 Qgis Server doesnt' respect the styling from Desktop
  • Loading branch information
@rldhont
rldhont committed Oct 10, 2016
1 parent 801d4cd commit e34116d
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 2 deletions.
52 changes: 50 additions & 2 deletions src/server/qgsserverprojectparser.cpp
View file
Expand Up @@ -234,7 +234,11 @@ QgsMapLayer* QgsServerProjectParser::createLayerFromElement( const QDomElement&
if ( !QgsMapLayerRegistry::instance()->mapLayer( id ) )
QgsMapLayerRegistry::instance()->addMapLayer( layer, false, false );
if ( layer->type() == QgsMapLayer::VectorLayer )
addValueRelationLayersForLayer( dynamic_cast<QgsVectorLayer *>( layer ) );
{
QgsVectorLayer* vlayer = qobject_cast<QgsVectorLayer *>( layer );
addValueRelationLayersForLayer( vlayer );
addJoinsToLayer( const_cast<QDomElement&>( elem ), vlayer );
}

return layer;
}
Expand Down Expand Up @@ -1541,13 +1545,57 @@ void QgsServerProjectParser::addJoinLayersForElement( const QDomElement& layerEl
{
QString id = joinNodeList.at( i ).toElement().attribute( "joinLayerId" );
QgsMapLayer* layer = mapLayerFromLayerId( id );
if ( layer )
if ( layer && !QgsMapLayerRegistry::instance()->mapLayer( id ) )
{
QgsMapLayerRegistry::instance()->addMapLayer( layer, false, false );
}
}
}

// Based on QgsVectorLayerJoinBuffer::readXml
void QgsServerProjectParser::addJoinsToLayer( const QDomElement& layerElem, QgsVectorLayer *vl ) const

This comment has been minimized.

Sign in to view

Copy link
@nyalldawson

nyalldawson Oct 10, 2016

Collaborator

Isn't it possible to refactor QgsVectorLayerJoinBuffer to avoid this duplication? It's things like this in server which has led to the current instability - people will update and change the format for the xml in QgsVectorLayerJoinBuffer::readXml without syncing the changes here.

This comment has been minimized.

Sign in to view

Copy link
@rldhont

rldhont Oct 11, 2016

Author Contributor

Hi @nyalldawson, I know that duplication is not the good way for helping maintaining and evolving code. The server needs a universal project parser. I think that all the readXml method has to be removed from Qgs_Layer_ classes, if we want something more reusable.

This comment has been minimized.

Sign in to view

Copy link
@nyalldawson

nyalldawson Oct 11, 2016

Collaborator

@rldhont I'm confused - why can't the existing method be modified to meet your needs?

As it stands this code is almost certain to break in future, because there's no unit tests covering it and no indication to anyone who modifies the join XML that the changes need to be duplicated here.

This comment has been minimized.

Sign in to view

Copy link
@rldhont

rldhont Oct 11, 2016

Author Contributor

@nyalldawson the QgsVectorLayerJoinBuffer* mJoinBuffer of the QgsVectorLayer is not accessible. I think I have 2 solutions:

  • generate a joinBuffer to readXml and then add the joins in the joinBuffer to the VectorLayer
  • add a method to VectorLayer to directly accessing its joinBuffer

But I think that we have to separate readXml methods from classes, QGIS classes will be simplier and all the Xml method are centralize in a classe. It's probably the base to have multi-project in QGIS and a more sustainable Server code, isn't it ?

This comment has been minimized.

Sign in to view

Copy link
@nyalldawson

nyalldawson Oct 11, 2016

Collaborator

Adding an accessor to the join buffer to QgsVectorLayer sounds good to me!

I'm not aware of any plans to change how XML is read/written, but multi projects is very much desired and hopefully will be in place for 3.0

This comment has been minimized.

Sign in to view

Copy link
@rldhont

rldhont Oct 11, 2016

Author Contributor

ok, but I think it was bad to add method to QgsVectorLayer #2454

This comment has been minimized.

Sign in to view

Copy link
@nyalldawson

nyalldawson Oct 11, 2016

Collaborator

There's no issue here- you couldn't have this accessor sitting outside the class

This comment has been minimized.

Sign in to view

Copy link
@rldhont

rldhont Oct 11, 2016

Author Contributor

;-) yeah but the readXml method could be outside in 3.0

This comment has been minimized.

Sign in to view

Copy link
@rldhont

rldhont Oct 11, 2016

Author Contributor

I'll create a PR for that
{
if ( !vl )
return;

QDomElement vectorJoinsElem = layerElem.firstChildElement( "vectorjoins" );
if ( vectorJoinsElem.isNull() )
{
return;
}

QDomNodeList joinList = vectorJoinsElem.elementsByTagName( "join" );
for ( int i = 0; i < joinList.size(); ++i )
{
QDomElement infoElem = joinList.at( i ).toElement();
QgsVectorJoinInfo info;
info.joinFieldName = infoElem.attribute( "joinFieldName" );
info.joinLayerId = infoElem.attribute( "joinLayerId" );
info.targetFieldName = infoElem.attribute( "targetFieldName" );
info.memoryCache = infoElem.attribute( "memoryCache" ).toInt();

info.joinFieldIndex = infoElem.attribute( "joinField" ).toInt(); //for compatibility with 1.x
info.targetFieldIndex = infoElem.attribute( "targetField" ).toInt(); //for compatibility with 1.x

QDomElement subsetElem = infoElem.firstChildElement( "joinFieldsSubset" );
if ( !subsetElem.isNull() )
{
QStringList* fieldNames = new QStringList;
QDomNodeList fieldNodes = infoElem.elementsByTagName( "field" );
for ( int i = 0; i < fieldNodes.count(); ++i )
*fieldNames << fieldNodes.at( i ).toElement().attribute( "name" );
info.setJoinFieldNamesSubset( fieldNames );
}

if ( infoElem.attribute( "hasCustomPrefix" ).toInt() )
info.prefix = infoElem.attribute( "customPrefix" );
else
info.prefix = QString::null;

vl->addJoin( info );
}
}

void QgsServerProjectParser::addValueRelationLayersForLayer( const QgsVectorLayer *vl ) const
{
if ( !vl )
Expand Down
3 changes: 3 additions & 0 deletions src/server/qgsserverprojectparser.h
View file
Expand Up @@ -112,7 +112,10 @@ class SERVER_EXPORT QgsServerProjectParser
QStringList wfsLayers() const;
QStringList wcsLayers() const;

/** Add layers for vector joins */
void addJoinLayersForElement( const QDomElement& layerElem ) const;
/** Update vector joins to layer, based on QgsVectorLayerJoinBuffer::readXml */
void addJoinsToLayer( const QDomElement& layerElem, QgsVectorLayer *vl ) const;

void addValueRelationLayersForLayer( const QgsVectorLayer *vl ) const;
/** Add layers which are necessary for the evaluation of the expression function 'getFeature( layer, attributField, value)'*/
Expand Down

0 comments on commit e34116d

Please sign in to comment.