Skip to content

Commit

Permalink
Merge pull request #2880 from square/jwilson.0325.consistent_order
Browse files Browse the repository at this point in the history
Fix a bug where outputs weren't deterministic
  • Loading branch information
oldergod committed Mar 26, 2024
2 parents 5dc0081 + 24a10e8 commit b793a55
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package com.squareup.wire.schema

import com.squareup.wire.testing.add
import kotlin.test.assertFailsWith
import okio.ForwardingFileSystem
import okio.Path
import okio.Path.Companion.toPath
import okio.fakefilesystem.FakeFileSystem
Expand Down Expand Up @@ -480,10 +481,58 @@ class LinkerTest {
assertThat(enumValueDeprecated!!.encodeMode).isNotNull()
}

@Test
fun schemaIsDeterministicEvenIfProtoPathOrderIsNot() {
fs.add(
"source-path/a.proto",
"""
|message A {
|}
""".trimMargin(),
)
fs.add(
"source-path/b.proto",
"""
|import "a.proto";
|extend A {
| optional string b = 1;
|}
""".trimMargin(),
)
fs.add(
"source-path/c.proto",
"""
|import "a.proto";
|extend A {
| optional string c = 2;
|}
""".trimMargin(),
)

val schemaSorted = loadAndLinkSchema()
assertThat((schemaSorted.getType("A") as MessageType).extensionFields.map { it.name })
.containsExactly("b", "c")

val schemaReversed = loadAndLinkSchema(reverseSort = true)
assertThat((schemaReversed.getType("A") as MessageType).extensionFields.map { it.name })
.containsExactly("b", "c")
}

private fun loadAndLinkSchema(
opaqueTypes: List<ProtoType> = listOf(),
reverseSort: Boolean = false,
): Schema {
val loader = SchemaLoader(fs)
val schemaLoaderFileSystem = when {
reverseSort -> {
object : ForwardingFileSystem(fs) {
override fun listRecursively(dir: Path, followSymlinks: Boolean): Sequence<Path> =
super.listRecursively(dir, followSymlinks).toList().reversed().asSequence()
}
}
else -> fs
}

val loader = SchemaLoader(schemaLoaderFileSystem)
loader.opaqueTypes = opaqueTypes
loader.initRoots(
sourcePath = listOf(Location.get("source-path")),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ class Linker {
}
}

// The order of the input files shows up in the order of extension fields in the output files.
// Sort the inputs to get consistent output even when the order of input files is inconsistent.
sourceFiles.sortBy { it.protoFile.location.path }

for (fileLinker in sourceFiles) {
fileLinker.requireTypesRegistered()
}
Expand Down

0 comments on commit b793a55

Please sign in to comment.