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

Allow free naming of project properties #60855

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 29 additions & 29 deletions src/core/project/qgsproject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,21 +116,6 @@ QStringList makeKeyTokens_( const QString &scope, const QString &key )
// be sure to include the canonical root node
keyTokens.push_front( QStringLiteral( "properties" ) );

//check validy of keys since an invalid xml name will will be dropped upon saving the xml file. If not valid, we print a message to the console.
for ( int i = 0; i < keyTokens.size(); ++i )
{
const QString keyToken = keyTokens.at( i );

//invalid chars in XML are found at http://www.w3.org/TR/REC-xml/#NT-NameChar
//note : it seems \x10000-\xEFFFF is valid, but it when added to the regexp, a lot of unwanted characters remain
const thread_local QRegularExpression sInvalidRegexp = QRegularExpression( QStringLiteral( "([^:A-Z_a-z\\x{C0}-\\x{D6}\\x{D8}-\\x{F6}\\x{F8}-\\x{2FF}\\x{370}-\\x{37D}\\x{37F}-\\x{1FFF}\\x{200C}-\\x{200D}\\x{2070}-\\x{218F}\\x{2C00}-\\x{2FEF}\\x{3001}-\\x{D7FF}\\x{F900}-\\x{FDCF}\\x{FDF0}-\\x{FFFD}\\-\\.0-9\\x{B7}\\x{0300}-\\x{036F}\\x{203F}-\\x{2040}]|^[^:A-Z_a-z\\x{C0}-\\x{D6}\\x{D8}-\\x{F6}\\x{F8}-\\x{2FF}\\x{370}-\\x{37D}\\x{37F}-\\x{1FFF}\\x{200C}-\\x{200D}\\x{2070}-\\x{218F}\\x{2C00}-\\x{2FEF}\\x{3001}-\\x{D7FF}\\x{F900}-\\x{FDCF}\\x{FDF0}-\\x{FFFD}])" ) );
if ( keyToken.contains( sInvalidRegexp ) )
{
const QString errorString = QObject::tr( "Entry token invalid : '%1'. The token will not be saved to file." ).arg( keyToken );
QgsMessageLog::logMessage( errorString, QString(), Qgis::MessageLevel::Critical );
}
}

return keyTokens;
}

