Skip to content

Commit

Permalink
use scala primitives (e.g. Integer -> Int) (#271)
Browse files Browse the repository at this point in the history
* use scala primitives (e.g. Integer -> Int)

motivation: minify diff for flatgraph migration

* fix autoconversion bug

attention with autoconversions around `Option[Int]` and similar:

```
scala> Option(null: Integer)
val res0: Option[Integer] = None

scala> val o: Option[Int] = Option(null: Integer)
val o: Option[Int] = Some(0)

scala> val o: Option[Int] = Option(null: Integer).asInstanceOf[Option[Int]]
val o: Option[Int] = None
```

* Properties.java -> Properties.scala to avoid java interop issues

scala performs quite some magic to handle Option[Int], and the bytecode looks
differently if javac or scalac compile something like `val x: Option[Int]`:
```
// scalac:
public static flatgraph.SinglePropertyKey<java.lang.Object> ArgumentIndex();

//javac:
public static final overflowdb.PropertyKey<scala.Int> ARGUMENT_INDEX;
```

Hence, the best way forward is to define these constants in scala.
  • Loading branch information
mpollmeier authored Jun 26, 2024
1 parent 5c45082 commit 5042694
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 76 deletions.
71 changes: 40 additions & 31 deletions codegen/src/main/scala/overflowdb/codegen/CodeGen.scala
Original file line number Diff line number Diff line change
Expand Up @@ -254,20 +254,26 @@ class CodeGen(schema: Schema) {
})
}

