Skip to content

PHPORM-167 Fix and test MongoDB failed queue provider #2838

New issue

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

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

Already on GitHub? # to your account

Merged
merged 3 commits into from
Apr 15, 2024

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Apr 9, 2024

Fix PHPORM-167

In Laravel 11.35, the DatabaseFailedJobProvider::ids was added in order to optimize loading this ids only instead of the full documents. laravel/framework#49186

This implementation is not compatible with MongoDB since it references the field id instead of _id.

Also, the method DatabaseFailedJobProvider::prune is not compatible with MongoDB because it tries to delete document by bulks of 1000, which is not supported. MongoDB can only delete one or all documents matching a criteria.

I'm also adding tests to the methods inherited from the parent DatabaseFailedJobProvider to ensure they work with a MongoDB connection.

Checklist

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

@GromNaN GromNaN requested a review from a team as a code owner April 9, 2024 18:40
@GromNaN GromNaN requested a review from jmikola April 9, 2024 18:40
@GromNaN GromNaN changed the base branch from 4.3 to 4.2 April 9, 2024 18:42
@GromNaN GromNaN changed the base branch from 4.2 to 4.3 April 9, 2024 18:42
Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested comment change.

Also a question about _id types. If the assumption is correct, then just resolve my comment there.

->collection('failed_jobs')
->raw()
->insertMany(array_map(static fn ($i) => [
'_id' => new ObjectId(sprintf('%024d', $i)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are _id fields within jobs always an ObjectId? I saw that MongoFailedJobProvider made some assumptions about that by using string for type hints and casting.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documents are added by the log method which doesn't specify _id. So we get an automatic ObjectId.

@GromNaN GromNaN merged commit bd6aa40 into mongodb:4.3 Apr 15, 2024
23 checks passed
@GromNaN GromNaN deleted the PHPORM-167 branch April 15, 2024 14:10
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants