Repository: Microsoft/thrifty (kotlin)

Issue:
Struct-valued constants are not yet implemented
As it stands today, the current thrifty schema is not able to parse struct valued constants. Wanted to know if this is a work in progress? If not, was this something consciously left out from implementation?

I am trying to load schemas for manipulating certain exceptions based on added annotations. Given that we only care about exceptions structs, is it possible to silently fail the constant schema generation so that it does not throw `IllegalStateException`? 

Several of our thrift definitions have struct valued constants so wondering if there is any workaround?

Pull request description:
This commit resolves #466

Patch:
diff --git a/thrifty-schema/src/main/kotlin/com/microsoft/thrifty/schema/Constant.kt b/thrifty-schema/src/main/kotlin/com/microsoft/thrifty/schema/Constant.kt
index 6479875a5..189d8ee23 100644
--- a/thrifty-schema/src/main/kotlin/com/microsoft/thrifty/schema/Constant.kt
+++ b/thrifty-schema/src/main/kotlin/com/microsoft/thrifty/schema/Constant.kt
@@ -106,6 +106,7 @@ class Constant private constructor (
         private val ENUM = EnumValidator
         private val COLLECTION = CollectionValidator
         private val MAP = MapValidator
+        private val STRUCT = StructValidator
 
         internal fun forType(type: ThriftType): ConstValueValidator {
             val tt = type.trueType
@@ -142,7 +143,13 @@ class Constant private constructor (
                 return MAP
             }
 
-            throw IllegalStateException("Struct-valued constants are not yet implemented")
+            if (tt.isStruct) {
+                // this should work for exception type as well. structType has isException field
+                return STRUCT
+            }
+
+            throw IllegalStateException("Illegal const definition. " +
+                    "Const must be of type [bool, byte, i16, i32, i64, double, string, enum, list, set, map, struct]")
         }
     }
 
@@ -380,6 +387,30 @@ class Constant private constructor (
         }
     }
 
