From ae1cb3fe6e87bc80e114fe36c55609493657c35b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20N=C3=BCcke?= Date: Sat, 16 Apr 2016 14:21:45 +0200 Subject: [PATCH] Fixed error in network node remapping when node gets removed during that remapping operation. E.g. file systems automatically connected together with the main component, as in the internet card. This should fix a potential crash with OC conduits in EnderIO. Also, unit tests \o/ You'll need ScalaTest and Mockito to run them. --- .../li/cil/oc/server/network/Network.scala | 23 +- src/test/scala/NetworkTest.scala | 235 ++++++++++++++++++ 2 files changed, 245 insertions(+), 13 deletions(-) create mode 100644 src/test/scala/NetworkTest.scala diff --git a/src/main/scala/li/cil/oc/server/network/Network.scala b/src/main/scala/li/cil/oc/server/network/Network.scala index 512f53336..7674a4bf4 100644 --- a/src/main/scala/li/cil/oc/server/network/Network.scala +++ b/src/main/scala/li/cil/oc/server/network/Network.scala @@ -272,12 +272,8 @@ private class Network private(private val data: mutable.Map[String, Network.Vert node.address = java.util.UUID.randomUUID().toString } while (data.contains(node.address) || otherNetwork.data.contains(node.address)) - if (neighbors.isEmpty) { - assert(otherNetwork.data.size == 1) - Network.joinNewNetwork(node) - } else { - neighbors.foreach(_.connect(node)) - } + Network.joinNewNetwork(node) + neighbors.filter(_.network != null).foreach(_.connect(node)) }) duplicates.head.data.network.asInstanceOf[Network.Wrapper].network @@ -589,6 +585,8 @@ object Network extends api.detail.NetworkAPI { new Packet(source, destination, port, data, ttl) } + var isServer = SideTracker.isServer _ + class NodeBuilder(val _host: Environment, val _reachability: Visibility) extends api.detail.Builder.NodeBuilder { def withComponent(name: String, visibility: Visibility) = new Network.ComponentBuilder(_host, _reachability, name, visibility) @@ -598,7 +596,7 @@ object Network extends api.detail.NetworkAPI { def withConnector() = withConnector(0) - def create() = if (SideTracker.isServer) new MutableNode with NodeVarargPart { + def create() = if (isServer()) new MutableNode with NodeVarargPart { val host = _host val reachability = _reachability } @@ -610,7 +608,7 @@ object Network extends api.detail.NetworkAPI { def withConnector() = withConnector(0) - def create() = if (SideTracker.isServer) new Component with NodeVarargPart { + def create() = if (isServer()) new Component with NodeVarargPart { val host = _host val reachability = _reachability val name = _name @@ -624,7 +622,7 @@ object Network extends api.detail.NetworkAPI { def withComponent(name: String) = withComponent(name, _reachability) - def create() = if (SideTracker.isServer) new Connector with NodeVarargPart { + def create() = if (isServer()) new Connector with NodeVarargPart { val host = _host val reachability = _reachability localBufferSize = _bufferSize @@ -633,7 +631,7 @@ object Network extends api.detail.NetworkAPI { } class ComponentConnectorBuilder(val _host: Environment, val _reachability: Visibility, val _name: String, val _visibility: Visibility, val _bufferSize: Double) extends api.detail.Builder.ComponentConnectorBuilder { - def create() = if (SideTracker.isServer) new ComponentConnector with NodeVarargPart { + def create() = if (isServer()) new ComponentConnector with NodeVarargPart { val host = _host val reachability = _reachability val name = _name @@ -729,9 +727,8 @@ object Network extends api.detail.NetworkAPI { } nbt.setInteger("port", port) nbt.setInteger("ttl", ttl) - val dataArray = data.toArray - nbt.setInteger("dataLength", dataArray.length) - for (i <- dataArray.indices) dataArray(i) match { + nbt.setInteger("dataLength", data.length) + for (i <- data.indices) data(i) match { case null | Unit | None => case value: java.lang.Boolean => nbt.setBoolean("data" + i, value) case value: java.lang.Integer => nbt.setInteger("data" + i, value) diff --git a/src/test/scala/NetworkTest.scala b/src/test/scala/NetworkTest.scala new file mode 100644 index 000000000..73d6839e1 --- /dev/null +++ b/src/test/scala/NetworkTest.scala @@ -0,0 +1,235 @@ +import li.cil.oc.api +import li.cil.oc.api.network.Environment +import li.cil.oc.api.network.Message +import li.cil.oc.api.network.Node +import li.cil.oc.api.network.Visibility +import li.cil.oc.server.network.Network +import li.cil.oc.server.network.{Node => MutableNode} +import org.scalatest._ +import org.scalatest.mock.MockitoSugar + +import scala.collection.convert.WrapAsScala._ + +class NetworkTest extends FlatSpec with MockitoSugar { + Network.isServer = () => true + api.API.network = Network + + val host = mock[Environment] + + "A Node" should "not belong to a network after creation" in { + val node = api.Network.newNode(host, Visibility.Network).create() + assert(node.network == null) + } + + it should "belong to a network after joining a new network" in { + val node = api.Network.newNode(host, Visibility.Network).create() + api.Network.joinNewNetwork(node) + assert(node.network != null) + } + + it should "not belong to a network after being removed from its new network" in { + val node = api.Network.newNode(host, Visibility.Network).create() + api.Network.joinNewNetwork(node) + node.remove() + assert(node.network == null) + } + + it should "have a neighbor after being connected to another node" in { + val node1 = api.Network.newNode(host, Visibility.Network).create() + api.Network.joinNewNetwork(node1) + val node2 = api.Network.newNode(host, Visibility.Network).create() + node1.connect(node2) + assert(node1.neighbors.nonEmpty) + assert(node2.neighbors.nonEmpty) + assert(node1.isNeighborOf(node2)) + assert(node2.isNeighborOf(node1)) + } + + it should "be reachable by neighbors when visibility is set to Neighbors" in { + val node1 = api.Network.newNode(host, Visibility.Neighbors).create() + api.Network.joinNewNetwork(node1) + val node2 = api.Network.newNode(host, Visibility.Network).create() + node1.connect(node2) + assert(node1.canBeReachedFrom(node2)) + } + + it should "be in the same network as nodes it is connected to" in { + val node1 = api.Network.newNode(host, Visibility.Network).create() + api.Network.joinNewNetwork(node1) + val node2 = api.Network.newNode(host, Visibility.Network).create() + node1.connect(node2) + val node3 = api.Network.newNode(host, Visibility.Network).create() + node2.connect(node3) + + assert(node1.network == node2.network) + assert(node2.network == node3.network) + assert(node1.network == node3.network) + } + + it should "have a different address than nodes it is connected to" in { + val node1 = api.Network.newNode(host, Visibility.Network).create() + api.Network.joinNewNetwork(node1) + val node2 = api.Network.newNode(host, Visibility.Network).create() + node1.connect(node2) + val node3 = api.Network.newNode(host, Visibility.Network).create() + node2.connect(node3) + + assert(node1.address != node2.address) + assert(node2.address != node3.address) + assert(node1.address != node3.address) + } + + it should "not be reachable by non neighbors when visibility is set to Neighbors" in { + val node1 = api.Network.newNode(host, Visibility.Neighbors).create() + api.Network.joinNewNetwork(node1) + val node2 = api.Network.newNode(host, Visibility.Network).create() + node1.connect(node2) + val node3 = api.Network.newNode(host, Visibility.Network).create() + node2.connect(node3) + assert(!node1.canBeReachedFrom(node3)) + } + + it should "be reachable by all nodes when visibility is set to Network" in { + val node1 = api.Network.newNode(host, Visibility.Network).create() + api.Network.joinNewNetwork(node1) + val node2 = api.Network.newNode(host, Visibility.Network).create() + node1.connect(node2) + val node3 = api.Network.newNode(host, Visibility.Network).create() + node2.connect(node3) + assert(node1.canBeReachedFrom(node2)) + assert(node1.canBeReachedFrom(node3)) + } + + it should "not be reachable by any node when visibility is set to None" in { + val node1 = api.Network.newNode(host, Visibility.None).create() + api.Network.joinNewNetwork(node1) + val node2 = api.Network.newNode(host, Visibility.Network).create() + node1.connect(node2) + val node3 = api.Network.newNode(host, Visibility.Network).create() + node2.connect(node3) + assert(!node1.canBeReachedFrom(node2)) + assert(!node1.canBeReachedFrom(node3)) + } + + it should "be in a separate network after a netsplit" in { + val node1 = api.Network.newNode(host, Visibility.Network).create() + api.Network.joinNewNetwork(node1) + val node2 = api.Network.newNode(host, Visibility.Network).create() + node1.connect(node2) + val node3 = api.Network.newNode(host, Visibility.Network).create() + node2.connect(node3) + + node2.remove() + + assert(node1.network != null) + assert(node2.network == null) + assert(node3.network != null) + assert(node1.network != node3.network) + } + + it should "change its address when joining a network containing a node with its address" in { + val node1 = api.Network.newNode(host, Visibility.Network).create() + api.Network.joinNewNetwork(node1) + val node2 = api.Network.newNode(host, Visibility.Network).create() + node2.asInstanceOf[MutableNode].address = node1.address + node1.connect(node2) + assert(node1.address != node2.address) + } + + "A Network" should "keep its local layout after being merged with another network" in { + val node1 = api.Network.newNode(host, Visibility.Network).create() + api.Network.joinNewNetwork(node1) + val node2 = api.Network.newNode(host, Visibility.Network).create() + node1.connect(node2) + val node3 = api.Network.newNode(host, Visibility.Network).create() + node2.connect(node3) + + val node4 = api.Network.newNode(host, Visibility.Network).create() + api.Network.joinNewNetwork(node4) + val node5 = api.Network.newNode(host, Visibility.Network).create() + node4.connect(node5) + val node6 = api.Network.newNode(host, Visibility.Network).create() + node5.connect(node6) + + node2.connect(node5) + + assert(node1.neighbors.size == 1 && node1.isNeighborOf(node2)) + assert(node3.neighbors.size == 1 && node3.isNeighborOf(node2)) + + assert(node4.neighbors.size == 1 && node4.isNeighborOf(node5)) + assert(node6.neighbors.size == 1 && node6.isNeighborOf(node5)) + + assert(node2.isNeighborOf(node5)) + } + + it should "keep its local layout after being merged with another network containing nodes with duplicate addresses at bridge points" in { + val node1 = api.Network.newNode(host, Visibility.Network).create() + api.Network.joinNewNetwork(node1) + val node2 = api.Network.newNode(host, Visibility.Network).create() + node1.connect(node2) + val node3 = api.Network.newNode(host, Visibility.Network).create() + node1.connect(node3) + val node4 = api.Network.newNode(host, Visibility.Network).create() + node3.connect(node4) + + val node5 = api.Network.newNode(host, Visibility.Network).create() + node5.asInstanceOf[MutableNode].address = node1.address + api.Network.joinNewNetwork(node5) + val node6 = api.Network.newNode(host, Visibility.Network).create() + node6.asInstanceOf[MutableNode].address = node2.address + node5.connect(node6) + val node7 = api.Network.newNode(host, Visibility.Network).create() + node7.asInstanceOf[MutableNode].address = node3.address + node5.connect(node7) + val node8 = api.Network.newNode(host, Visibility.Network).create() + node8.asInstanceOf[MutableNode].address = node4.address + node7.connect(node8) + + node3.connect(node7) + + assert(node1.neighbors.size == 2 && node1.isNeighborOf(node2) && node1.isNeighborOf(node3)) + assert(node2.neighbors.size == 1 && node2.isNeighborOf(node1)) + assert(node3.neighbors.size == 3 && node3.isNeighborOf(node1) && node3.isNeighborOf(node4) && node3.isNeighborOf(node7)) + assert(node4.neighbors.size == 1 && node4.isNeighborOf(node3)) + + assert(node5.neighbors.size == 2 && node5.isNeighborOf(node6) && node5.isNeighborOf(node7)) + assert(node6.neighbors.size == 1 && node6.isNeighborOf(node5)) + assert(node7.neighbors.size == 3 && node7.isNeighborOf(node5) && node7.isNeighborOf(node8) && node7.isNeighborOf(node3)) + assert(node8.neighbors.size == 1 && node8.isNeighborOf(node7)) + } + + it should "not error when nodes disconnect themselves in a remapping operation" in { + val host = new Environment { + val node1 = api.Network.newNode(this, Visibility.Network).create() + val node2 = api.Network.newNode(this, Visibility.Network).create() + + api.Network.joinNewNetwork(node1) + + override def node: Node = node1 + + override def onMessage(message: Message): Unit = {} + + override def onConnect(node: Node): Unit = { + if (node == node1) { + node.connect(node2) + } + } + + override def onDisconnect(node: Node): Unit = { + if (node == node1) { + node2.remove() + } + } + } + + val node3 = api.Network.newNode(host, Visibility.Network).create() + node3.asInstanceOf[MutableNode].address = host.node.address + api.Network.joinNewNetwork(node3) + + node3.connect(host.node) + + assert(host.node1.neighbors.size == 2 && host.node1.isNeighborOf(host.node2) && host.node1.isNeighborOf(node3)) + assert(host.node2.neighbors.size == 1 && host.node2.isNeighborOf(host.node1)) + assert(node3.neighbors.size == 1 && node3.isNeighborOf(host.node1)) + } +}