RegisterIdConvention bug - when property Id is present

[Felipe Ceotto]

Hi there.

I have used RegisterIdConvention to register a different id convention to the default, which told RavenDB to use a different field to create the Id but because the object already had an int property called Id it ignored my convention and used the default anyways.

I had expected that RegisterIdConvention would override the default behaviour so I have considered this one a bug. In the end I had to rename my Id property to something else.

Has anybody had the same issue and if so, how have you gotten around it?

Cheers,
Felipe.

[Oren Eini (Ayende Rahien)]

Can you create a failing test?

--
You received this message because you are subscribed to the Google Groups "RavenDB - 2nd generation document database" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ravendb+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Felipe Ceotto]

Sure, just give me a few minutes. :)

[Felipe Ceotto]

Hello again,

Please see attached a failing test. Mind the TODO where you should replace the URL for your own instance of RavenDB. I didn't create an Embedded version but will do that shortly if that would be better.

Cheers,
Felipe.

On Friday, 16 May 2014 12:23:07 UTC+1, Oren Eini wrote:

[Felipe Ceotto]

I probably should have posted the code directly as well, so here you go:

namespace RavenDB_RegisterIdConvention_FailingTest

{

    using System;

    using System.Collections.Generic;

    using NUnit.Framework;

    using Raven.Client;

    using Raven.Client.Document;

    /// <summary>

    /// Represents my class with an id property.

    /// </summary>

    public class MyClassWithIdProperty

    {

        /// <summary>

        /// Gets or sets the id.

        /// </summary>

        /// <value>

        /// The id.

        /// </value>

        public int Id { get; set; }

        /// <summary>

        /// Gets or sets the reference.

        /// </summary>

        /// <value>

        /// The reference.

        /// </value>

        public string Reference { get; set; }

        /// <summary>

        /// Gets or sets the creation time.

        /// </summary>

        /// <value>

        /// The creation time.

        /// </value>

        public DateTime CreationTime { get; set; }

    }

    /// <summary>

    /// Represents the unit tests for register id convention.

    /// </summary>

    [TestFixture]

    public class RegisterIdConventionTest

    {

        private IDocumentStore _documentStore;

        /// <summary>

        /// Initializes each test.

        /// </summary>

        [SetUp]

        public void Init()

        {

            // TODO: Replace URL below with your own instance of RavenDB.

            _documentStore = new DocumentStore { Url = "http://localhost:8082", DefaultDatabase = "Test" };

            _documentStore.Initialize();

            _documentStore.Conventions.RegisterIdConvention<MyClassWithIdProperty>((dbName, commands, myClassWithIdProperty) => "myClasses/" + myClassWithIdProperty.Reference);

        }

        /// <summary>

        /// Finalises each test.

        /// </summary>

        [TearDown]

        public void TearDown()

        {

            _documentStore.Dispose();

        }

        /// <summary>

        /// Tests that the stored id is as expected.

        /// </summary>

        [Test]

        public void TestStoredIdIsCorrect()

        {

            MyClassWithIdProperty myClassWithIdProperty = new MyClassWithIdProperty { Id = 1, Reference = Guid.NewGuid().ToString(), CreationTime = DateTime.Now };

            string supposedId = "myClasses/" + myClassWithIdProperty.Reference;

            string documentId;

            using (IDocumentSession documentSession = _documentStore.OpenSession())

            {

                documentSession.Store(myClassWithIdProperty);

                documentSession.SaveChanges();

                documentId = documentSession.Advanced.GetDocumentId(myClassWithIdProperty);

            }

            Assert.AreEqual(supposedId, documentId, "Retrieved id was incorrect.");

        }

    }

}

[Felipe Ceotto]

I'm sorry to bother again, but please see attached a test with Embedded, also failing.

I added a test with a class that does not have an Id property and that one passes as expected.

Code below:

namespace RavenDB_RegisterIdConvention_FailingTest

{

    using System;

    using System.Collections.Generic;

    using NUnit.Framework;

    using Raven.Client;

    using Raven.Client.Embedded;

    /// <summary>

    /// Represents the unit tests for register id convention.

    /// </summary>

    [TestFixture]

    public class RegisterIdConventionTest

    {

        private IDocumentStore _documentStore;

        /// <summary>

        /// Initializes each test.

        /// </summary>

        [SetUp]

        public void Init()

        {

            if ((null != _documentStore) && !_documentStore.WasDisposed)

            {

                return;

            }

            _documentStore = new EmbeddableDocumentStore { DataDirectory = "Data", UseEmbeddedHttpServer = false };

            _documentStore.Initialize();

            _documentStore.Conventions.RegisterIdConvention<MyClassWithIdProperty>((dbName, commands, myClassWithIdProperty) => "myClasses/" + myClassWithIdProperty.Reference);

            _documentStore.Conventions.RegisterIdConvention<MyClassWithoutIdProperty>((dbName, commands, myClassWithIdProperty) => "myClassesNoId/" + myClassWithIdProperty.Reference);

        }

