Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for doctrine/orm 3.0 #43

Closed
captain-igloo opened this issue Feb 13, 2024 · 21 comments
Closed

Add support for doctrine/orm 3.0 #43

captain-igloo opened this issue Feb 13, 2024 · 21 comments
Assignees
Labels

Comments

@captain-igloo
Copy link

doctrine/orm 3.0 was released a few days ago. Would it be possible to add support for this?

Thanks

@Alexandre-T
Copy link
Contributor

I am currently forking and updating libraries (geo-parser and wkt-parser) required to migrate to doctrine/lexer:3. The goal is to make doctrine-spatial compatible with doctrine/orm:3.

@lukena-xxl
Copy link

Hello. When can we expect support for "doctrine/orm 3.1"? Thank you

@Alexandre-T
Copy link
Contributor

Alexandre-T commented Mar 13, 2024

I don't know. I am a volunteer, doing this in my spare time, and I haven't yet studied the impact between doctrine:orm 2.0 and doctrine:orm 3.0, let alone Doctrine 3.1.

@Alexandre-T
Copy link
Contributor

longitude/geo-parser is done.
Todo :

  • longitude-one/wkt-parser
  • doctrine/orm:3

@Alexandre-T Alexandre-T added dependencies enhancement New feature or request labels Mar 31, 2024
@Alexandre-T Alexandre-T self-assigned this Mar 31, 2024
@Alexandre-T
Copy link
Contributor

Alexandre-T commented Mar 31, 2024

  • longitude-one/geo-parser is upgraded. A pre-release 3.0.0.RC is published, it's compatible with doctrine/lexer:3.0
  • longitude-one/wkt-parser is upgraded. A pre-release 3.0.0.RC is published, it's compatible with doctrine/lexer:3.0 (and experimental 3.1)

I can now focus on longitude-one/doctrine-spatial

Todo :

  • Fork creof/wkb-parser
  • Upgrade longitude-one/wkb-parser
  • Fix deprecations on longitude-one/doctrine-spatial with the last versions doctrine/orm:2.* (2.19.3 today)
  • Upgrade longitude-one/doctrine-spatial to be compliant with the last versions of doctrine/orm:3.1.* (3.1.1 today)
  • Have a look on doctrine/orm:3.2
  • Have a look on doctrine/orm:4.0

@remy-theroux
Copy link

thanks for your time

@danielgwood
Copy link

Thanks for your time. If you have a Ko-fi or similar I'd love to make a donation, it's a helpful package.

@Alexandre-T
Copy link
Contributor

Alexandre-T commented Apr 10, 2024

Thanks for your time. If you have a Ko-fi or similar I'd love to make a donation, it's a helpful package.

Thank you very much @danielgwood : ko-fi.com/alexandret

  • longitude-one/geo-parser is upgraded. A pre-release 3.0.0.RC is published, it's compatible with doctrine/lexer:3.0
  • longitude-one/wkt-parser is upgraded. A pre-release 3.0.0.RC is published, it's compatible with doctrine/lexer:3.0 (and experimental 3.1)
  • longitude-one/wkb-parser is upgraded. A pre-release 3.0.0.RC is published (no dependencies)

All CrEOF libraries have been replaced (thanks for their initial work) by the longitude-one ones.
I now focus on longitude-one/doctrine-spatial. The doctrine3-feature branch is pushed. The CI is working! A lot of deprecation are thrown as expected, but it will help me to fix everything!

Todo :

  • Fork creof/wkb-parser
  • Upgrade longitude-one/wkb-parser
  • Fix deprecations on longitude-one/doctrine-spatial with the last versions of longitude-one/*parser libraries
  • Fix deprecations on longitude-one/doctrine-spatial with the last versions doctrine/orm:2.* (2.19.3 today)
  • Upgrade longitude-one/doctrine-spatial to be compliant with the "old" versions of doctrine/orm:3.0.*
  • Upgrade longitude-one/doctrine-spatial to be compliant with the last versions of doctrine/orm:3.1.* (3.1.1 today)
  • Have a look on doctrine/orm:3.2
  • Have a look on doctrine/orm:4.0

@Alexandre-T
Copy link
Contributor

Alexandre-T commented Apr 26, 2024

I'm about to create a pull request on the dev branch, targeting a merge with main. This will release longitude-one:doctrine-spatial/4.1.0. This version remains "only" compatible with doctrine/orm:2.19.

Good News!

  • I've made significant progress! Because the curent branch is already compatible with doctrine/orm:3.0, and all tests are passing successfully. I only have to update the composer.json. (ie: I already did the job 🎉​🎉​ )
  • The three dependencies, longitude-one/*-parser:3.0.0.RC, are working flawlessly. I plan to publish them as version 3.0.0 this weekend. (done)
  • Following that, I'll focus on compatibility with doctrine/orm:3.1. In my estimation, this should only take a few hours of development. (done)
  • Once compatibility is achieved, I'll be able to publish longitude-one:doctrine-spatial/5.0.0.RC.

Target Release Date:

Based on my progress, I'm confident I can publish the 5.0.0 release candidate version by May 14th.
The final release will depend on your feedbacks.

Alexandre-T added a commit that referenced this issue Apr 27, 2024
## Doctrine 3 Feature
* Compatibility with doctrine/orm:2.19
* Compatibility with doctrine/orm:3.1
* Compatibility tested (and currently working) with doctrine/orm:3.2.x-dev
* Compatibility tested (and currently working) with doctrine/orm:4.0.x-dev
* Fixing #40 
* Fixing #42 
* Fixing #43 
* Fixing #55


## List of commits
* Composer upgrade feature (#47)
* PHP-Code-sniffer upgraded
* Composer upgraded (internal)
* Github Actions upgraded
* Using last stable version of geo-parser and wkt-parser
* Using last stable version of longitude-one/wkb-parser instead of creof/wkb-parser
* Upgrading to Phpunit 10 to be able to stop tests on deprecation
* Fixing deprecations from geo-parser
* Fixing hosts and ports
* Restoring code coverage
* Add pcov for code coverage
* Fixing a bug
* TODO check the code coverage
* pcov added in PHP
* Fixing directory for code coverage
* Fixing directory for code coverage
* PHP-CS-FIXER
* Quality code steps
* Deprecated method getName() removed Issue #40 fixed
* Some deprecated calls removed, Deprecated File SQL Logger replaced, Unused methods removed
* Ormtest split feature (#52)
* Some deprecated calls removed, Deprecated File SQL Logger replaced, Unused methods removed
* Logger class declared outside of OrmTestCase.php
* Truncate tables before each test
* Removing method
* Quality code improved
* Typo
* Decrease OrmTestCase class complexity
* Phpunit files updated
* Removing 'else' expressions
* Improving quality code
* Test were not executed
* Message fixed
* Typo fixed
* Class-string called without a string
* MySql replaced with MySQL
* All internal exceptions implements ExceptionInterface.
* Fixing comment
* Issue #42 fixed (#53)
* test to reproduce issue added
* Fixing test
* Issue #42 fixed
* Issue 12 (#54)
* Removing travis directories and files
* Creating a new composer script to create code coverage in cov format
* Merging *.cov into a clover.xml file
* Fixing coveralls.io
* Fixing issues with doctrine/orm:3.0
* Adding "with all dependencies" option
* Adding test with doctrine/orm:4.1.x-dev
* Headers updated
* README.md updated
@remy-theroux
Copy link

Good job, thank you that we encourage us to migrate to doctrine 3 ;-)

@manaka02
Copy link

Thank you very much !!

@tiapnn
Copy link

tiapnn commented Apr 29, 2024

Cheers 🍻

@Alexandre-T
Copy link
Contributor

Alexandre-T commented Apr 29, 2024

Thanks for your messages! Don't hesitate to give me feedback on the migration process for your applications. My demo application migrated without errors, but I only use a limited portion of the library and just the PostgreSQL engine.

@captain-igloo
Copy link
Author

Thanks for your work on this. I have found:

  • the 5.0.0.RC release still depends on doctrine/orm 2
  • AbstractPoint.php has "TODO remove this line in version 5"
  • Previously longitude-one/wkt-parser returned float, now returns string, eg:
$parser = new \LongitudeOne\Geo\WKT\Parser();
$point = $parser->parse('POINT(174 -39.1)');

$point['value'][1] should be float but is now a string "-39.1"

@remy-theroux
Copy link

remy-theroux commented Apr 30, 2024

Thanks for your work on this. I have found:

* the 5.0.0.RC release still depends on doctrine/orm 2

* AbstractPoint.php has ["TODO remove this line in version 5"](https://github.com/longitude-one/doctrine-spatial/blob/dev/lib/LongitudeOne/Spatial/PHP/Types/AbstractPoint.php#L149)

* Previously longitude-one/wkt-parser returned float, now returns string, eg:
$parser = new \LongitudeOne\Geo\WKT\Parser();
$point = $parser->parse('POINT(174 -39.1)');

$point['value'][1] should be float but is now a string "-39.1"

The migration is available on this branch dev-doctrine3-feature

@remy-theroux
Copy link

Was available, seems to be on dev branch now.

@Alexandre-T
Copy link
Contributor

Alexandre-T commented Apr 30, 2024

Thank you, for your feedbacks @captain-igloo !

* AbstractPoint.php has ["TODO remove this line in version 5"](https://github.com/longitude-one/doctrine-spatial/blob/dev/lib/LongitudeOne/Spatial/PHP/Types/AbstractPoint.php#L149)

I created the issue #57 about AbstractPoint. My bad! :-) I will fix it tomorrow.

* Previously longitude-one/wkt-parser returned float, now returns string, eg:
$parser = new \LongitudeOne\Geo\WKT\Parser();
$point = $parser->parse('POINT(174 -39.1)');

$point['value'][1] should be float but is now a string "-39.1"

About the wkt-parser, Yes, it returns no longer float, but string. Because the doctrine/lexer only accepts string as input, I had to update the CrEOF code. And I got a lot of conversion issues when I convert it internally. Latitudes like 42.66 could become 42.67 or 42.65!!!! So, I readproof lexer and postgis manuals and the advices was to avoid float and used decimal or strings.

Why this decision... Because neither the geo-parser nor doctrine-spatial manipulates numbers. Doctrine-spatial sends these values to PostGIS or MySQL, and it is the DBMS that does the calculations and the ORM retrieves strings. IMHO, my floating-point conversions were only creating unnecessary conversion problems.

Thanks for your work on this. I have found:

* the 5.0.0.RC release still depends on doctrine/orm 2

Yes, version 5.0.0 is still compatible with doctrine/orm:^2.19. The composer.json file allows it, and unlike AbstractPoint, this is not an oversight on my part. To my surprise, my tests revealed that the code worked with both versions almost flawlessly. I applied one or two changes and the code is compatible with all versions, from 2.19 to 4.0! To ensure that this does not change over time, I have added a matrix to change the orm in the actions.

I believe that a team that wants to stay on version orm:2.19 can use version 4doctrine-spatial:4 . A team that plans to upgrade can go to 5. This allows the team not to change everything at once.

Alexandre-T added a commit that referenced this issue May 1, 2024
Alexandre-T added a commit that referenced this issue May 1, 2024
Alexandre-T added a commit that referenced this issue May 1, 2024
* Fix comments in #43
* Fix #57
* "declare(strict_types=1)" added.
* Fix CI
* Fixing comment headers
@Alexandre-T
Copy link
Contributor

If no issues are reported, longitude-one/doctrine-spatial:5.0.0 will be release on May 15th

Alexandre-T added a commit that referenced this issue May 15, 2024
* Interfaces created to prepare 3.1.0 and 4.0.0

* Interfaces created to prepare 3.1.0 and 4.0.0

* Security issue in doctrine 2.8.3

* Headers updated

* Removing no-more existent rule

* Doctrine3 feature (#55)

## Doctrine 3 Feature
* Compatibility with doctrine/orm:2.19
* Compatibility with doctrine/orm:3.1
* Compatibility tested (and currently working) with doctrine/orm:3.2.x-dev
* Compatibility tested (and currently working) with doctrine/orm:4.0.x-dev
* Fixing #40 
* Fixing #42 
* Fixing #43 
* Fixing #55


## List of commits
* Composer upgrade feature (#47)
* PHP-Code-sniffer upgraded
* Composer upgraded (internal)
* Github Actions upgraded
* Using last stable version of geo-parser and wkt-parser
* Using last stable version of longitude-one/wkb-parser instead of creof/wkb-parser
* Upgrading to Phpunit 10 to be able to stop tests on deprecation
* Fixing deprecations from geo-parser
* Fixing hosts and ports
* Restoring code coverage
* Add pcov for code coverage
* Fixing a bug
* TODO check the code coverage
* pcov added in PHP
* Fixing directory for code coverage
* Fixing directory for code coverage
* PHP-CS-FIXER
* Quality code steps
* Deprecated method getName() removed Issue #40 fixed
* Some deprecated calls removed, Deprecated File SQL Logger replaced, Unused methods removed
* Ormtest split feature (#52)
* Some deprecated calls removed, Deprecated File SQL Logger replaced, Unused methods removed
* Logger class declared outside of OrmTestCase.php
* Truncate tables before each test
* Removing method
* Quality code improved
* Typo
* Decrease OrmTestCase class complexity
* Phpunit files updated
* Removing 'else' expressions
* Improving quality code
* Test were not executed
* Message fixed
* Typo fixed
* Class-string called without a string
* MySql replaced with MySQL
* All internal exceptions implements ExceptionInterface.
* Fixing comment
* Issue #42 fixed (#53)
* test to reproduce issue added
* Fixing test
* Issue #42 fixed
* Issue 12 (#54)
* Removing travis directories and files
* Creating a new composer script to create code coverage in cov format
* Merging *.cov into a clover.xml file
* Fixing coveralls.io
* Fixing issues with doctrine/orm:3.0
* Adding "with all dependencies" option
* Adding test with doctrine/orm:4.1.x-dev
* Headers updated
* README.md updated

* Removing unused file

* Code coverage improved

* Issue #57 fixed (#59)

* Fix comments in #43
* Fix #57
* "declare(strict_types=1)" added.
* Fix CI
* Fixing comment headers

* Quality tools feature (#61)

* Longitude-One quality tools added
* Headers updated
* PHP-Cs-Fixer Rules updated
* Code sniffer rules updated

* Ecologic feature (#63)

* Ecologic feature.
To avoid useless tests, the full tests are only launched when tests and quality tools are completed on stable version.

* Specify return types  (#56)

* Return types of test methods specified
* Quality-tools after merge
* PHP Stan level 0
* Restore compatibility with ORM ^2.19
* Architecture updated
* PHP-Stan level 2
* PHP-Stan level 3
* PHP-Stan level 4
* Fixing UnsupportedPlatform test
* PHP-Stan level 5
* PHP-Stan level 6
* Fixing platform test on orm:^2.19
* PHP-Stan level 7
* PHP-Stan level 8
* PHP-Stan level 9

* Fixing warning

* Issue #65 (#66)

* Enhancement #65 implemented
* Php-Stan check activated
* Add delta because results are different between MySQL5.7 and MySQL8.0

* Return types mentioned (#67)

* Return types mentioned
* Unexpected file removed
* Show installed libraries
* Remove no-longer existing error in baseline
* Remove no-longer used libraries in composer

* Php-stan fix

* Php-stan fix
@Alexandre-T
Copy link
Contributor

Alexandre-T commented May 15, 2024

Version 5.0.0 released : #69

@captain-igloo
Copy link
Author

Further to your comment about floats changing to strings above. Now LongitudeOne\Spatial\PHP\Types\AbstractPoint::setX() and setY() take a string. Should these not take int or string?

https://github.com/longitude-one/doctrine-spatial/blob/main/lib/LongitudeOne/Spatial/PHP/Types/AbstractPoint.php#L127

@Alexandre-T
Copy link
Contributor

Alexandre-T commented May 17, 2024

Thank you for your feedback.

I edited my comment, because I was wrong. Don't hesitate to create an issue.

Yes, it should.

IMHO, the geo-parser could have a lot of issues with these unexpected conversions. I have to check it and proofread the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants