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

PHPORM-230 Convert DateTimeInterface to UTCDateTime in queries #3105

Merged
merged 2 commits into from
Aug 26, 2024

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Aug 21, 2024

Fix PHPORM-230

By default, DateTimeInterface objects are converted to empty objects by the PHP driver. In this package, they are already explicitly converted to MongoDB\BSON\UTCDateTime objects in Eloquent models (depending on cast) and DB queries.

This PR generalize this transformation by applying the conversion to every command: including DB::insert and DB::update values.

Checklist

  • Add tests and ensure they pass
  • Add an entry to the CHANGELOG.md file
  • Update documentation for new features

@GromNaN GromNaN added this to the 5.0 milestone Aug 21, 2024
@GromNaN GromNaN requested a review from a team as a code owner August 21, 2024 10:08
@GromNaN GromNaN requested a review from mana2-bot August 21, 2024 10:08
@GromNaN GromNaN requested review from alcaeus and removed request for mana2-bot August 21, 2024 11:05
@masterbater
Copy link
Contributor

masterbater commented Aug 21, 2024

Is this a fix to this issue?
I tested
"mongodb/laravel-mongodb": "5.0.x-dev"

Got this error in laravel breeze starter,
run php artisan test

Stack trace ```php TypeError: Carbon\Carbon::parse(): Argument #1 ($time) must be of type DateTimeInterface|Carbon\WeekDay|Carbon\Month|string|int|float|null, array given, called in /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Auth/Passwords/DatabaseTokenRepository.php on line 149 and defined in /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/nesbot/carbon/src/Carbon/Traits/Creator.php:206 Stack trace: #0 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Auth/Passwords/DatabaseTokenRepository.php(149): Carbon\Carbon::parse() #1 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Auth/Passwords/DatabaseTokenRepository.php(137): Illuminate\Auth\Passwords\DatabaseTokenRepository->tokenExpired() #2 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Auth/Passwords/PasswordBroker.php(132): Illuminate\Auth\Passwords\DatabaseTokenRepository->exists() #3 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Auth/Passwords/PasswordBroker.php(99): Illuminate\Auth\Passwords\PasswordBroker->validateReset() #4 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Auth/Passwords/PasswordBrokerManager.php(144): Illuminate\Auth\Passwords\PasswordBroker->reset() #5 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(357): Illuminate\Auth\Passwords\PasswordBrokerManager->__call() #6 /home/gwapoko/laravelprojects/mongodbonlylaravel/app/Http/Controllers/Auth/NewPasswordController.php(46): Illuminate\Support\Facades\Facade::__callStatic() #7 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(46): App\Http\Controllers\Auth\NewPasswordController->store() #8 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Routing/Route.php(260): Illuminate\Routing\ControllerDispatcher->dispatch() #9 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Routing/Route.php(206): Illuminate\Routing\Route->runController() #10 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Routing/Router.php(808): Illuminate\Routing\Route->run() #11 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(144): Illuminate\Routing\Router->Illuminate\Routing\{closure}() #12 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Auth/Middleware/RedirectIfAuthenticated.php(35): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #13 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Auth\Middleware\RedirectIfAuthenticated->handle() #14 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Http/Middleware/AddLinkHeadersForPreloadedAssets.php(19): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #15 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Http\Middleware\AddLinkHeadersForPreloadedAssets->handle() #16 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/inertiajs/inertia-laravel/src/Middleware.php(86): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #17 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Inertia\Middleware->handle() #18 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Routing/Middleware/SubstituteBindings.php(51): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #19 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Routing\Middleware\SubstituteBindings->handle() #20 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/VerifyCsrfToken.php(88): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #21 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Foundation\Http\Middleware\VerifyCsrfToken->handle() #22 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/View/Middleware/ShareErrorsFromSession.php(49): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #23 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\View\Middleware\ShareErrorsFromSession->handle() #24 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Session/Middleware/StartSession.php(121): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #25 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Session/Middleware/StartSession.php(64): Illuminate\Session\Middleware\StartSession->handleStatefulRequest() #26 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Session\Middleware\StartSession->handle() #27 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Cookie/Middleware/AddQueuedCookiesToResponse.php(37): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #28 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse->handle() #29 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Cookie/Middleware/EncryptCookies.php(75): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #30 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Cookie\Middleware\EncryptCookies->handle() #31 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(119): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #32 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Routing/Router.php(807): Illuminate\Pipeline\Pipeline->then() #33 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Routing/Router.php(786): Illuminate\Routing\Router->runRouteWithinStack() #34 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Routing/Router.php(750): Illuminate\Routing\Router->runRoute() #35 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Routing/Router.php(739): Illuminate\Routing\Router->dispatchToRoute() #36 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(201): Illuminate\Routing\Router->dispatch() #37 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(144): Illuminate\Foundation\Http\Kernel->Illuminate\Foundation\Http\{closure}() #38 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #39 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ConvertEmptyStringsToNull.php(31): Illuminate\Foundation\Http\Middleware\TransformsRequest->handle() #40 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull->handle() #41 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #42 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TrimStrings.php(51): Illuminate\Foundation\Http\Middleware\TransformsRequest->handle() #43 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Foundation\Http\Middleware\TrimStrings->handle() #44 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Http/Middleware/ValidatePostSize.php(27): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #45 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Http\Middleware\ValidatePostSize->handle() #46 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/PreventRequestsDuringMaintenance.php(110): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #47 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Foundation\Http\Middleware\PreventRequestsDuringMaintenance->handle() #48 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Http/Middleware/HandleCors.php(49): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #49 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Http\Middleware\HandleCors->handle() #50 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Http/Middleware/TrustProxies.php(57): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #51 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Http\Middleware\TrustProxies->handle() #52 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(119): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}() #53 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(176): Illuminate\Pipeline\Pipeline->then() #54 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(145): Illuminate\Foundation\Http\Kernel->sendRequestThroughRouter() #55 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php(604): Illuminate\Foundation\Http\Kernel->handle() #56 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php(394): Illuminate\Foundation\Testing\TestCase->call() #57 /home/gwapoko/laravelprojects/mongodbonlylaravel/tests/Feature/Auth/PasswordResetTest.php(47): Illuminate\Foundation\Testing\TestCase->post() #58 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Support/Testing/Fakes/NotificationFake.php(257): P\Tests\Feature\Auth\PasswordResetTest->{closure}() #59 [internal function]: Illuminate\Support\Testing\Fakes\NotificationFake->Illuminate\Support\Testing\Fakes\{closure}() #60 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Collections/Arr.php(926): array_filter() #61 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Collections/Collection.php(391): Illuminate\Support\Arr::where() #62 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Support/Testing/Fakes/NotificationFake.php(256): Illuminate\Support\Collection->filter() #63 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Support/Testing/Fakes/NotificationFake.php(90): Illuminate\Support\Testing\Fakes\NotificationFake->sent() #64 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(357): Illuminate\Support\Testing\Fakes\NotificationFake->assertSentTo() #65 /home/gwapoko/laravelprojects/mongodbonlylaravel/tests/Feature/Auth/PasswordResetTest.php(46): Illuminate\Support\Facades\Facade::__callStatic() #66 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/pestphp/pest/src/Factories/TestCaseMethodFactory.php(110): P\Tests\Feature\Auth\PasswordResetTest->{closure}() #67 [internal function]: P\Tests\Feature\Auth\PasswordResetTest->Pest\Factories\{closure}() #68 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/pestphp/pest/src/Concerns/Testable.php(337): call_user_func_array() #69 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/pestphp/pest/src/Support/ExceptionTrace.php(26): P\Tests\Feature\Auth\PasswordResetTest->Pest\Concerns\{closure}() #70 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/pestphp/pest/src/Concerns/Testable.php(337): Pest\Support\ExceptionTrace::ensure() #71 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/pestphp/pest/src/Concerns/Testable.php(254): P\Tests\Feature\Auth\PasswordResetTest->__callClosure() #72 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/pestphp/pest/src/Factories/TestCaseFactory.php(196) : eval()'d code(71): P\Tests\Feature\Auth\PasswordResetTest->__runTest() #73 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/phpunit/phpunit/src/Framework/TestCase.php(1122): P\Tests\Feature\Auth\PasswordResetTest->__pest_evaluable_password_can_be_reset_with_valid_token() #74 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/phpunit/phpunit/src/Framework/TestCase.php(654): PHPUnit\Framework\TestCase->runTest() #75 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/phpunit/phpunit/src/Framework/TestRunner.php(105): PHPUnit\Framework\TestCase->runBare() #76 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/phpunit/phpunit/src/Framework/TestCase.php(488): PHPUnit\Framework\TestRunner->run() #77 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/phpunit/phpunit/src/Framework/TestSuite.php(349): PHPUnit\Framework\TestCase->run() #78 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/phpunit/phpunit/src/Framework/TestSuite.php(349): PHPUnit\Framework\TestSuite->run() #79 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/phpunit/phpunit/src/Framework/TestSuite.php(349): PHPUnit\Framework\TestSuite->run() #80 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/phpunit/phpunit/src/TextUI/TestRunner.php(62): PHPUnit\Framework\TestSuite->run() #81 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/phpunit/phpunit/src/TextUI/Application.php(198): PHPUnit\TextUI\TestRunner->run() #82 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/pestphp/pest/src/Kernel.php(103): PHPUnit\TextUI\Application->run() #83 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/pestphp/pest/bin/pest(91): Pest\Kernel->handle() #84 /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/pestphp/pest/bin/pest(99): {closure}() #85 {main} ----------------------------------------------------------------------------------

Carbon\Carbon::parse(): Argument #1 ($time) must be of type DateTimeInterface|Carbon\WeekDay|Carbon\Month|string|int|float|null, array given, called in /home/gwapoko/laravelprojects/mongodbonlylaravel/vendor/laravel/framework/src/Illuminate/Auth/Passwords/DatabaseTokenRepository.php on line 149

</details>

@masterbater
Copy link
Contributor

masterbater commented Aug 21, 2024

image I didnt open an issue since Im not using a stable release, if its not ok here then let me know thanks

@masterbater
Copy link
Contributor

masterbater commented Aug 21, 2024

If the issue above is unrelated to this PR let me know I will delete this, sorry for polluting comments here
Sorry the solution was very simple just add this in Service Provider

MongoDB\Laravel\Auth\PasswordResetServiceProvider::class,

@GromNaN
Copy link
Member Author

GromNaN commented Aug 21, 2024

the solution was very simple just add this in Service Provider

MongoDB\Laravel\Auth\PasswordResetServiceProvider::class,

Your issue could have been avoided by automatically converting UTCDateTime to DateTimeImmutable objects in query results. That way, the custom DatabaseTokenRepository would become useless. I haven't do that at first, because it rases opinionated choices (class Carbon or DateTime, immutability, timezone).

@masterbater
Copy link
Contributor

the solution was very simple just add this in Service Provider
MongoDB\Laravel\Auth\PasswordResetServiceProvider::class,

Your issue could have been avoided by automatically converting UTCDateTime to DateTimeImmutable objects in query results. That way, the custom DatabaseTokenRepository would become useless. I haven't do that at first, because it rases opinionated choices (class Carbon or DateTime, immutability, timezone).

That would have been nice. You do what's best. Just my opinion it might be better if in docs there is quick section for FAQ or cheatsheet that is not by default supported by laravel mongodb and need to use a Custom Provider. Just minor improvement so people that are already familiar can jump straight to it hehe, but then again the documentation is great

@GromNaN
Copy link
Member Author

GromNaN commented Aug 26, 2024

I decided not convert dates in results for now. Tracking in PHPORM-234

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

Successfully merging this pull request may close these issues.

3 participants