only replace '_id' at end of columnName

[rharink]

I've encountered a problem with foreign-key column-names that have a '_id' (e.g. _identity) in their name.
These columns don't get renamed properly (partner_identity_card -> partnerentityCard).

The fix could be a preg_replace instead of a string_replace that only replaces the _id at the end of a column name.

[rharink]

Could someone point me in the right direction for creating a test-case? I would need a (mock) driver that supports foreign-keys

[Ocramius]

@rharink check https://github.com/doctrine/doctrine2/blob/master/tests/Doctrine/Tests/ORM/Functional/Ticket for examples of test cases.

I'd suggest a scenario that runs on SQLite only and:

1. creates a table
2. instantiates the DatabaseDriver
3. verifies the schema information coming from DatabaseDriver

[rharink]

@Ocramius Thanks for the reply,

The SqlLite driver doesn't support foreign keys. The DatabaseDriver class will not resolve foreign keys, thus the method getFieldNameForColumn is never called in the testcase.

returns false for Sqllite

$this->_em->getConnection()->getDatabasePlatform()->supportsForeignKeyConstraints())

[Ocramius]

True, would need to design this with MySQL or equivalent

[rharink]

Should I use a DbalFunctionalTestCase or is there something else I can use to setup a MySQL test?
Thanks for the help!

[rharink]

Sorry should read more before asking ;) I found documentation on how to run tests on mysql.

[rharink]

@Ocramius Checks have passed, should / can I do anything else?

[Ocramius]

Overall looking good: needs adjustments in test naming and placing (need to check if we can turn this into a full unit tests, eventually - not sure if that is feasible)

[rharink]

@Ocramius can I do something else? maybe add a unit test in the databaseDriverTest.php? Just let me know

[Ocramius]

LGTM 👍

@rharink can you rebase on top of upstream/2.6 so that we can see if the CI failures are related to this patch at all?

Besides that, just needs a merge from our end.

[rharink]

Ok, I've merged upstream.

[greg0ire]

I rebased, I fixed the build in #7731, this means the tests might now pass.

[ostrolucky]

Test case is unfortunately not correct. It passes for previous behaviour too.

[greg0ire]

Wow good catch! Well at least the build is green now 😅

@rharink please look into this 🙏

[rharink]

Good one! Ill look in to it.

[lcobucci]

@ostrolucky nice catch indeed, thanks!

@rharink since we're aiming at releasing the last patch of the 2.6.x series and release 2.7.0 I'm moving this to 2.7.1 so we don't put a lot of pressure on your shoulders 😄

Thanks for putting effort into this and I hope we can solve it together in the next patch 👍

[rharink]

I've improved the unit-test. it didn't pass for previous behaviour as the key gh7684IdentityTest in the associationMapping would not have been present. Or am i interpreting 'previous behaviour' wrong 😅

[ostrolucky]

Test is correct now. @lcobucci I would say we can merge this now to 2.6.5 what do you say?

[greg0ire]

Please kindly squash your commits together. If you don't, we'll try to remember to do it for you but it's best if you save us this trouble.

How to do that?

1. git rebase -i origin/2.6, assuming origin is a git remote that points to this repository, and not your fork. If you're not sure what your remotes are, run git remote -vvv, there should be your fork and the holy/reference/base/origin/whatever-you-call-it repository.
2. A window will show up with many lines, replace pick with fixup on every line but the first one
3. Close your editor, git should do its magic, and you should end up with one commit
4. Use git push --force to overwrite what you already push. Don't forget the --force option otherwise git will try to merge both things together.

[lcobucci]

@rharink @ostrolucky awesome, thanks for being that quick. Please follow @greg0ire's instructions and let's ship this!

[rharink]

@lcobucci @ostrolucky done, lets go!

[lcobucci]

@rharink 🚢