        /// <summary>

        /// Finalises each test.

        /// </summary>

        [TearDown]

        public void TearDown()

        {

            _documentStore.Dispose();

        }

        /// <summary>

        /// Tests that the stored id is as per registered convention when given object has a property called id.

        /// </summary>

        [Test]

        public void TestStoredIdIsCorrectWithConventionAndIdProperty()

        {

            MyClassWithIdProperty myClassWithIdProperty = new MyClassWithIdProperty { Id = 1, Reference = Guid.NewGuid().ToString(), CreationTime = DateTime.Now };

            string supposedId = "myClasses/" + myClassWithIdProperty.Reference;

            string documentId;

            using (IDocumentSession documentSession = _documentStore.OpenSession())

            {

                documentSession.Store(myClassWithIdProperty);

                documentSession.SaveChanges();

                documentId = documentSession.Advanced.GetDocumentId(myClassWithIdProperty);

            }

            Assert.AreEqual(supposedId, documentId, "Retrieved id was incorrect.");

        }

        /// <summary>

        /// Tests that the stored id is as per registered convention when given object does not have a property called id.

        /// </summary>

        [Test]

        public void TestStoredIdIsCorrectWithConventionAndWithoutIdProperty()

        {

            MyClassWithoutIdProperty myClassWithIdProperty = new MyClassWithoutIdProperty { Reference = Guid.NewGuid().ToString(), CreationTime = DateTime.Now };

            string supposedId = "myClassesNoId/" + myClassWithIdProperty.Reference;

            string documentId;

            using (IDocumentSession documentSession = _documentStore.OpenSession())

            {

                documentSession.Store(myClassWithIdProperty);

                documentSession.SaveChanges();

                documentId = documentSession.Advanced.GetDocumentId(myClassWithIdProperty);

            }

            Assert.AreEqual(supposedId, documentId, "Retrieved id was incorrect.");

        }

    }

    /// Represents my class without an id property.

    /// </summary>

    public class MyClassWithoutIdProperty

    {

        /// <summary>

        /// Gets or sets the reference.

        /// </summary>

        /// <value>

        /// The reference.

        /// </value>

        public string Reference { get; set; }

        /// <summary>

        /// Gets or sets the creation time.

        /// </summary>

        /// <value>

        /// The creation time.

        /// </value>

        public DateTime CreationTime { get; set; }

    }

}

On Friday, 16 May 2014 12:23:07 UTC+1, Oren Eini wrote:

[Oren Eini (Ayende Rahien)]

You need to also setup the what is the id property it will use, in this scenario.

See FindIdentityProperty 

Oren Eini

CEO

Mobile: + 972-52-548-6969

Office:  + 972-4-622-7811

Fax:      + 972-153-4622-7811

[Felipe Ceotto]

Hello again.

I changed my initialisation to include the following:

    _documentStore.Conventions.FindIdentityProperty = info => info.Name == "Reference";

This works but in a different way than I expected as the returned id ignores the prefix. The Ids should have been according to these conventions:

            _documentStore.Conventions.RegisterIdConvention<MyClassWithIdProperty>((dbName, commands, myClassWithIdProperty) => "myClasses/" + myClassWithIdProperty.Reference);

            _documentStore.Conventions.RegisterIdConvention<MyClassWithoutIdProperty>((dbName, commands, myClassWithIdProperty) => "myClassesNoId/" + myClassWithIdProperty.Reference);

But the tests are failing:

  Retrieved id was incorrect.

  Expected string length 46 but was 36. Strings differ at index 0.

  Expected: "myClasses/def8462a-b6fe-4f98-9c72-27c987a25b79"

  But was:  "def8462a-b6fe-4f98-9c72-27c987a25b79"

and

  Retrieved id was incorrect.

  Expected string length 50 but was 36. Strings differ at index 0.

  Expected: "myClassesNoId/ff555f37-6f9a-4d69-bf53-fbe80bf7dc60"

  But was:  "ff555f37-6f9a-4d69-bf53-fbe80bf7dc60"

As you can see, the GUIDs are actually the same, only the prefixes are not, whereas before, without the FindIdentityProperty, the second test passed as the ID returned was like this:

  Expected: "myClassesNoId/ff555f37-6f9a-4d69-bf53-fbe80bf7dc60"

  Was:  "myClassesNoId/ff555f37-6f9a-4d69-bf53-fbe80bf7dc60"

What would the explanation for that be? If using FindIdentityProperty will the ID conventions be ignored?

Thanks!

[Michael Yarichuk]

Hi,

Can you share code - how your convention initialization looks now?

Best regards,

 

Michael Yarichuk

RavenDB Core Team

Tel: 972-4-6227811

Fax:972-153-4-6227811

Email : michael....@hibernatingrhinos.com

 

[Felipe Ceotto]

Sure, here you go:

            _documentStore = new EmbeddableDocumentStore { DataDirectory = "Data", UseEmbeddedHttpServer = false };

            _documentStore.Initialize();

            _documentStore.Conventions.FindIdentityProperty = info => info.Name == "Reference";

            _documentStore.Conventions.RegisterIdConvention<MyClassWithIdProperty>((dbName, commands, myClassWithIdProperty) => "myClasses/" + myClassWithIdProperty.Reference);

            _documentStore.Conventions.RegisterIdConvention<MyClassWithoutIdProperty>((dbName, commands, myClassWithIdProperty) => "myClassesNoId/" + myClassWithIdProperty.Reference);

See attached for full solution.

Cheers!

Felipe.

[Bjørn Moe]

Did this ever get resolved? I can't get this to work either. When loading or saving documents, the delegates i send to RegisterIdConvention never gets called at all.

[Oren Eini (Ayende Rahien)]

Can you share a failing test?

Hibernating Rhinos Ltd  

Oren Eini l CEO l Mobile: + 972-52-548-6969

Office: +972-4-622-7811 l Fax: +972-153-4-622-7811

 

On Thu, Sep 18, 2014 at 7:47 PM, Bjørn Moe <bjor...@gmail.com> wrote:

Did this ever get resolved? I can't get this to work either. When loading or saving documents, the delegates i send to RegisterIdConvention never gets called at all.

--

[Bjørn Moe]

Did som more research. Felipe Ceotto latest example (in zip) illustrates it:

If 

_documentStore.Conventions.FindIdentityProperty = info => info.Name == "Reference";

is there, the Felipe Ceotto's tests fails, the delegates within RegisterIdConvention never gets called. In the ravenDB source code in GenerateEntityIdOnTheCilent.cs:

public string GetOrGenerateDocumentKey(object entity)

{

string id;

TryGetIdFromInstance(entity, out id);

if (id == null)

{

// Generate the key up front

id = generateKey(entity);

}

The method TryGetIdFromInstance sets the Id because of the FindIdentityProperty configuration, and "generateKey" never gets called. If we for example set 

_documentStore.Conventions.FindIdentityProperty = info => info.Name == "ExistsNowhere";

then TryGetIdFromInstance can not find the id, and id = generateKey(entity); gets called, which results in our RegisterIdConvention methods getting called and working properly.

Is this the intended behaviour? It seems to me very counter-intuitive. The ID Conventions should override any default id-finding method I'd think?

[Oren Eini (Ayende Rahien)]

Do you have a value in the Reference property?

Hibernating Rhinos Ltd  

Oren Eini l CEO l Mobile: + 972-52-548-6969

Office: +972-4-622-7811 l Fax: +972-153-4-622-7811

 

--

[Bjørn Moe]

Yes, ref. source, there is a value there. That should not matter though, the delegates provided to RegisterIdConvention should have been called either way. The problem is that RegisterIdConvention() does nothing when RavenDB is able to find any kind of identity property, either the default "Id" or as in the example above, a property which name is "Reference".

-Bjørn Moe

[Oren Eini (Ayende Rahien)]

Yes, that is by design. If there is a value in the property, it already _has_ a value, we only call the delegates if you have null there.

[Bjørn Moe]

OK, thanks a lot for your help! :-) I think it would help if the docs were a bit clearer about this point though... :-)

By the way, upon loading, we have the FindFullDocumentKeyFromNonStringIdentifier method to enable us to do for example session.Load<Corvus>(22), resulting in a document id  "hello/corvus/22" for example. This works, but requires me to create my whole own system if we want multiple independent classes (or possibly assemblies) to each be able to configure their ID conventions (since we cannot "append" to anything on the store.Conventions, like we can with RegisterIdConvention). How about something like:

_docStore.Conventions.RegisterIdLoadConvention<Corvus,int>(theInt => "hello/corvus/" + theInt);

Which could be used by DefaultFindFullDocumentKeyFromNonStringIdentifier in DocumentConvention.cs. I tested this on the 2.5 version of raven, you can see it here: https://github.com/cocytus/ravendb/commit/bc0e67af9891df62b0761cf4f1c7882d2d47f41e . Would this be interesting to add to RavenDB?

-Bjørn Moe

[Oren Eini (Ayende Rahien)]

A Pull Request for that would be great.

[Bjørn Moe]

Created at https://github.com/ayende/ravendb/pull/1415 .

--
You received this message because you are subscribed to a topic in the Google Groups "RavenDB - 2nd generation document database" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/ravendb/Rz5RbBw4MzE/unsubscribe.
To unsubscribe from this group and all its topics, send an email to ravendb+u...@googlegroups.com.

[Oren Eini (Ayende Rahien)]

Do you have a CLA with this?

ravendb.net/contributing

[Bjørn Moe]

Just sent!