Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #44 #87 #71 #88

Merged
merged 7 commits into from
Jan 24, 2019
Merged

Fixes #44 #87 #71 #88

merged 7 commits into from
Jan 24, 2019

Conversation

jgrocha
Copy link
Contributor

@jgrocha jgrocha commented Dec 17, 2017

This fixes bugs #44 #87 #71

Problem #44

This problem is related with SLDs where the layer name is not a valid XML identifier.

To reproduce the error, let us change one layer name, for example, with something like Città Metropolitana.
The layer will not be properly uploaded to Geoserver because the style upload will fail.

The style is uploaded with two different Geoserver API calls.

First call

The first call always works, because it uses the name defined in the interface by the user or suggested by the plugin.
The name suggested by the plugin is in this case Citta_Metropolitana (returned by xmlNameFixUp).

<style><name>Citta_Metropolitana</name><filename>Citta_Metropolitana.sld</filename></style>

Second call

The second call will upload the SLD style. As it is now, the sld is returned by getGsCompatibleSld, which is not aware of a alternative layer name.
The SLD will be:

<?xml version="1.0" encoding="UTF-8"?>
<StyledLayerDescriptor xmlns="http://www.opengis.net/sld" xmlns:ogc="http://www.opengis.net/ogc"
                       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.0.0"
                       xmlns:xlink="http://www.w3.org/1999/xlink"
                       xsi:schemaLocation="http://www.opengis.net/sld http://schemas.opengis.net/sld/1.0.0/StyledLayerDescriptor.xsd"
                       xmlns:sld="http://www.opengis.net/sld">
    <NamedLayer>
        <sld:Name>Città Metropolitana</sld:Name>
        <UserStyle>
            <sld:Name>Città Metropolitana</sld:Name>
            <sld:FeatureTypeStyle>

The upload will fail is this case, because <NamedLayer> must be a existing layer in Geoserver.

Solution

To make sure that the SLD <NamedLayer> is equal to the previous name used in the first API call, we can make a string replacement in the generated SLD.

sld =  sld.replace(layer.name(), name)

The previous SLD will be:

<?xml version="1.0" encoding="UTF-8"?>
<StyledLayerDescriptor xmlns="http://www.opengis.net/sld" xmlns:ogc="http://www.opengis.net/ogc"
                       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.0.0"
                       xmlns:xlink="http://www.w3.org/1999/xlink"
                       xsi:schemaLocation="http://www.opengis.net/sld http://schemas.opengis.net/sld/1.0.0/StyledLayerDescriptor.xsd"
                       xmlns:sld="http://www.opengis.net/sld">
    <NamedLayer>
        <sld:Name>Citta_Metropolitana</sld:Name>
        <UserStyle>
            <sld:Name>Citta_Metropolitana</sld:Name>
            <sld:FeatureTypeStyle>

The upload of the SLD will succeed.

Discussion

In this PR, I've changed xmlNameFixUp to:

def xmlNameFixUp(name):
    n = unicode(unicodedata.normalize('NFKD', name).encode('ascii','ignore'))
    n = n.replace(u' ', u'_')  # doesn't hurt to always do this
    if not xmlNameIsValid(n) and not n.startswith(u'_'):
        rx = QtCore.QRegExp(r'^(?=XML|\d|\W).*', QtCore.Qt.CaseInsensitive)
        if rx.exactMatch(n):
            n = u"_{0}".format(n)
    return n

Geoserver accepts and handles stores and styles with UFT-8 names. The name Città_Metropolitana is a valid identifier in Geoserver.

But if we use UTF-8 names for layers or styles, the plugin will break, bug #70

Since we need to rewrite the entire plugin for QGIS 3 and Python 3, we can postpone UTF-8 identifies support for the next major release.

Replace propor layer.name() with an xml compliant ascii version of it
Keeps UTF-8 characters
@jgrocha jgrocha changed the title Fixes #44 #87 Fixes #44 #87 #71 Dec 17, 2017
@volaya
Copy link
Contributor

volaya commented Dec 18, 2017

Very nice job. Look like it is breaking the CI build, though. Not sure it's related to this change, can you confirm?

@jgrocha
Copy link
Contributor Author

jgrocha commented Dec 18, 2017

@volaya The CI was already broken. Maybe we need some help from @elpaso to review the test environment.

This PR does not break any tests in my local environment.

I didn't added any additional test to guarantee that layer names and styles are properly handled by the plugin. It is just a matter of using UTF-8 identifiers in the layers and styles used in the tests.

I can add them later, because we still need to fix #70.

@jgrocha jgrocha changed the title Fixes #44 #87 #71 WIP: Fixes #44 #87 #71 Dec 20, 2017
@jgrocha
Copy link
Contributor Author

jgrocha commented Dec 20, 2017

I've added WIP to the PR title.
I would like to test it with more use cases.
I also would like to check why CI is broken,

@jgrocha jgrocha changed the title WIP: Fixes #44 #87 #71 Fixes #44 #87 #71 Dec 22, 2017
@jgrocha
Copy link
Contributor Author

jgrocha commented Dec 22, 2017

These fixes depends on https://github.com/boundlessgeo/gsconfig/pull/175

@volaya volaya merged commit b0658f7 into planetfederal:master Jan 24, 2019
@volaya
Copy link
Contributor

volaya commented Jan 24, 2019

The CI is not working at the moment, it requires some extra setup, so I am mergin this anyway, since it looks safe and it has been here for too long

Thanks!

@luipir
Copy link
Contributor

luipir commented Jan 24, 2019

countdown ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants