Fixed Document annotation identification bug #22

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

marnusw commented Aug 15, 2011

In AnnotationDriver#loadMetadataForClass() it was incorrectly assumed that an array of annotations indexed by annotation name was returned; in fact a numerically indexed array is received. This patch fixes the identification of the annotations that are present.

Since only one Document realted annotation should be present the method now also throws an exception if more than one is found. (One could just use the first annotation, but this may cause confusion when debugging.)

Not sure if this was the only outstanding problem related to issue CODM-28 but I think it was related.

@marnusw marnusw The read class annotations are now correctly processed as an array of
numerically indexed values in AnnotationDriver#loadMetadataForClass().

Since only one Document realted annotation should be present an exception is
thrown if more than one is found. (One could just use the first annotation,
but this may cause confusion when debugging.)

The type of the instanced annotation class is used to determine the type
of Document since there are no array keys to check against.
45c4f55
@ghost

ghost commented Sep 13, 2011

Not sure if the patch you posted is related to the issue where by exceptions is thrown indicating that document class is invalid. I patched classes locally "downloaded and overwrote class" but I'm still getting exception:

Class TestCompany\HomeBundle\Resources\couchdb\persistance\CouchdbPersistanceSite is not a valid document or mapped super class. 

my document class looks like this:

namespace TestCompany\HomeBundle\Resources\Couchdb\Persistance;

/** @Document */
class CouchdbPersistanceSite{
    
    /** @Id */
    private $id;
    
    /** @Field(type="string") */
    private $name;
    
    /**
     * @return the $id
     */
    public function getId() {
        return $this->id;
    }

    /**
     * @return the $name
     */
    public function getName() {
        return $this->name;
    }

    /**
     * @param field_type $id
     */
    public function setId($id) {
        $this->id = $id;
    }

    /**
     * @param field_type $name
     */
    public function setName($name) {
        $this->name = $name;
    }
}

Kindly advice.

marnusw closed this Sep 13, 2011

marnusw reopened this Sep 13, 2011

Contributor

marnusw commented Sep 13, 2011

@kwasigyasiagyei
It seems your problem could be related. The exception you've posted is thrown when the $documentAnnot does not match any of the tested class types and the else part of the if-else chain is reached (see the end of the updated section).

Try a var_dump of $documentAnnot and see what comes out and why the class type is not valid.

Let me know if you're still stuck.

@ghost

ghost commented Sep 13, 2011

Thanks for your response, further investigate indicates that the exception is throw from code below:

    public function loadMetadataForClass($className, ClassMetadata $class)
    {
        foreach ($this->drivers as $namespace => $driver) {
            if (strpos($className, $namespace) === 0) {
                $driver->loadMetadataForClass($className, $class);
                return;
            }
        }

        throw MappingException::classIsNotAValidDocument($className);
    }

It appears the code is unable to find driver required to load class meta data. I'm using the doctrine-odm symfony bundle, which I believe may probably be the problem. But I'm not so sure, cause class throwing exception is

DriverChain in namespace Doctrine\ODM\CouchDB\Mapping\Driver

Dump of driver array = empty array 0_o

var_dump( $this->drivers );

Contributor

marnusw commented Sep 13, 2011

@kwasigyasiagyei
It seems you are trying to load the metadata with an empty driver chain. Check out the instantiation of the Configuration object you pass to the constructor of your DocumentManager. You should be doing something like the setup in section 3.3 here.

Since you are using a driver chain you actually have to add an instance of ...\Driver\AnnotationDriver to it using DriverChain#addDriver(); you can use
$metadataDriver = $config->newDefaultAnnotationDriver($documentPaths);
to obtain the driver or look at its constructor and instantiate one yourself. You can also do away with the driver chain entirely and just use the AnnotationDriver (they have the same interface), the driver chain is only necessary when the metadata of different namespaces should be loaded with different drivers.

@ghost

ghost commented Sep 14, 2011

@marnusw

You are right in saying that, however based on my understand of dependency injection and Symphony 2 bundles, I believe since I'm using a configured bundle all that should happen automatically. I'm currently finding difficulties in figuring out exactly where configuration $metadataDriver = $config->newDefaultAnnotationDriver($documentPaths); object is configured.

Based on the above it means that I haven't yet reached the class you made changes on, so this issue actually has nothing to with your patch.

When I find a solution I will update you in case you use the bundled Symfony2 option at some point in time.

Thanks for your time.

Contributor

marnusw commented Sep 14, 2011

Thanks. I use the Zend framework, but I've integrated the symfony dependency injection container. I'll send you my xml file when I'm at my pc again incase that helps you.

Regards,
Marnus

