zmactep / hasbolt

Haskell driver for Neo4j 3+ (BOLT protocol)
BSD 3-Clause "New" or "Revised" License
82 stars 13 forks source link

[Feature] - Transactions support #15

Closed gvolpe closed 5 years ago

gvolpe commented 5 years ago

closes #14

Hi @zmactep, try it out and let me know what you think. Before merging I should:

Here's the output of the Spec (including the debugging lines):

Transaction
Sending command "BEGIN"
Sending Cypher "CREATE (p:Person { name: 'Simon', age: 61 } ) RETURN p"
Sending Cypher "CREATE (p:Person { name: 'Philip', age: 63 } ) RETURN p"
Sending Cypher "CREATE (p:Person { name: 'Erik', age: 56 } ) RETURN p"
Sending command "COMMIT"
  commits
Sending command "BEGIN"
Sending Cypher "CREATE (g:Guitar { brand: 'Gibson' } ) RETURN g"
Sending Cypher "CREATE (g:Guitar { brand: 'Fender' } ) RETURN g"
Sending Cypher "CREATE (g:Guitar { brand: 'Ibanez' } ) RETURN g"
Sending Cypher "This is going to make it fail, BOOM!"
user error (code: "Neo.ClientError.Statement.SyntaxError", message: "Invalid input 'T': expected <init> (line 1, column 1 (offset: 0))\n\"This is going to make it fail, BOOM!\"\n ^")
Starting ROLLBACK
Sending command "ROLLBACK"
  rollsback due to a malformed query

In another PR I would like to setup the CI Build to have a running instance of Neo4j so we can test cypher queries against it, including the transactions. What do you think?

gvolpe commented 5 years ago

Just seeing that the CI build is using Stack with an old GHC version. I think that I'd be better to build with Cabal or otherwise we will need to add the different stack.yaml files for the different LTS. What do you think @zmactep?

gvolpe commented 5 years ago

I fixed it for now but I think we could improve the CI build for different GHC versions in a future PR :)

zmactep commented 5 years ago

One general thought. Maybe it would be better to organize the transaction as a number of BoltActionTs? Not as a number of Cyphers? We wouldn't break any backward compability. I think it could be not so hard to do.

gvolpe commented 5 years ago

Actually that's an excellent idea @zmactep , I'm on it!

gvolpe commented 5 years ago

@zmactep the CI build needs to be re-triggered.

zmactep commented 5 years ago

Hi!

I've rewriten your PR with much simplier code without direct Pipe manipulations and so on. You can find it here: #16. I like the new API more too:

query1 :: MonadIO m => BoltActionT m [Record]
query1 = query "CREATE (g:Guitar { brand: 'Gibson' } ) RETURN g"

query2 :: MonadIO m => BoltActionT m ()
query2 = query_ "CREATE (g:Guitar { brand: 'Fender' } ) RETURN g"

query3 :: MonadIO m => BoltActionT m Int
query3 = length <$> query "CREATE (g:Guitar { brand: 'Ibanez' } ) RETURN g"

query4 :: MonadIO m => BoltActionT m Record
query4 = head <$> query "This is going to make it fail, BOOM!"

query5 :: MonadIO m => BoltActionT m [Record]
query5 = query "CREATE (g:Guitar { brand: 'Schecter' } ) RETURN g"

-- We can combine different types of queries or even perform some actions inside
transaction :: MonadIO m => BoltActionT m [Record]
transaction = transact $ do
    records <- query1
    query2
    number <- query3
    forM_ [0..number] $ \_ ->
      query4
    query5

So, if you are ok with it, i'll merge it.