Doctrine Common
  1. Doctrine Common
  2. DCOM-28

Extract Common Persistance Interfaces

    Details

    • Type: New Feature New Feature
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      I discussed this with jwage on symfony day cologne and this also came up during discussions with @dzuelke at IPC yesterday. So i hacked up a first patch for discussion that adds a Doctrine\Common\Persistance namespace and extracts the functionality all our 3 layers implement with regards to EntityManager/EntityRepository (and equivalents).

      Additionally i think it might make sense to also add an interface "ObjectMetadata" that has several getters-only that allow access to the field, identifier and association mapping information. This stuff is not necessarly compatible across layers when returned as its "array" representation, but for libraries hooking into the metadata (symfony admin generator) this might not even be necessary.

        Issue Links

          Activity

          Hide
          Jonathan H. Wage added a comment -

          Added the interfaces here https://github.com/doctrine/common/commit/59e6b8c6edcb271622923035b687a063c2b47ce8

          I implemented them here in the mongodb-odm https://github.com/doctrine/mongodb-odm/commit/8d02e8439fb6737de1e23e1953a643858a8a6c68

          and the ORM https://github.com/doctrine/doctrine2/commit/68a40996841b1dbec3b8de5c1038809e5db512b7

          I think we can add a few more methods to ClassMetadata interface that are always gonna exist between the different persistence layers. Let me know what you think and what you want to add.

          Show
          Jonathan H. Wage added a comment - Added the interfaces here https://github.com/doctrine/common/commit/59e6b8c6edcb271622923035b687a063c2b47ce8 I implemented them here in the mongodb-odm https://github.com/doctrine/mongodb-odm/commit/8d02e8439fb6737de1e23e1953a643858a8a6c68 and the ORM https://github.com/doctrine/doctrine2/commit/68a40996841b1dbec3b8de5c1038809e5db512b7 I think we can add a few more methods to ClassMetadata interface that are always gonna exist between the different persistence layers. Let me know what you think and what you want to add.
          Hide
          Guilherme Blanco added a comment -

          Jon is working on this.

          Show
          Guilherme Blanco added a comment - Jon is working on this.
          Hide
          Lukas Kahwe added a comment -

          CouchDB ODM also has DocumentRepositry::findMany(array $ids)
          Should we add that to the interface?

          Show
          Lukas Kahwe added a comment - CouchDB ODM also has DocumentRepositry::findMany(array $ids) Should we add that to the interface?
          Hide
          Benjamin Eberlei added a comment -

          No, that method is only on the repository because CouchDB doesn't need persisters (yet). Its not part of the interfaceable public methods.

          Show
          Benjamin Eberlei added a comment - No, that method is only on the repository because CouchDB doesn't need persisters (yet). Its not part of the interfaceable public methods.
          Hide
          Lukas Kahwe added a comment -

          Not sure I understand. The method is used in DocumentRepository::findBy() as well as in PersistentIdsCollection::load. Seems unnecessary for that method to be public just for this. At any rate imho the method seems convenient and also allows for more efficient access in many RDBMS compared to the generic findBy($criteria) method. So it seems worthwhile adding it.

          Show
          Lukas Kahwe added a comment - Not sure I understand. The method is used in DocumentRepository::findBy() as well as in PersistentIdsCollection::load. Seems unnecessary for that method to be public just for this. At any rate imho the method seems convenient and also allows for more efficient access in many RDBMS compared to the generic findBy($criteria) method. So it seems worthwhile adding it.
          Hide
          Benjamin Eberlei added a comment -

          It doesn't matter what CouchDB uses internally on the DocumentRepository, i don't think this method is particularly useful in another context than CouchDBs use of Collections. In any case the method is just a proxy for findBy(array("id" => array(1, 2, 3, 4, 5, 6))); and i am not sure we need such a method on an interface just for convenience, Repository::find() use-case is much broader.

          Show
          Benjamin Eberlei added a comment - It doesn't matter what CouchDB uses internally on the DocumentRepository, i don't think this method is particularly useful in another context than CouchDBs use of Collections. In any case the method is just a proxy for findBy(array("id" => array(1, 2, 3, 4, 5, 6))); and i am not sure we need such a method on an interface just for convenience, Repository::find() use-case is much broader.
          Hide
          Lukas Kahwe added a comment -

          Actually using findMany($ids) ias clearly more efficient in CouchDB than using findBy(array("id" => $ids));
          The same applies to PHPCR, since going by the node path means you can go via the normal node API, rather than the search API.
          So this is already 2 out of 3 examples and I do not know MongoDB enough to tell if they also have some specific API advantage.

          Show
          Lukas Kahwe added a comment - Actually using findMany($ids) ias clearly more efficient in CouchDB than using findBy(array("id" => $ids)); The same applies to PHPCR, since going by the node path means you can go via the normal node API, rather than the search API. So this is already 2 out of 3 examples and I do not know MongoDB enough to tell if they also have some specific API advantage.
          Hide
          Benjamin Eberlei added a comment -

          Hm, you might be right. Ok, this should be included.

          What i am still pondering with is adding array $orderBy and $firstResult, $maxResults to findOneBy() and findBy() and findAll().

          @Jon: Would this be possible for MongoDB?

          It would not be possible for all use-cases in CouchDB, but for some it can work.

          Show
          Benjamin Eberlei added a comment - Hm, you might be right. Ok, this should be included. What i am still pondering with is adding array $orderBy and $firstResult, $maxResults to findOneBy() and findBy() and findAll(). @Jon: Would this be possible for MongoDB? It would not be possible for all use-cases in CouchDB, but for some it can work.
          Hide
          Lukas Kahwe added a comment -

          Just a super trivial pull to add this to the interface:
          https://github.com/doctrine/common/pull/11

          Show
          Lukas Kahwe added a comment - Just a super trivial pull to add this to the interface: https://github.com/doctrine/common/pull/11
          Hide
          Lukas Kahwe added a comment -

          also wondering if we want to include the idgenerator API in the interface?

          Show
          Lukas Kahwe added a comment - also wondering if we want to include the idgenerator API in the interface?
          Hide
          Benjamin Eberlei added a comment -

          Hm yes, i think that is necessary. At least the differentation between assigned and auto-generated ids is relevant for metadata queries.

          Show
          Benjamin Eberlei added a comment - Hm yes, i think that is necessary. At least the differentation between assigned and auto-generated ids is relevant for metadata queries.
          Hide
          Jonathan H. Wage added a comment -

          Yes, findMany() is possible with MongoDB.

          I think it would just be a proxy to:

          public function findMany(array $ids)
          {
              return $this->findBy(array('_id' => array('$in' => $ids)));
          }
          
          Show
          Jonathan H. Wage added a comment - Yes, findMany() is possible with MongoDB. I think it would just be a proxy to: public function findMany(array $ids) { return $ this ->findBy(array('_id' => array('$in' => $ids))); }
          Hide
          Benjamin Eberlei added a comment -

          Hm, What is this syntax? This is not conforming to the EntityRepository interface.

          The only two allowed methods are:

          IN Query:

          $ids = array(...);
          findBy(array('_id' => $ids)); 
          

          Equals = Query:

          $ids = 1234;
          findBy(array('_id' => $id)); 
          

          Everything else is not portable accross implementations and should only be able through DocumentManager::CreateQuery* sort of apis.

          Show
          Benjamin Eberlei added a comment - Hm, What is this syntax? This is not conforming to the EntityRepository interface. The only two allowed methods are: IN Query: $ids = array(...); findBy(array('_id' => $ids)); Equals = Query: $ids = 1234; findBy(array('_id' => $id)); Everything else is not portable accross implementations and should only be able through DocumentManager::CreateQuery* sort of apis.
          Hide
          Jonathan H. Wage added a comment -

          To find by things in MongoDB you just give an array of key => value pairs. It gets passed straight through to MongoDB. The $in syntax is just soemthing mongodb supports. It's not anything specific to Doctrine.

          Show
          Jonathan H. Wage added a comment - To find by things in MongoDB you just give an array of key => value pairs. It gets passed straight through to MongoDB. The $in syntax is just soemthing mongodb supports. It's not anything specific to Doctrine.
          Hide
          Lukas Kahwe added a comment -

          i want to heat this topic back up:

          • findMany()
          • id generator API
          Show
          Lukas Kahwe added a comment - i want to heat this topic back up: findMany() id generator API

            People

            • Assignee:
              Jonathan H. Wage
              Reporter:
              Benjamin Eberlei
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated: