Merge remote-tracking branch 'open/master' into anthony-os-merge-20180406

# Conflicts:
#	testing/node-driver/src/main/kotlin/net/corda/testing/node/MockServices.kt

node/src/main/kotlin/net/corda/node/services/api/ServiceHubInternal.kt

@@ -58,6 +58,61 @@ interface NetworkMapCacheBaseInternal : NetworkMapCacheBase {
 interface ServiceHubInternal : ServiceHub {
     companion object {
         private val log = contextLogger()
+
+        fun recordTransactions(statesToRecord: StatesToRecord, txs: Iterable<SignedTransaction>,
+                               validatedTransactions: WritableTransactionStorage,
+                               stateMachineRecordedTransactionMapping: StateMachineRecordedTransactionMappingStorage,
+                               vaultService: VaultServiceInternal) {
+
+            require(txs.any()) { "No transactions passed in for recording" }
+            val recordedTransactions = txs.filter { validatedTransactions.addTransaction(it) }
+            val stateMachineRunId = FlowStateMachineImpl.currentStateMachine()?.id
+            if (stateMachineRunId != null) {
+                recordedTransactions.forEach {
+                    stateMachineRecordedTransactionMapping.addMapping(stateMachineRunId, it.id)
+                }
+            } else {
+                log.warn("Transactions recorded from outside of a state machine")
+            }
+
+            if (statesToRecord != StatesToRecord.NONE) {
+                // When the user has requested StatesToRecord.ALL we may end up recording and relationally mapping states
+                // that do not involve us and that we cannot sign for. This will break coin selection and thus a warning
+                // is present in the documentation for this feature (see the "Observer nodes" tutorial on docs.corda.net).
+                //
+                // The reason for this is three-fold:
+                //
+                // 1) We are putting in place the observer mode feature relatively quickly to meet specific customer
+                //    launch target dates.
+                //
+                // 2) The right design for vaults which mix observations and relevant states isn't entirely clear yet.
+                //
+                // 3) If we get the design wrong it could create security problems and business confusions.
+                //
+                // Back in the bitcoinj days I did add support for "watching addresses" to the wallet code, which is the
+                // Bitcoin equivalent of observer nodes:
+                //
+                //   https://bitcoinj.github.io/working-with-the-wallet#watching-wallets
+                //
+                // The ability to have a wallet containing both irrelevant and relevant states complicated everything quite
+                // dramatically, even methods as basic as the getBalance() API which required additional modes to let you
+                // query "balance I can spend" vs "balance I am observing". In the end it might have been better to just
+                // require the user to create an entirely separate wallet for observing with.
+                //
+                // In Corda we don't support a single node having multiple vaults (at the time of writing), and it's not
+                // clear that's the right way to go: perhaps adding an "origin" column to the VAULT_STATES table is a better
+                // solution. Then you could select subsets of states depending on where the report came from.
+                //
+                // The risk of doing this is that apps/developers may use 'canned SQL queries' not written by us that forget
+                // to add a WHERE clause for the origin column. Those queries will seem to work most of the time until
+                // they're run on an observer node and mix in irrelevant data. In the worst case this may result in
+                // erroneous data being reported to the user, which could cause security problems.
+                //
+                // Because the primary use case for recording irrelevant states is observer/regulator nodes, who are unlikely
+                // to make writes to the ledger very often or at all, we choose to punt this issue for the time being.
+                vaultService.notifyAll(statesToRecord, recordedTransactions.map { it.coreTransaction })
+            }
+        }
     }
 
     override val vaultService: VaultServiceInternal
@@ -80,54 +135,7 @@ interface ServiceHubInternal : ServiceHub {
     val networkMapUpdater: NetworkMapUpdater
     override val cordappProvider: CordappProviderInternal
     override fun recordTransactions(statesToRecord: StatesToRecord, txs: Iterable<SignedTransaction>) {
-        require(txs.any()) { "No transactions passed in for recording" }
+        recordTransactions(statesToRecord, txs, validatedTransactions, stateMachineRecordedTransactionMapping, vaultService)
-        val recordedTransactions = txs.filter { validatedTransactions.addTransaction(it) }
-        val stateMachineRunId = FlowStateMachineImpl.currentStateMachine()?.id
-        if (stateMachineRunId != null) {
-            recordedTransactions.forEach {
-                stateMachineRecordedTransactionMapping.addMapping(stateMachineRunId, it.id)
-            }
-        } else {
-            log.warn("Transactions recorded from outside of a state machine")
-        }
-
-        if (statesToRecord != StatesToRecord.NONE) {
-            // When the user has requested StatesToRecord.ALL we may end up recording and relationally mapping states
-            // that do not involve us and that we cannot sign for. This will break coin selection and thus a warning
-            // is present in the documentation for this feature (see the "Observer nodes" tutorial on docs.corda.net).
-            //
-            // The reason for this is three-fold:
-            //
-            // 1) We are putting in place the observer mode feature relatively quickly to meet specific customer
-            //    launch target dates.
-            //
-            // 2) The right design for vaults which mix observations and relevant states isn't entirely clear yet.
-            //
-            // 3) If we get the design wrong it could create security problems and business confusions.
-            //
-            // Back in the bitcoinj days I did add support for "watching addresses" to the wallet code, which is the
-            // Bitcoin equivalent of observer nodes:
-            //
-            //   https://bitcoinj.github.io/working-with-the-wallet#watching-wallets
-            //
-            // The ability to have a wallet containing both irrelevant and relevant states complicated everything quite
-            // dramatically, even methods as basic as the getBalance() API which required additional modes to let you
-            // query "balance I can spend" vs "balance I am observing". In the end it might have been better to just
-            // require the user to create an entirely separate wallet for observing with.
-            //
-            // In Corda we don't support a single node having multiple vaults (at the time of writing), and it's not
-            // clear that's the right way to go: perhaps adding an "origin" column to the VAULT_STATES table is a better
-            // solution. Then you could select subsets of states depending on where the report came from.
-            //
-            // The risk of doing this is that apps/developers may use 'canned SQL queries' not written by us that forget
-            // to add a WHERE clause for the origin column. Those queries will seem to work most of the time until
-            // they're run on an observer node and mix in irrelevant data. In the worst case this may result in
-            // erroneous data being reported to the user, which could cause security problems.
-            //
-            // Because the primary use case for recording irrelevant states is observer/regulator nodes, who are unlikely
-            // to make writes to the ledger very often or at all, we choose to punt this issue for the time being.
-            vaultService.notifyAll(statesToRecord, txs.map { it.coreTransaction })
-        }
     }
 
     fun getFlowFactory(initiatingFlowClass: Class<out FlowLogic<*>>): InitiatedFlowFactory<*>?

node/src/test/kotlin/net/corda/node/services/vault/NodeVaultServiceTest.kt

@@ -140,6 +140,18 @@ class NodeVaultServiceTest {
         return tryLockFungibleStatesForSpending(lockId, baseCriteria, amount, Cash.State::class.java)
     }
 
+    @Test
+    fun `duplicate insert of transaction does not fail`() {
+        database.transaction {
+            val cash = Cash()
+            val howMuch = 100.DOLLARS
+            val issuance = TransactionBuilder(null as Party?)
+            cash.generateIssue(issuance, Amount(howMuch.quantity, Issued(DUMMY_CASH_ISSUER, howMuch.token)), services.myInfo.singleIdentity(), dummyNotary.party)
+            val transaction = issuerServices.signInitialTransaction(issuance, DUMMY_CASH_ISSUER.party.owningKey)
+            services.recordTransactions(transaction)
+            services.recordTransactions(transaction)
+        }
+    }
 
     @Test
     fun `states not local to instance`() {

testing/node-driver/src/main/kotlin/net/corda/testing/node/MockServices.kt

@@ -19,10 +19,13 @@ import net.corda.core.contracts.StateRef
 import net.corda.core.cordapp.CordappProvider
 import net.corda.core.crypto.SecureHash
 import net.corda.core.flows.FlowLogic
+import net.corda.core.flows.StateMachineRunId
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.identity.PartyAndCertificate
+import net.corda.core.messaging.DataFeed
 import net.corda.core.messaging.FlowHandle
 import net.corda.core.messaging.FlowProgressHandle
+import net.corda.core.messaging.StateMachineTransactionMapping
 import net.corda.core.node.*
 import net.corda.core.node.services.*
 import net.corda.core.serialization.SerializeAsToken
@@ -37,6 +40,7 @@ import net.corda.node.services.api.WritableTransactionStorage
 import net.corda.node.services.config.ConfigHelper
 import net.corda.node.services.config.configOf
 import net.corda.node.services.config.parseToDbSchemaFriendlyName
+import net.corda.node.services.api.*
 import net.corda.node.services.identity.InMemoryIdentityService
 import net.corda.node.services.schema.HibernateObserver
 import net.corda.node.services.schema.NodeSchemaService
@@ -148,9 +152,10 @@ open class MockServices private constructor(
                     override val vaultService: VaultService = makeVaultService(database.hibernateConfig, schemaService)
 
                     override fun recordTransactions(statesToRecord: StatesToRecord, txs: Iterable<SignedTransaction>) {
-                        super.recordTransactions(statesToRecord, txs)
+                        ServiceHubInternal.recordTransactions(statesToRecord, txs,
-                        // Refactored to use notifyAll() as we have no other unit test for that method with multiple transactions.
+                                validatedTransactions as WritableTransactionStorage,
-                        (vaultService as VaultServiceInternal).notifyAll(statesToRecord, txs.map { it.coreTransaction })
+                                mockStateMachineRecordedTransactionMappingStorage,
+                                vaultService as VaultServiceInternal)
                     }
 
                     override fun jdbcSession(): Connection = database.createSession()
@@ -166,6 +171,19 @@ open class MockServices private constructor(
             // compiler and then the c'tor itself.
             return Throwable().stackTrace[3].className.split('.').dropLast(1).joinToString(".")
         }
+
+        // Because Kotlin is dumb and makes not publicly visible objects public, thus changing the public API.
+        private val mockStateMachineRecordedTransactionMappingStorage = MockStateMachineRecordedTransactionMappingStorage()
+    }
+
+    private class MockStateMachineRecordedTransactionMappingStorage : StateMachineRecordedTransactionMappingStorage {
+        override fun addMapping(stateMachineRunId: StateMachineRunId, transactionId: SecureHash) {
+            throw UnsupportedOperationException()
+        }
+
+        override fun track(): DataFeed<List<StateMachineTransactionMapping>, StateMachineTransactionMapping> {
+            throw UnsupportedOperationException()
+        }
     }
 
     private constructor(cordappLoader: CordappLoader, identityService: IdentityService, networkParameters: NetworkParameters,