writeConstantsFile("Properties", schema.properties.map { property =>
val src = {
// Properties.scala
val propertyKeysConstantsSource = {
schema.properties.map { property =>
val valueType = typeFor(property)
val cardinality = property.cardinality
import Property.Cardinality
val completeType = cardinality match {
case Cardinality.One(_) => valueType
case Cardinality.ZeroOrOne => valueType
case Cardinality.List => s"scala.collection.IndexedSeq<$valueType>"
val completeType = property.cardinality match {
case Property.Cardinality.One(_) => valueType
case Property.Cardinality.ZeroOrOne => valueType
case Property.Cardinality.List => s"IndexedSeq[$valueType]"
}
s"""public static final overflowdb.PropertyKey<$completeType> ${property.name} = new overflowdb.PropertyKey<>("${property.name}");"""
s"""val ${camelCaseCaps(property.name)}: overflowdb.PropertyKey[$completeType] = new overflowdb.PropertyKey("${property.name}")""".stripMargin.trim
}
ConstantContext(property.name, src, property.comment)
})
}.mkString("\n\n")
val file = baseDir.createChild("Properties.scala").write(
s"""package ${schema.basePackage}
|
|object Properties {
|$propertyKeysConstantsSource
|}""".stripMargin
)
results.append(file)

results.toSeq
}
Expand Down Expand Up @@ -343,7 +349,7 @@ class CodeGen(schema: Schema) {
properties.map { property =>
val name = property.name
val nameCamelCase = camelCase(name)
val tpe = getCompleteType(property)
val tpe = getCompleteType(property, nullable = false)

property.cardinality match {
case Cardinality.One(_) =>
Expand Down Expand Up @@ -479,7 +485,7 @@ class CodeGen(schema: Schema) {
def generateNodeBaseTypeSource(nodeBaseType: NodeBaseType): String = {
val className = nodeBaseType.className
val properties = nodeBaseType.properties
val propertyAccessors = Helpers.propertyAccessors(properties)
val propertyAccessors = Helpers.propertyAccessors(properties, nullable = false)

val mixinsForBaseTypes = nodeBaseType.extendz.map { baseTrait =>
s"with ${baseTrait.className}"
Expand Down Expand Up @@ -601,7 +607,7 @@ class CodeGen(schema: Schema) {

val newNodePropertySetters = nodeBaseType.properties.map { property =>
val camelCaseName = camelCase(property.name)
val tpe = getCompleteType(property)
val tpe = getCompleteType(property, nullable = false)
s"def ${camelCaseName}_=(value: $tpe): Unit"
}.mkString(lineSeparator)

Expand Down Expand Up @@ -856,7 +862,7 @@ class CodeGen(schema: Schema) {
case Cardinality.One(_) =>
s"""this._$memberName = $newNodeCasted.$memberName""".stripMargin
case Cardinality.ZeroOrOne =>
s"""this._$memberName = $newNodeCasted.$memberName.orNull""".stripMargin
s"""this._$memberName = $newNodeCasted.$memberName match { case None => null; case Some(value) => value }""".stripMargin
case Cardinality.List =>
s"""this._$memberName = if ($newNodeCasted.$memberName != null) $newNodeCasted.$memberName else collection.immutable.ArraySeq.empty""".stripMargin
}
Expand Down Expand Up @@ -993,7 +999,7 @@ class CodeGen(schema: Schema) {
s"""trait ${className}Base extends AbstractNode $mixinsForExtendedNodesBase $mixinsForMarkerTraits {
| def asStored : StoredNode = this.asInstanceOf[StoredNode]
|
| ${Helpers.propertyAccessors(properties)}
| ${Helpers.propertyAccessors(properties, nullable = false)}
|
| $abstractContainedNodeAccessors
|}
Expand Down Expand Up @@ -1021,7 +1027,7 @@ class CodeGen(schema: Schema) {
val nodeRefImpl = {
val propertyDelegators = properties.map { key =>
val name = camelCase(key.name)
s"""override def $name: ${getCompleteType(key)} = get().$name"""
s"""override def $name: ${getCompleteType(key, nullable = false)} = get().$name"""
}.mkString(lineSeparator)

val propertyDefaultValues = propertyDefaultValueImpl(s"$className.PropertyDefaults", properties)
Expand Down Expand Up @@ -1095,14 +1101,14 @@ class CodeGen(schema: Schema) {

val updateSpecificPropertyImpl: String = {
import Property.Cardinality
def caseEntry(name: String, accessorName: String, cardinality: Cardinality, baseType: String) = {
def caseEntry(name: String, accessorName: String, cardinality: Cardinality, baseType: String, baseTypeNullable: String) = {
val setter = cardinality match {
case Cardinality.One(_) | Cardinality.ZeroOrOne =>
s"value.asInstanceOf[$baseType]"
case Cardinality.List =>
s"""value match {
| case null => collection.immutable.ArraySeq.empty
| case singleValue: $baseType => collection.immutable.ArraySeq(singleValue)
| case singleValue: $baseTypeNullable => collection.immutable.ArraySeq(singleValue)
| case coll: IterableOnce[Any] if coll.iterator.isEmpty => collection.immutable.ArraySeq.empty
| case arr: Array[_] if arr.isEmpty => collection.immutable.ArraySeq.empty
| case arr: Array[_] => collection.immutable.ArraySeq.unsafeWrapArray(arr).asInstanceOf[IndexedSeq[$baseType]]
Expand All @@ -1120,10 +1126,10 @@ class CodeGen(schema: Schema) {
s"""|case "$name" => this._$accessorName = $setter"""
}

val forKeys = properties.map(p => caseEntry(p.name, camelCase(p.name), p.cardinality, typeFor(p))).mkString(lineSeparator)
val forKeys = properties.map(p => caseEntry(p.name, camelCase(p.name), p.cardinality, typeFor(p), typeFor(p, nullable = true))).mkString(lineSeparator)

val forContainedNodes = nodeType.containedNodes.map(containedNode =>
caseEntry(containedNode.localName, containedNode.localName, containedNode.cardinality, containedNode.classNameForStoredNode)
caseEntry(containedNode.localName, containedNode.localName, containedNode.cardinality, containedNode.classNameForStoredNode, containedNode.classNameForStoredNode)
).mkString(lineSeparator)

s"""override protected def updateSpecificProperty(key:String, value: Object): Unit = {
Expand Down Expand Up @@ -1161,10 +1167,11 @@ class CodeGen(schema: Schema) {
val fieldName = s"_$publicName"
val (publicType, tpeForField, fieldAccessor, defaultValue) = {
val valueType = typeFor(property)
val valueTypeNullable = typeFor(property, nullable = true)
property.cardinality match {
case Cardinality.One(_) =>
(valueType, valueType, fieldName, s"$className.PropertyDefaults.${property.className}")
case Cardinality.ZeroOrOne => (s"Option[$valueType]", valueType, s"Option($fieldName)", "null")
(valueType, valueTypeNullable, fieldName, s"$className.PropertyDefaults.${property.className}")
case Cardinality.ZeroOrOne => (s"Option[$valueType]", valueTypeNullable, s"Option($fieldName).asInstanceOf[Option[$valueType]]", "null")
case Cardinality.List => (s"IndexedSeq[$valueType]", s"IndexedSeq[$valueType]", fieldName, "collection.immutable.ArraySeq.empty")
}
}
Expand Down Expand Up @@ -1732,27 +1739,29 @@ class CodeGen(schema: Schema) {

def generateNewNodeSource(nodeType: NodeType, properties: Seq[Property[?]], inEdges: Map[String, Set[String]], outEdges: Map[String, Set[String]]) = {
import Property.Cardinality
case class FieldDescription(name: String, valueType: String, fullType: String, cardinality: Cardinality)
case class FieldDescription(name: String, valueType: String, valueTypeNullable: String, fullType: String, cardinality: Cardinality)
val fieldDescriptions = mutable.ArrayBuffer.empty[FieldDescription]

for (property <- properties) {
fieldDescriptions += FieldDescription(
camelCase(property.name),
typeFor(property),
getCompleteType(property),
typeFor(property, nullable = true),
getCompleteType(property, nullable = false),
property.cardinality)
}

for (containedNode <- nodeType.containedNodes) {
fieldDescriptions += FieldDescription(
containedNode.localName,
typeFor(containedNode),
typeFor(containedNode),
getCompleteType(containedNode),
containedNode.cardinality)
}

val memberVariables = fieldDescriptions.reverse.map {
case FieldDescription(name, _, fullType, cardinality) =>
case FieldDescription(name, _, _, fullType, cardinality) =>
val defaultValue = cardinality match {
case Cardinality.One(default) => defaultValueImpl(default)
case Cardinality.ZeroOrOne => "None"
Expand Down Expand Up @@ -1812,22 +1821,22 @@ class CodeGen(schema: Schema) {
val mixins = nodeType.extendz.map{baseType => s"with ${baseType.className}New"}.mkString(" ")

val propertySettersImpl = fieldDescriptions.map {
case FieldDescription(name, valueType , _, cardinality) =>
case FieldDescription(name, valueType, valueTypeNullable, _, cardinality) =>
cardinality match {
case Cardinality.One(_) =>
s"""def $name(value: $valueType): this.type = {
s"""def $name(value: $valueTypeNullable): this.type = {
| this.$name = value
| this
|}
|""".stripMargin

case Cardinality.ZeroOrOne =>
s"""def $name(value: $valueType): this.type = {
| this.$name = Option(value)
s"""def $name(value: $valueTypeNullable): this.type = {
| this.$name = Option(value).asInstanceOf[Option[$valueType]]
| this
|}
|
|def $name(value: Option[$valueType]): this.type = $name(value.orNull)
|def $name(value: Option[$valueType]): this.type = $name(value match { case None => null; case Some(value) => value: $valueTypeNullable })
|""".stripMargin

case Cardinality.List =>
Expand Down
27 changes: 13 additions & 14 deletions codegen/src/main/scala/overflowdb/codegen/Helpers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,17 @@ object Helpers {
case nonEmptyString => Some(nonEmptyString)
}

def typeFor[A](property: Property[A]): String = {
val isMandatory = property.isMandatory
def typeFor[A](property: Property[A], nullable: Boolean = false): String = {
property.valueType match {
case ValueType.Boolean => if (isMandatory) "Boolean" else "java.lang.Boolean"
case ValueType.Boolean => if (nullable) "java.lang.Boolean" else "Boolean"
case ValueType.String => "String"
case ValueType.Byte => if (isMandatory) "Byte" else "java.lang.Byte"
case ValueType.Short => if (isMandatory) "Short" else "java.lang.Short"
case ValueType.Int => if (isMandatory) "scala.Int" else "Integer"
case ValueType.Long => if (isMandatory) "Long" else "java.lang.Long"
case ValueType.Float => if (isMandatory) "Float" else "java.lang.Float"
case ValueType.Double => if (isMandatory) "Double" else "java.lang.Double"
case ValueType.Char => if (isMandatory) "scala.Char" else "Character"
case ValueType.Byte => if (nullable) "java.lang.Byte" else "Byte"
case ValueType.Short => if (nullable) "java.lang.Short" else "Short"
case ValueType.Int => if (nullable) "Integer" else "scala.Int"
case ValueType.Long => if (nullable) "java.lang.Long" else "Long"
case ValueType.Float => if (nullable) "java.lang.Float" else "Float"
case ValueType.Double => if (nullable) "java.lang.Double" else "Double"
case ValueType.Char => if (nullable) "Character" else "scala.Char"
case ValueType.List => "Seq[_]"
case ValueType.NodeRef => "overflowdb.NodeRef[_]"
case ValueType.Unknown => "java.lang.Object"
Expand Down Expand Up @@ -120,8 +119,8 @@ object Helpers {
}
}

def getCompleteType[A](property: Property[?]): String =
getCompleteType(property.cardinality, typeFor(property))
def getCompleteType[A](property: Property[?], nullable: Boolean = true): String =
getCompleteType(property.cardinality, typeFor(property, nullable))

def typeFor(containedNode: ContainedNode): String = {
containedNode.nodeType match {
Expand Down Expand Up @@ -200,10 +199,10 @@ object Helpers {
}.mkString(s"$lineSeparator| ")
}

def propertyAccessors(properties: Seq[Property[?]]): String = {
def propertyAccessors(properties: Seq[Property[?]], nullable: Boolean = true): String = {
properties.map { property =>
val camelCaseName = camelCase(property.name)
val tpe = getCompleteType(property)
val tpe = getCompleteType(property, nullable = nullable)
s"def $camelCaseName: $tpe"
}.mkString(lineSeparator)
}
Expand Down
12 changes: 6 additions & 6 deletions integration-tests/tests/src/test/scala/Schema01Test.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ import testschema01.nodes._
import testschema01.edges._
import testschema01.traversal._
import scala.jdk.CollectionConverters.IteratorHasAsScala

class Schema01Test extends AnyWordSpec with Matchers {
import testschema01.traversal._
"constants" in {
PropertyNames.NAME shouldBe "NAME"
Properties.ORDER.name shouldBe "ORDER"
Properties.Order.name shouldBe "ORDER"
PropertyNames.ALL.contains("OPTIONS") shouldBe true
Properties.ALL.contains(Properties.OPTIONS) shouldBe true

NodeTypes.NODE1 shouldBe "NODE1"
NodeTypes.ALL.contains(Node2.Label) shouldBe true
Expand Down Expand Up @@ -42,8 +42,8 @@ class Schema01Test extends AnyWordSpec with Matchers {

"lookup and traverse nodes/edges/properties" in {
// generic traversal
graph.nodes.asScala.property(Properties.NAME).toSetMutable shouldBe Set("node 1a", "node 1b", "node 2a", "node 2b")
graph.edges.asScala.property(Properties.NAME).toSetMutable shouldBe Set("edge 2")
graph.nodes.asScala.property(Properties.Name).toSetMutable shouldBe Set("node 1a", "node 1b", "node 2a", "node 2b")
graph.edges.asScala.property(Properties.Name).toSetMutable shouldBe Set("edge 2")
node1Traversal.out.toList shouldBe Seq(node2a)
node1Traversal.name.toSetMutable shouldBe Set("node 1a", "node 1b")
node1Traversal.order.l shouldBe Seq(2)
Expand All @@ -58,7 +58,7 @@ class Schema01Test extends AnyWordSpec with Matchers {
val edge2Specific = edge2.asInstanceOf[Edge2]
val name: String = node1aSpecific.name
name shouldBe "node 1a"
val o1: Option[Integer] = node1aSpecific.order
val o1: Option[Int] = node1aSpecific.order
node1aSpecific.order shouldBe Some(2)
node1bSpecific.order shouldBe None
val o2: Seq[String] = node2aSpecific.options
Expand Down Expand Up @@ -97,7 +97,7 @@ class Schema01Test extends AnyWordSpec with Matchers {
.name("name1")
.node3(node3)
.options(Seq("one", "two", "three"))
.placements(Seq(1,2,3): Seq[Integer])
.placements(Seq(1,2,3))

val builder = new BatchedUpdate.DiffGraphBuilder
builder.addNode(newNode2)
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/tests/src/test/scala/Schema02Test.scala
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class Schema02Test extends AnyWordSpec with Matchers {
copy.order(3)
copy.order shouldBe Some(3)

copy.order(Some(4: Integer))
copy.order(Some(4))
copy.order shouldBe Some(4)

original.name shouldBe "A"
Expand Down
30 changes: 15 additions & 15 deletions integration-tests/tests/src/test/scala/Schema04Test.scala
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,18 @@ class Schema04Test extends AnyWordSpec with Matchers {
val node2 = graph.addNode(Node1.Label).asInstanceOf[Node1]
val edge1 = node1.addEdge(Edge1.Label, node2).asInstanceOf[Edge1]
val properties = Seq(
Properties.BOOL.of(false),
Properties.STR.of("foo"),
Properties.BYTE.of(100: Byte),
Properties.SHORT.of(101: Short),
Properties.INT.of(102),
Properties.LONG.of(103),
Properties.FLOAT1.of(Float.NaN),
Properties.FLOAT2.of(104.4f),
Properties.DOUBLE1.of(Double.NaN),
Properties.DOUBLE2.of(105.5),
Properties.CHAR.of('Z'),
Properties.INT_LIST.of(ArraySeq(3, 4, 5)),
Properties.Bool.of(false),
Properties.Str.of("foo"),
Properties.Byte.of(100: Byte),
Properties.Short.of(101: Short),
Properties.Int.of(102),
Properties.Long.of(103),
Properties.Float1.of(Float.NaN),
Properties.Float2.of(104.4f),
Properties.Double1.of(Double.NaN),
Properties.Double2.of(105.5),
Properties.Char.of('Z'),
Properties.IntList.of(ArraySeq(3, 4, 5)),
)
properties.foreach(node1.setProperty)
properties.foreach(edge1.setProperty)
Expand Down Expand Up @@ -164,8 +164,8 @@ class Schema04Test extends AnyWordSpec with Matchers {
val node1 = graph.addNode(Node1.Label).asInstanceOf[Node1]
val node2 = graph.addNode(Node1.Label).asInstanceOf[Node1]
val edge1 = node1.addEdge(Edge1.Label, node2).asInstanceOf[Edge1]
node1.setProperty(Properties.INT_LIST.name, Array(1,2,3))
edge1.setProperty(Properties.INT_LIST.name, Array(3,4,5))
node1.setProperty(Properties.IntList.name, Array(1,2,3))
edge1.setProperty(Properties.IntList.name, Array(3,4,5))

node1.intList shouldBe IndexedSeq(1,2,3)
edge1.intList shouldBe IndexedSeq(3,4,5)
Expand All @@ -175,7 +175,7 @@ class Schema04Test extends AnyWordSpec with Matchers {
val node1 = NewNode1()
.bool(true)
.str("foo")
.intList(Seq(1,2,3): Seq[Integer])
.intList(Seq(1,2,3))

val node2 = NewNode1().node1Inner(node1)

Expand Down
18 changes: 9 additions & 9 deletions integration-tests/tests/src/test/scala/Schema05Test.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@ class Schema05Test extends AnyWordSpec with Matchers {
val node2 = graph.addNode(Node1.Label).asInstanceOf[Node1]
val edge1 = node1.addEdge(Edge1.Label, node2).asInstanceOf[Edge1]
val properties = Seq(
Properties.BOOL.of(false),
Properties.STR.of("foo"),
Properties.BYTE.of(100: Byte),
Properties.SHORT.of(101: Short),
Properties.INT.of(102),
Properties.LONG.of(103),
Properties.FLOAT.of(104.4f),
Properties.DOUBLE.of(105.5),
Properties.CHAR.of('Z'),
Properties.Bool.of(false),
Properties.Str.of("foo"),
Properties.Byte.of(100: Byte),
Properties.Short.of(101: Short),
Properties.Int.of(102),
Properties.Long.of(103),
Properties.Float.of(104.4f),
Properties.Double.of(105.5),
Properties.Char.of('Z'),
)
properties.foreach(node1.setProperty)
properties.foreach(edge1.setProperty)
Expand Down

0 comments on commit 5042694

Please sign in to comment.