+    private object StructValidator : ConstValueValidator {
+        override fun validate(symbolTable: SymbolTable, expected: ThriftType, valueElement: ConstValueElement) {
+            if (valueElement is MapValueElement) { // struct valued constants should always be defined as a Map
+                val struct = expected as StructType
+                val fields = struct.fields
+                val map = valueElement.value
+                for ((key, value) in map) {
+                    if (key !is LiteralValueElement) {
+                        throw IllegalStateException("${expected.name} struct const keys must be string")
+                    }
+                    // validate the struct defined fields are listed in the const valued struct map
+                    // field name must match the map key
+                    val field = fields.firstOrNull { it.name == key.value }
+                            ?: throw IllegalStateException("${expected.name} struct has no field ${key.value}")
+                    Constant.validate(symbolTable, value, field.type)
+                }
+            } else {
+                throw IllegalStateException(
+                        "Type Error: const ${expected.name} should be declared as a Map"
+                )
+            }
+        }
+    }
+
     companion object {
         @VisibleForTesting
         internal fun validate(symbolTable: SymbolTable, value: ConstValueElement, expected: ThriftType) {


Test patch:
diff --git a/thrifty-schema/src/test/kotlin/com/microsoft/thrifty/schema/LoaderTest.kt b/thrifty-schema/src/test/kotlin/com/microsoft/thrifty/schema/LoaderTest.kt
index 02d0402f2..416396957 100644
--- a/thrifty-schema/src/test/kotlin/com/microsoft/thrifty/schema/LoaderTest.kt
+++ b/thrifty-schema/src/test/kotlin/com/microsoft/thrifty/schema/LoaderTest.kt
@@ -20,6 +20,7 @@
  */
 package com.microsoft.thrifty.schema
 
+import com.microsoft.thrifty.schema.parser.*
 import io.kotest.assertions.throwables.shouldThrow
 import io.kotest.assertions.throwables.shouldThrowExactly
 import io.kotest.assertions.throwables.shouldThrowMessage
@@ -28,6 +29,8 @@ import io.kotest.matchers.shouldBe
 import io.kotest.matchers.shouldNotBe
 import io.kotest.matchers.string.shouldContain
 import io.kotest.matchers.throwable.shouldHaveMessage
+import org.junit.jupiter.api.Assertions.assertTrue
+import org.junit.jupiter.api.Assertions.fail
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.io.TempDir
 import java.io.File
@@ -992,6 +995,173 @@ class LoaderTest {
         shouldThrowExactly<LoadFailedException> { load(thrift) }
     }
 
+    @Test
+    fun structValuedConstant() {
+        val thrift = """
+
+            enum Membership {
+                MEMBER = 0
+                NON_MEMBER = 1
+                UNDER_REVIEW = 2
+            }
+            
+            struct Location {
+                1: required string currencyCode
+                2: required list<string> languages
+                3: required Membership membership
+            }
+            
+            struct Region {
+                1: required list<Location> locations 
+                2: optional bool isActive
+            }
+
+            const Region DEFAULT_REGION = {
+                "locations": [{"currencyCode" : "USD", "languages" : ["English", "Spanish"], "membership" : Membership.MEMBER}],
+                "isActive" : true
+            }
+
+        """.trimIndent()
+
+        val schema = load(thrift)
+        val constants = schema.constants
+        constants.size shouldBe 1
+        val const = constants[0]
+        val map = const.value as MapValueElement
+        map.value.entries.size shouldBe 2
+        for ((key, value) in map.value.entries) {
+            assertTrue(key is LiteralValueElement)
+            val keyKey = (key as LiteralValueElement).value
+            when(keyKey) {
+                "locations" -> {
+                    val keyVal = (value as ListValueElement).value
+                    keyVal.size shouldBe 1
+                    val locationMap = keyVal[0] as MapValueElement
+                    locationMap.value.size shouldBe 3
+                    for ((locKey, locVal) in locationMap.value.entries) {
+                        when((locKey as LiteralValueElement).value) {
+                            "currencyCode" -> {
+                                assertTrue((locVal as LiteralValueElement).value == "USD")
+                            }
+                            "languages" -> {
+                                val languages = (locVal as ListValueElement).value
+                                languages.size shouldBe 2
+                                for (language in languages) {
+                                    val lang = (language as LiteralValueElement).value
+                                    assertTrue(lang in listOf("English", "Spanish"))
+                                }
+                            }
+                            "membership" -> {
+                                assertTrue((locVal as IdentifierValueElement).value == "Membership.MEMBER")
+                            }
+                            else -> {
+                                fail("Invalid const key; must be currencyCode, languages or membership")
+                            }
+                        }
+                    }
+                }
+                "isActive" -> {
+                    assertTrue((value as IdentifierValueElement).value == "true")
+                }
+                else -> {
+                    fail("Invalid const key; must be either locations or isActive")
+                }
+            }
+        }
+    }
+
+    @Test
+    fun invalidStructValuedConstant() {
+        val thrift = """
+            
+            struct Region {
+                1: required string name 
+                2: optional bool isActive
+            }
+
+            const UnknownRegion DEFAULT_REGION = {
+                "name": "US",
+                "nonActive" : true // field name does not exist in Region
+            }
+
+        """.trimIndent()
+
+        val e = shouldThrow<LoadFailedException> { load(thrift) }
+        e.message shouldContain "Failed to resolve type 'UnknownRegion'"
+    }
+
+    @Test
+    fun invalidKeyStructValuedConstant() {
+        val thrift = """
+            
+            struct Region {
+                1: required string name 
+                2: optional bool isActive
+            }
+
+            const Region DEFAULT_REGION = {
+                // 1 is not a string; invalid definition
+                1 : "US",
+                "isActive" : true
+            }
+
+        """.trimIndent()
+
+        val e = shouldThrow<LoadFailedException> { load(thrift) }
+        e.message shouldContain "Region struct const keys must be string"
+    }
+
+    @Test
+    fun invalidFieldStructValuedConstant() {
+        val thrift = """
+            
+            struct Region {
+                1: required string name 
+                2: optional bool isActive
+            }
+
+            const Region DEFAULT_REGION = {
+                "name": "US",
+                "nonActive" : true // field name does not exist in Region
+            }
+
+        """.trimIndent()
+
+        val e = shouldThrow<LoadFailedException> { load(thrift) }
+        e.message shouldContain "Region struct has no field nonActive"
+    }
+
+    @Test
+    fun invalidEnumStructValuedConst() {
+        val thrift = """
+
+            enum Membership {
+                MEMBER = 0
+                NON_MEMBER = 1
+                UNDER_REVIEW = 2
+            }
+            
+            struct Location {
+                1: required Membership membership
+            }
+            
+            struct Region {
+                1: required list<Location> locations 
+            }
+
+            // invalid Enum
+            const Region DEFAULT_REGION = {
+                "locations": [{"membership" : Membership.WRONG}]
+            }
+
+        """.trimIndent()
+
+        val e = shouldThrow<LoadFailedException> { load(thrift) }
+        e.message shouldContain "'Membership.WRONG' is not a member of enum type Membership: members=[MEMBER, NON_MEMBER, UNDER_REVIEW]"
+    }
+
+
+
     private fun load(thrift: String): Schema {
         val f = File.createTempFile("test", ".thrift", tempDir)
         f.writeText(thrift)