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

Feature/new mongodb driver #14230

Closed
wants to merge 36 commits into from

Conversation

tacxou
Copy link

@tacxou tacxou commented Jul 3, 2019

Hello!

In raising this pull request, I confirm the following:

  • I have read and understood the Contributing Guidelines
  • I have checked that another pull request for this purpose does not exist
  • I wrote some tests for this PR
  • I have updated the relevant CHANGELOG
  • I have created a PR for the documentation about this change
  • Added Phalcon\Mvc\Collection\Document::__construct()
  • Added Phalcon\Mvc\Collection\Document::count()
  • Changed Phalcon\Mvc\Collection\Document
    • Now implements Countable
  • Changed Phalcon\Mvc\Collection::findFirst()
    • Now returns a null or CollectionInterface
  • Changed Phalcon\Mvc\Collection::getId()
    • Now returns a \MongoDB\BSON\ObjectId
  • Depreciated Phalcon\Mvc\Collection::createIfNotExist()
  • Deleted Phalcon\Mvc\Collection::summatory()
    • This method to group records no longer exists in the new driver. It can be easily replaced by the MongoDB $group in aggregation framework, you can use Phalcon\Mvc\Collection::aggregate() method.
  • Changed Phalcon\Mvc\Collection\Manager::getCustomEventsManager()
    • Now return a null or Phalcon\Events\ManagerInterface
  • Changed Phalcon\Mvc\Collection\Manager::getConnection()
    • Now return a \MongoDB\Database or Phalcon\Db\Adapter\AdapterInterface
  • Refactoring Phalcon\Mvc\Collection\Manager
    • Events triggered in collections are now prefixed with collection: instead of model:
    • Modification of variable names to match collections (var model to var collection)
  • Refactoring Phalcon\Mvc\Collection
    • Modification of variables of type MongoId to the new type ObjectId
    • Modification of variable names to match collections (var mongoId to var objectId)

Thanks

@sergeyklay
Copy link
Contributor

sergeyklay commented Jul 8, 2019

@tacxticx88 Could you please rebase. I've installed mongodb extension in #14236

phalcon/Mvc/Collection.zep Outdated Show resolved Hide resolved
phalcon/Mvc/Collection.zep Outdated Show resolved Hide resolved
phalcon/Mvc/Collection/ManagerInterface.zep Outdated Show resolved Hide resolved
phalcon/Mvc/Collection.zep Outdated Show resolved Hide resolved
phalcon/Mvc/Collection.zep Show resolved Hide resolved
phalcon/Mvc/Collection.zep Outdated Show resolved Hide resolved
@sergeyklay sergeyklay added breaks bc Functionality that breaks Backwards Compatibility new feature request Planned Feature or New Feature Request wip Work In Progress labels Jul 9, 2019
@CameronHall
Copy link
Contributor

What is this waiting on?

@ruudboon
Copy link
Member

ruudboon commented Aug 4, 2019

@tacxticx88 Can you fix the 2 line enters to pass the last test and update the changelog?
https://travis-ci.org/phalcon/cphalcon/jobs/567264410

Copy link
Contributor

@sergeyklay sergeyklay left a comment

Choose a reason for hiding this comment

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

There are some minor questions I would like understand to. Also we'll need update change log (see my comments). But at the moment it seems good to me.

phalcon/Mvc/Collection.zep Show resolved Hide resolved
phalcon/Mvc/Collection.zep Show resolved Hide resolved
phalcon/Mvc/Collection.zep Show resolved Hide resolved
phalcon/Mvc/Collection/Document.zep Show resolved Hide resolved
phalcon/Mvc/Collection/Manager.zep Outdated Show resolved Hide resolved
phalcon/Mvc/Collection/Manager.zep Show resolved Hide resolved
phalcon/Mvc/Collection/ManagerInterface.zep Outdated Show resolved Hide resolved
phalcon/Mvc/CollectionInterface.zep Show resolved Hide resolved
phalcon/Mvc/CollectionInterface.zep Show resolved Hide resolved
@CameronHall CameronHall mentioned this pull request Aug 10, 2019
3 tasks
@niden niden added the 4.1 label Aug 18, 2019
@ruudboon
Copy link
Member

@tacxticx88 If you have time can you rebase your branch to see where we stand?

@tacxou
Copy link
Author

tacxou commented Jan 15, 2020

@ruudboon I'll do that. Yes, my work was moved to a private storage facility before the switch to Phalcon v4.

@ruudboon
Copy link
Member

@tacxticx88 We're currently upgrading our testsuite as well. If you need some help rewriting the test let us know.

@ruudboon
Copy link
Member

ruudboon commented Jun 5, 2020

@tacxticx88 Any news on this? If you have time to work on this can you switch to the 4.1.x branch? thnx

@niden
Copy link
Member

niden commented Jun 18, 2020

Mentioning this one more time just in case.

For this PR to work and to be part of core, we need to ensure that it does not reference any classes or interfaces that a different extension has (in this case the php mongo extension). Doing so will require that extension to be loaded first before Phalcon even starts. Example is the PSR extension which is now a requirement for Phalcon to work.

@tacxou
Copy link
Author

tacxou commented Jun 19, 2020

Hello, I'm currently looking for a solution to this problem because the Collection class should implement the interface of MongoDb to be able to automatically caste the model and its child classes..

@tacxou
Copy link
Author

tacxou commented Jun 19, 2020

I suggest to finally write the ODM in PHP in order to take advantage of MongoDb's Composer repository. In my case the work is already well advanced. What do you think of that? MongoDb having its driver written mostly in PHP.

@niden
Copy link
Member

niden commented Jun 23, 2020

I suggest to finally write the ODM in PHP in order to take advantage of MongoDb's Composer repository. In my case the work is already well advanced. What do you think of that? MongoDb having its driver written mostly in PHP.

@tacxticx88 I think that might be the best solution at least for the time being until we regroup and figure out a way to solve the technical issues that will appear by including the mongo interfaces in the core.

We can set up an incubator repository and all the work and support can go there. People will then be able to add the component using composer.

Maybe later on we can include this functionality with FFI in the core or something similar - not sure now just thinking out loud.

I agree, we need to get this done in the incubator.

@ruudboon
Copy link
Member

ruudboon commented Sep 2, 2020

@tacxticx88 Do you agree on closing this one?

@tacxou
Copy link
Author

tacxou commented Sep 2, 2020

@tacxticx88 Do you agree on closing this one?

Yes. See the new phalcon/incobator-mongodb repository

@ruudboon
Copy link
Member

Closing this in favour of the new https://github.com/phalcon/incubator-mongodb repo

@ruudboon ruudboon closed this Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks bc Functionality that breaks Backwards Compatibility new feature request Planned Feature or New Feature Request wip Work In Progress
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants