Skip to content

Commit

Permalink
Fix the implementation of isCollectingNSAttributes #135. Deal properly
Browse files Browse the repository at this point in the history
with the empty namespace, also ensure to deal with sealed types and
force remapping of prefixes (overriding the policy).
  • Loading branch information
pdvrieze committed Apr 17, 2023
1 parent 885c83a commit e6267af
Show file tree
Hide file tree
Showing 4 changed files with 267 additions and 7 deletions.
6 changes: 6 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ Fixes:
- Make the StAXReader not skip the StartDocument event initially.
- Make XmlBufferedReader.nextTagEvent process/ignore StartDocument.
- Made ignorable whitespace more consistent. #128
- Fix handling of `isCollectingNSAttributes` (#135). This will now properly
handle sealed polymorphism as well as deal properly with the default
namespace: properties without prefix will not register the null namespace.
If the default namespace is used anywhere, this ensures that prefixes are
used otherwise. This will avoid all occurences of `xmlns=""` (and
`xmlns:prefix=""`)

# 0.85.0 – Tying things up
*(Feb 19, 2023)<br />*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ package nl.adaptivity.xmlutil.serialization

import kotlinx.serialization.*
import kotlinx.serialization.descriptors.SerialDescriptor
import kotlinx.serialization.descriptors.SerialKind
import kotlinx.serialization.descriptors.StructureKind
import kotlinx.serialization.encoding.CompositeDecoder
import kotlinx.serialization.modules.EmptySerializersModule
import kotlinx.serialization.modules.SerializersModule
Expand All @@ -33,8 +35,10 @@ import nl.adaptivity.xmlutil.core.impl.multiplatform.Language
import nl.adaptivity.xmlutil.core.impl.multiplatform.StringWriter
import nl.adaptivity.xmlutil.core.impl.multiplatform.use
import nl.adaptivity.xmlutil.serialization.XML.Companion.encodeToWriter
import nl.adaptivity.xmlutil.serialization.impl.*
import nl.adaptivity.xmlutil.serialization.impl.ChildCollector
import nl.adaptivity.xmlutil.serialization.impl.NamespaceCollectingXmlWriter
import nl.adaptivity.xmlutil.serialization.impl.PrefixWrappingPolicy
import nl.adaptivity.xmlutil.serialization.impl.XmlQNameSerializer
import nl.adaptivity.xmlutil.serialization.structure.XmlAttributeMapDescriptor
import nl.adaptivity.xmlutil.serialization.structure.XmlDescriptor
Expand Down Expand Up @@ -251,12 +255,24 @@ public class XML constructor(
val xmlDescriptor = root.getElementDescriptor(0)

val encoder = when {
config.isCollectingNSAttributes ->
config.isCollectingNSAttributes -> {
val collectedNamespaces = collectNamespaces(xmlDescriptor, xmlEncoderBase, serializer, value)
val prefixMap = collectedNamespaces.associate { it.namespaceURI to it.prefix }
val newConfig = XmlConfig(XmlConfig.Builder(config).apply {
policy = PrefixWrappingPolicy(policy ?: policyBuilder().build(), prefixMap)
})
val remappedEncoderBase = XmlEncoderBase(serializersModule, newConfig, target)
val newRootName = rootName?.remapPrefix(prefixMap)
val newRoot = XmlRootDescriptor(remappedEncoderBase, serializer.descriptor, newRootName)
val newDescriptor = newRoot.getElementDescriptor(0)


xmlEncoderBase.NSAttrXmlEncoder(
xmlDescriptor,
collectNamespaces(xmlDescriptor, xmlEncoderBase, serializer, value),
newDescriptor,
collectedNamespaces,
-1
)
}

else -> xmlEncoderBase.XmlEncoder(xmlDescriptor, -1)
}
Expand All @@ -281,7 +297,18 @@ public class XML constructor(
fun collect(prefix: String, namespaceUri: String) {
if (namespaceUri !in namespaceToPrefixMap) {
if (prefix in prefixToNamespaceMap) { // prefix with different usage
pendingNamespaces.add(namespaceUri)
// For the default namespace, always force this to be the empty prefix (remap
// all other namespaces)
if (namespaceUri.isEmpty()) {
prefixToNamespaceMap[""]?.let { oldDefaultNamespace ->
pendingNamespaces.add(oldDefaultNamespace)
namespaceToPrefixMap.remove(oldDefaultNamespace)
}
prefixToNamespaceMap[""] = ""
namespaceToPrefixMap[""] = ""
} else {
pendingNamespaces.add(namespaceUri)
}
} else { // Prefix has not been seen before
if (namespaceUri in pendingNamespaces) { // If it matches a pending namespace use that
pendingNamespaces.remove(namespaceUri)
Expand All @@ -295,10 +322,23 @@ public class XML constructor(
fun collect(descriptor: XmlDescriptor) {
val prefix = descriptor.tagName.prefix
val namespaceUri = descriptor.tagName.namespaceURI
collect(prefix, namespaceUri)
/* Don't register attributes without prefix in the default namespace (that doesn't
* require namespace declarations). #135
*/
if (descriptor.effectiveOutputKind != OutputKind.Attribute || namespaceUri.isNotEmpty() || prefix.isNotEmpty()) {
collect(prefix, namespaceUri)
}

val childrenToCollect = mutableListOf<XmlDescriptor>()
if (descriptor is XmlPolymorphicDescriptor) {
childrenToCollect.addAll(descriptor.polyInfo.values)
}
for (elementIndex in 0 until descriptor.elementsCount) {
childrenToCollect.add(descriptor.getElementDescriptor(elementIndex))
}

for (childDescriptor in childrenToCollect) {

for (childIdx in 0 until descriptor.elementsCount) {
val childDescriptor = descriptor.getElementDescriptor(childIdx)
if (childDescriptor.overriddenSerializer == XmlQNameSerializer) {
throw QNamePresentException()
}
Expand Down Expand Up @@ -343,6 +383,7 @@ public class XML constructor(
}

return prefixToNamespaceMap.asSequence()
.filterNot { (prefix, ns) -> prefix.isEmpty() && ns.isEmpty() } // skip empy namespace
.map { XmlEvent.NamespaceImpl(it.key, it.value) }
.sortedBy { it.prefix }
.toList()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Copyright (c) 2023.
*
* This file is part of xmlutil.
*
* This file is licenced to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You should have received a copy of the license with the source distribution.
* Alternatively, you may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package nl.adaptivity.xmlutil.serialization.impl

import nl.adaptivity.xmlutil.*
import nl.adaptivity.xmlutil.serialization.OutputKind
import nl.adaptivity.xmlutil.serialization.XmlSerializationPolicy
import nl.adaptivity.xmlutil.serialization.structure.SafeParentInfo

internal class PrefixWrappingPolicy(val basePolicy: XmlSerializationPolicy, val prefixMap: Map<String, String>) : XmlSerializationPolicy by basePolicy {
private fun QName.remapPrefix(): QName {
val prefixMap = prefixMap
return remapPrefix(prefixMap)
}

override fun effectiveName(
serializerParent: SafeParentInfo,
tagParent: SafeParentInfo,
outputKind: OutputKind,
useName: XmlSerializationPolicy.DeclaredNameInfo
): QName {
return basePolicy.effectiveName(serializerParent, tagParent, outputKind, useName).remapPrefix()
}

override fun serialTypeNameToQName(
typeNameInfo: XmlSerializationPolicy.DeclaredNameInfo,
parentNamespace: Namespace
): QName {
return basePolicy.serialTypeNameToQName(typeNameInfo, parentNamespace).remapPrefix()
}

override fun serialUseNameToQName(
useNameInfo: XmlSerializationPolicy.DeclaredNameInfo,
parentNamespace: Namespace
): QName {
return basePolicy.serialUseNameToQName(useNameInfo, parentNamespace).remapPrefix()
}

@Deprecated("It is recommended to override serialTypeNameToQName and serialUseNameToQName instead")
override fun serialNameToQName(serialName: String, parentNamespace: Namespace): QName {
@Suppress("DEPRECATION")
return basePolicy.serialNameToQName(serialName, parentNamespace).remapPrefix()
}

override fun mapEntryName(serializerParent: SafeParentInfo, isListEluded: Boolean): QName {
return super.mapEntryName(serializerParent, isListEluded).remapPrefix()
}
}

internal fun QName.remapPrefix(prefixMap: Map<String, String>): QName {
return QName(namespaceURI, localPart, prefixMap[namespaceURI] ?: prefix)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/*
* Copyright (c) 2021.
*
* This file is part of xmlutil.
*
* This file is licenced to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You should have received a copy of the license with the source distribution.
* Alternatively, you may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

@file:UseSerializers(QNameSerializer::class)
@file:OptIn(ExperimentalXmlUtilApi::class)

package nl.adaptivity.xml.serialization

import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
import kotlinx.serialization.UseSerializers
import kotlinx.serialization.json.Json
import nl.adaptivity.xmlutil.ExperimentalXmlUtilApi
import nl.adaptivity.xmlutil.QName
import nl.adaptivity.xmlutil.QNameSerializer
import nl.adaptivity.xmlutil.serialization.*
import kotlin.test.*

/**
* Test to check that namespace collecting works "better": #135
*/
class CollectingDefaultNamespaceTest : PlatformTestBase<CollectingDefaultNamespaceTest.GPXv11>(
GPXv11("kotlinx.serialization", tracks = listOf(
GPXv11.Track("Test", GPXv11.Track.Extensions(
GPXv11.Track.Extensions.TrackExtension("red"),
GPXv11.Track.Extensions.DefaultNsExtension("Some comment")
))
)),
GPXv11.serializer(),
baseXmlFormat = XML { isCollectingNSAttributes = true; autoPolymorphic = true; indent = 4 },
baseJsonFormat = Json { encodeDefaults = false }
) {
override val expectedXML: String =
"""<ns1:gpx xmlns:ns1="http://www.topografix.com/GPX/1/1" xmlns:gpxx="http://www.garmin.com/xmlschemas/GpxExtensions/v3" creator="kotlinx.serialization" version="1.1">
<ns1:trk>
<ns1:name>Test</ns1:name>
<ns1:extensions>
<gpxx:TrackExtension>
<gpxx:DisplayColor>red</gpxx:DisplayColor>
</gpxx:TrackExtension>
<comment data="Some comment"/>
</ns1:extensions>
</ns1:trk>
</ns1:gpx>"""
override val expectedJson: String =
"{\"creator\":\"kotlinx.serialization\",\"tracks\":[{\"name\":\"Test\",\"extensions\":{\"gpxx\":[{\"type\":\"nl.adaptivity.xml.serialization.CollectingDefaultNamespaceTest.GPXv11.Track.Extensions.TrackExtension\",\"DisplayColor\":\"red\"},{\"type\":\"nl.adaptivity.xml.serialization.CollectingDefaultNamespaceTest.GPXv11.Track.Extensions.DefaultNsExtension\",\"data\":\"Some comment\"}]}}]}"

@Test
override fun testGenericSerializeXml() {
super.testGenericSerializeXml()
}

@Test
fun testNamespaceDecls() {
val serialized = baseXmlFormat.encodeToString(serializer, value)
val lines = serialized.lines()
for ((idx, line) in lines.drop(1).withIndex()) {
assertFalse(actual = "xmlns" in line, "Namespace declaration found in line ${idx+1}: $line")
}
val xmlDecls = lines[0].split(" ")
.filter { it.startsWith("xmlns:") }

assertContains(xmlDecls, "xmlns:gpxx=\"http://www.garmin.com/xmlschemas/GpxExtensions/v3\"")
assertEquals(1, xmlDecls.count { "=\"http://www.topografix.com/GPX/1/1\"" in it }, "Namespace declaration for topographix not found")
assertEquals(2, xmlDecls.size)
}

@Serializable
@XmlSerialName("gpx", GPXv11.NAMESPACE, "")
data class GPXv11(
val creator: String,
val version: String = "1.1",
val tracks: List<Track>
) {
companion object {
const val NAMESPACE = "http://www.topografix.com/GPX/1/1"
}

@Serializable
@SerialName("trk")
data class Track(
@XmlElement(true)
val name: String? = null,
@XmlElement(true)
val extensions: Extensions? = null
) {
@Serializable
@SerialName("extensions")
class Extensions(
vararg val gpxx: Extension,
) {



override fun equals(other: Any?): Boolean {
if (this === other) return true
if (other == null || this::class != other::class) return false

other as Extensions

return gpxx.contentEquals(other.gpxx)
}

override fun hashCode(): Int {
return gpxx.contentHashCode()
}

@Serializable
sealed class Extension

/// http://www8.garmin.com/xmlschemas/GpxExtensionsv3.xsd
@Serializable
@XmlSerialName("TrackExtension", "http://www.garmin.com/xmlschemas/GpxExtensions/v3", "gpxx")
data class TrackExtension(
@SerialName("DisplayColor") @XmlElement(true)
val displayColor: String? = null,
): Extension()

@Serializable
@XmlSerialName("comment", "", "")
data class DefaultNsExtension(val data: String): Extension()
}
}
}

}

0 comments on commit e6267af

Please sign in to comment.