Expand Down Expand Up @@ -1323,20 +1308,20 @@ void dump_( const QgsProjectPropertyKey &topQgsPropertyKey )
* scope. "layers" is a list containing three string values.
*
* \code{.xml}
* <properties>
* <fsplugin>
* <foo type="int" >42</foo>
* <baz type="int" >1</baz>
* <layers type="QStringList" >
* <properties name="properties">
* <properties name="fsplugin">
* <properties name="foo" type="int" >42</properties>
* <properties name="baz" type="int" >1</properties>
* <properties name="layers" type="QStringList">
* <value>railroad</value>
* <value>airport</value>
* </layers>
* <xyqzzy type="int" >1</xyqzzy>
* <bar type="double" >123.456</bar>
* <feature_types type="QStringList" >
* </properties>
* <properties name="xyqzzy" type="int" >1</properties>
* <properties name="bar" type="double" >123.456</properties>
* <properties name="feature_types" type="QStringList">
* <value>type</value>
* </feature_types>
* </fsplugin>
* </properties>
* </properties>
* </properties>
* \endcode
*
Expand Down Expand Up @@ -3984,10 +3969,25 @@ bool QgsProject::createEmbeddedLayer( const QString &layerId, const QString &pro
const QDomElement propertiesElem = sProjectDocument.documentElement().firstChildElement( QStringLiteral( "properties" ) );
if ( !propertiesElem.isNull() )
{
const QDomElement absElem = propertiesElem.firstChildElement( QStringLiteral( "Paths" ) ).firstChildElement( QStringLiteral( "Absolute" ) );
if ( !absElem.isNull() )
QDomElement e = propertiesElem.firstChildElement( QStringLiteral( "Paths" ) );
if ( e.isNull() )
{
e = propertiesElem.firstChildElement( QStringLiteral( "properties" ) );
while ( !e.isNull() && e.attribute( QStringLiteral( "name" ) ) != QStringLiteral( "Paths" ) )
e = e.nextSiblingElement( QStringLiteral( "properties" ) );

e = e.firstChildElement( QStringLiteral( "properties" ) );
while ( !e.isNull() && e.attribute( QStringLiteral( "name" ) ) != QStringLiteral( "Absolute" ) )
e = e.nextSiblingElement( QStringLiteral( "properties" ) );
}
else
{
e = e.firstChildElement( QStringLiteral( "Absolute" ) );
}

if ( !e.isNull() )
{
useAbsolutePaths = absElem.text().compare( QLatin1String( "true" ), Qt::CaseInsensitive ) == 0;
useAbsolutePaths = e.text().compare( QLatin1String( "true" ), Qt::CaseInsensitive ) == 0;
}
}

Expand Down
47 changes: 28 additions & 19 deletions src/core/project/qgsprojectproperty.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,15 @@ bool QgsProjectPropertyValue::readXml( const QDomNode &keyNode )

// keyElement is created by parent QgsProjectPropertyKey
bool QgsProjectPropertyValue::writeXml( QString const &nodeName,
QDomElement &keyElement,
QDomDocument &document )
QDomElement &keyElement,
QDomDocument &document )
{
QDomElement valueElement = document.createElement( nodeName );
QDomElement valueElement = document.createElement( QStringLiteral( "properties" ) );

// remember the type so that we can rebuild it when the project is read in
valueElement.setAttribute( QStringLiteral( "name" ), nodeName );
valueElement.setAttribute( QStringLiteral( "type" ), mValue.typeName() );


// we handle string lists differently from other types in that we
// create a sequence of repeated elements to cover all the string list
// members; each value will be in a <value></value> tag.
Expand Down Expand Up @@ -362,33 +362,41 @@ bool QgsProjectPropertyKey::readXml( const QDomNode &keyNode )

while ( i < subkeys.count() )
{
const QDomNode subkey = subkeys.item( i );
QString name;

if ( subkey.nodeName() == QStringLiteral( "properties" ) &&
subkey.hasAttributes() && // if we have attributes
subkey.isElement() && // and we're an element
subkey.toElement().hasAttribute( QStringLiteral( "name" ) ) ) // and we have a "name" attribute
name = subkey.toElement().attribute( QStringLiteral( "name" ) );
else
name = subkey.nodeName();

// if the current node is an element that has a "type" attribute,
// then we know it's a leaf node; i.e., a subkey _value_, and not
// a subkey
if ( subkeys.item( i ).hasAttributes() && // if we have attributes
subkeys.item( i ).isElement() && // and we're an element
subkeys.item( i ).toElement().hasAttribute( QStringLiteral( "type" ) ) ) // and we have a "type" attribute
if ( subkey.hasAttributes() && // if we have attributes
subkey.isElement() && // and we're an element
subkey.toElement().hasAttribute( QStringLiteral( "type" ) ) ) // and we have a "type" attribute
{
// then we're a key value
delete mProperties.take( subkeys.item( i ).nodeName() );
mProperties.insert( subkeys.item( i ).nodeName(), new QgsProjectPropertyValue );
//
delete mProperties.take( name );
mProperties.insert( name, new QgsProjectPropertyValue );

QDomNode subkey = subkeys.item( i );

if ( !mProperties[subkeys.item( i ).nodeName()]->readXml( subkey ) )
if ( !mProperties[name]->readXml( subkey ) )
{
QgsDebugError( QStringLiteral( "unable to parse key value %1" ).arg( subkeys.item( i ).nodeName() ) );
QgsDebugError( QStringLiteral( "unable to parse key value %1" ).arg( name ) );
}
}
else // otherwise it's a subkey, so just recurse on down the remaining keys
{
addKey( subkeys.item( i ).nodeName() );

QDomNode subkey = subkeys.item( i );
addKey( name );

if ( !mProperties[subkeys.item( i ).nodeName()]->readXml( subkey ) )
if ( !mProperties[name]->readXml( subkey ) )
{
QgsDebugError( QStringLiteral( "unable to parse subkey %1" ).arg( subkeys.item( i ).nodeName() ) );
QgsDebugError( QStringLiteral( "unable to parse subkey %1" ).arg( name ) );
}
}

Expand All @@ -408,7 +416,8 @@ bool QgsProjectPropertyKey::writeXml( QString const &nodeName, QDomElement &elem
// If it's an _empty_ node (i.e., one with no properties) we need to emit
// an empty place holder; else create new Dom elements as necessary.

QDomElement keyElement = document.createElement( nodeName ); // Dom element for this property key
QDomElement keyElement = document.createElement( "properties" ); // Dom element for this property key
keyElement.toElement().setAttribute( QStringLiteral( "name" ), nodeName );

if ( ! mProperties.isEmpty() )
{
Expand Down
78 changes: 0 additions & 78 deletions tests/src/python/test_qgsproject.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,84 +63,6 @@ def __init__(self, methodName):
QgisTestCase.__init__(self, methodName)
self.messageCaught = False

def test_makeKeyTokens_(self):
# see http://www.w3.org/TR/REC-xml/#d0e804 for a list of valid characters

invalidTokens = []
validTokens = []

# all test tokens will be generated by prepending or inserting characters to this token
validBase = "valid"

# some invalid characters, not allowed anywhere in a token
# note that '/' must not be added here because it is taken as a separator by makeKeyTokens_()
invalidChars = "+*,;<>|!$%()=?#\x01"

# generate the characters that are allowed at the start of a token (and at every other position)
validStartChars = ":_"
charRanges = [
(ord("a"), ord("z")),
(ord("A"), ord("Z")),
(0x00F8, 0x02FF),
(0x0370, 0x037D),
(0x037F, 0x1FFF),
(0x200C, 0x200D),
(0x2070, 0x218F),
(0x2C00, 0x2FEF),
(0x3001, 0xD7FF),
(0xF900, 0xFDCF),
(0xFDF0, 0xFFFD),
# (0x10000, 0xEFFFF), while actually valid, these are not yet accepted by makeKeyTokens_()
]
for r in charRanges:
for c in range(r[0], r[1]):
validStartChars += chr(c)

# generate the characters that are only allowed inside a token, not at the start
validInlineChars = "-.\xB7"
charRanges = [
(ord("0"), ord("9")),
(0x0300, 0x036F),
(0x203F, 0x2040),
]
for r in charRanges:
for c in range(r[0], r[1]):
validInlineChars += chr(c)

# test forbidden start characters
for c in invalidChars + validInlineChars:
invalidTokens.append(c + validBase)

# test forbidden inline characters
for c in invalidChars:
invalidTokens.append(validBase[:4] + c + validBase[4:])

# test each allowed start character
for c in validStartChars:
validTokens.append(c + validBase)

# test each allowed inline character
for c in validInlineChars:
validTokens.append(validBase[:4] + c + validBase[4:])

logger = QgsApplication.messageLog()
logger.messageReceived.connect(self.catchMessage)
prj = QgsProject.instance()

for token in validTokens:
self.messageCaught = False
prj.readEntry("test", token)
myMessage = f"valid token '{token}' not accepted"
assert not self.messageCaught, myMessage

for token in invalidTokens:
self.messageCaught = False
prj.readEntry("test", token)
myMessage = f"invalid token '{token}' accepted"
assert self.messageCaught, myMessage

logger.messageReceived.disconnect(self.catchMessage)

def catchMessage(self):
self.messageCaught = True

Expand Down
Loading