Throw OptimisticLockException when connection::commit() returns false

[chosroes]

Proposal for #7893 in order to handle silently failed commits

[SenseException]

Thank you for your PR. Please add unit tests that are covering your changes.

[chosroes]

Thank you for your PR. Please add unit tests that are covering your changes.

I'd like to but there are currently no tests for exception in commit process, or I should create another connection mock with commit that return false ?

[chosroes]

Tests added and checks passed

[SenseException]

Thank you for the changes. You introduced changes in .scrutinizer.yml, the travis file and composer.json. Are these changes relevant to your feature?

[chosroes]

Yes since doctrine/dbal now needs at least php 7.2

[ostrolucky]

Lot of tests fail for me locally. Not sure why travis did not start here, but tests must run

[chosroes]

@ostrolucky I think there is a problem with travis. The request is in status "Abuse detected"...
I had no account in travis CI. I created one just now...
https://travis-ci.org/doctrine/orm/requests?utm_medium=notification&utm_source=github_status

By the way, I changed my unit test. There was already lot of tests failures in the 2.8.x branch for me locally. So I don't know what to do with that.

[greg0ire]

Apparently the issue was transient, here is your build!

[ostrolucky]

Aand it shows failures I mentioned. I guess UPGRADE.md should also be upgraded, since this is minor BC break.

[chosroes]

I can't get travis to work with me...
(It works after contacting travais support)

[SenseException]

Is this still considered a WIP?

[chosroes]

@SenseException Not for me

[chosroes]

Any update ?

[SenseException]

I'm undecided for the usage of $conn->commit() === false. It introduces the usage of an already existing boolean return value in that method. It is true that 3.x is still using it, but master has changed it to void. Maybe I'm planning to far into the future, because we can deprecate it in the last release of 3.x or respectively implement that optimistic lock case differently in master. //cc @ostrolucky

Otherwise I'm fine with the changes.

[chosroes]

@SenseException I don't know for master, the current commit without error may have be solve. But meanwhile, we have to do with the current interface that can return false

[ostrolucky]

master has changed it to void

Good point. Fortunately that won't break this code, just make bool condition dead. I expect phpstan will alert us on it in future so we can remove it then.

[SenseException]

If postponing the issue with bool -> void is acceptable for this PR, then I'll give my approval.