Replace BiMap with plain maps #861
Copy link
Copy link
Closed
Description
graphql.kickstart.tools.util.BiMap is a copy of Guava's BiMap we added when we dropped Guava (661769e). Neither Java nor Kotlin has a bidirectional map, so it stuck around. After #851 it doesn't really pull its weight anymore.
Where it's used:
SchemaClassScannerbuilds aBiMap<TypeDefinition, JavaType>(TypeClassDictionary).SchemaParserreads it both ways:dictionary[definition]for enum classes anddictionary.inverse()forInterfaceTypeResolverandUnionTypeResolver.BiMap#putthrowingvalue already presentis the only thing that enforces one class per output type. It shows up asError creating bimap of type => class, which isn't a very helpful message. It also catches classes found through resolver return types, not just dictionary entries, so a replacement needs an explicit check.SchemaParserDictionarystores a plain map since Let input types share a dictionary class #851.getDictionary()still returns aBiMapfor backwards compatibility (graphql-spring-boot callsbuilder.dictionary(dictionary.getDictionary())), but itsinverse()only keeps the first name when several input types share a class.
Proposal:
- Internally, use two plain maps plus an explicit duplicate check that throws a
SchemaClassScannerErrornaming the class and the types. No API change.
typeToClass.entries.groupBy({ it.value }, { it.key })
.filterValues { it.size > 1 }
.forEach { (clazz, types) -> throw SchemaClassScannerError("...") }
val classToType = typeToClass.entries.associate { (type, clazz) -> clazz to type }- Add a replacement for
getDictionary()that returnsMap<String, Class<*>>under a new name, and deprecategetDictionary(). Changing the return type in place would break binary compatibility. - Switch graphql-spring-boot to the new method.
- Remove
BiMapin the next major.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels
Activity