------Original Message------
From: Kwasi
To: Marnus Weststrate
Subject: Re: [couchdb-odm] Fixed Document annotation identification bug (#22)
Sent: Sep 14, 2011 06:56

@marnusw

You are right in saying that, however based on my understand of dependency injection and Symphony 2 bundles, I believe since I'm using a configured bundle all that should happen automatically. I currently finding difficult figuring out exactly where a new configuration is configured.

Based on the above it means that I haven't yet reached the class you made changes on, so this issue actually has nothing to with your patch.

When I find a solution I will update you in case you use the bundled Symfony2 option at some point in time.

Thanks for your time.

Reply to this email directly or view it on GitHub:
#22 (comment)

Sent via my BlackBerry from Vodacom - let your email find you!

Contributor

marnusw commented Sep 14, 2011

@kwasigyasiagyei

Symfony dependency injection setup with Doctrine CouchDb ODM.

My ini file:

[parameters]

doctrine.database.host = localhost
doctrine.database.port = 5984
doctrine.database.username = "<user>"
doctrine.database.password = "<secret>"
doctrine.database.name = "pockets_test_database"

doctrine.document_paths[] = "<path to document classes"
doctrine.annotations.namespace = "Doctrine\ODM\CouchDB\Mapping\\"
doctrine.annotations.ignoreUnknown = true
doctrine.metadata.cache.class = "Doctrine\Common\Cache\ArrayCache"
doctrine.proxy_path = APPLICATION_PATH . "/../Pockets/Helpers/Doctrine/Proxies"
doctrine.lucene_handler_name = "_fti"

My XML file:

<?xml version="1.0"?>
<container xmlns="http://symfony-project.org/2.0/container">
    <services>

        <!-- $sc->getService('users.doc_manager'); -->

        <!-- DOCTRINE COUCH_DB DOCUMENT MANAGER -->
        <service id="users.doc_manager" shared="false" class="Doctrine\ODM\CouchDB\DocumentManager">
            <argument type="service" id="couchdb.user_db" />
            <argument type="service" id="doctrine.odm.config" />
        </service>
        <service id="doctrine.odm.config" class="Doctrine\ODM\CouchDB\Configuration">
            <call method="setProxyDir"><argument>%doctrine.proxy_path%</argument></call>
            <call method="setMetadataDriverImpl"><argument type="service" id="doctrine.odm.annotation_driver" /></call>
            <call method="setMetadataCacheImpl"><argument type="service"><service class="%doctrine.metadata.cache.class%" /></argument></call>
            <call method="setLuceneHandlerName"><argument>%doctrine.lucene_handler_name%</argument></call>
        </service>
        <service id="doctrine.odm.annotation_driver" class="Doctrine\ODM\CouchDB\Mapping\Driver\AnnotationDriver">
            <argument type="service">
              <service class="Doctrine\Common\Annotations\AnnotationReader">
                <call method="setDefaultAnnotationNamespace"><argument>%doctrine.annotations.namespace%</argument></call>
                <call method="setIgnoreNotImportedAnnotations"><argument>%doctrine.annotations.ignoreUnknown%</argument></call>
              </service>
            </argument>
            <argument>%doctrine.document_paths%</argument>
        </service>

        <!-- COUCH_DB DATABASE CLIENTS -->
        <service id="couchdb.user_db" class="Doctrine\CouchDB\CouchDBClient">
            <argument type="service">
                <service class="Doctrine\CouchDB\HTTP\SocketClient">
                    <argument>%doctrine.database.host%</argument>
                    <argument>%doctrine.database.port%</argument>
                    <argument>%doctrine.database.username%</argument>
                    <argument>%doctrine.database.password%</argument>
                </service>
            </argument>
            <argument>%doctrine.database.name%</argument>
        </service>

    </services>
</container>
@ghost

ghost commented Sep 14, 2011

Thanks. :)

@4things did you use YAML or XML in your s2 config? I'm running into the same problem.

OIC... this being in src/Foo/BarBundle/Resources/config/services.xml

The calls in the Controller are then:

    $client = $this->container->get('couchdb.user_db');
    $documentManager = $this->container->get('users.doc_manager');        

But I'm still getting "Foo is not a valid document or mapped super class." Curses...

@ghost

ghost commented Oct 12, 2011

I opted to develop my own set of drivers and ODM. Was not getting any joy
from developers. Will have libs available by mid next month currently in
alpha state.
On Oct 12, 2011 6:58 PM, "Yitzchak Schaffer" <
reply@reply.github.com>
wrote:

The calls in the Controller are then:

   $client = $this->container->get('couchdb.user_db');
   $documentManager = $this->container->get('users.doc_manager');

But I'm still getting "Foo is not a valid document or mapped super class."
Curses...

Reply to this email directly or view it on GitHub:
#22 (comment)

Member

lsmith77 commented Oct 12, 2011

@4things: err could you explain why you have taken this step? is this lib flawed in your opinion? i have to admit that I do not have much time to work on couchdb odm atm .. and I think also @beberlei is quite busy with all the various things he has on his plate .. if the issue is just that PR's and issues are not being addressed in a timely fashion .. then we should rather look to increase the number of active devs than to splinter the resources across several libs.

@ghost

ghost commented Oct 14, 2011

@lsmith77: My reasons for building my own driver was purely because all the drivers currently available are either too difficult to configure, lack documentation or just don't work, and also I want to venture into building highly configurable applications. I would have loved to carry on with doctrine-odm, but yeah, the code base is to far ahead lots of catching up, easier building from ground up...

And also I true don't believe that a document data store needs a one to one class mapping (ODM) the whole idea of schema-less db is to have no restrictions. ODM with relation database like constrains = "DRDBMS", my understand was that is I add a new field to a form that captures product information that's it, the rest will figure its self out and plugin a rules engine in the equation and we get magic.

Member

lsmith77 commented Oct 14, 2011

Ah ok .. so you are not trying to provide the same abstraction at all. That being said you still might out Http lib useful:
https://github.com/doctrine/couchdb-odm/tree/master/lib/Doctrine/CouchDB

If necessary we could provide a subtree merge.

Owner

beberlei commented Oct 14, 2011

This issue was due to Doctrine common and CouchDB being out of sync and is fixed for some days now.

beberlei closed this Oct 14, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment