yasser777 / nettiers

Automatically exported from code.google.com/p/nettiers
0 stars 0 forks source link

EntityId is settable, but setting it does not change the associated fields in the entity data. #183

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

Given an entity named Entity, and a entity key class named EntityKey.

1. Construct a new instance of Entity.
2. Assign values to its primary key columns' properties.
3. Construct an instance of EntityKey with *different* values than those
used for the entity.
4. Assign the key from step 3 to Entity.EntityId
5. Examine the primary key columns' properties of the Entity.

What is the expected output? What do you see instead?

I expect the entity's fields to have their values updated because you
assigned a new key.

Instead, the old values remain.

What version of .netTiers and CodeSmith are you using?

netTiers 2.2.0.471, 2.2.0.603
CodeSmith 4.1, 5.0 

Please provide any additional information below.

Original issue reported on code.google.com by codemonk...@gmail.com on 25 Feb 2009 at 11:09

GoogleCodeExporter commented 9 years ago
Thank you for your feedback and taking the time to report this.  

I hadn't considered this scenario before and I was curious what your end goal 
would
be since arguably it may be  easier to just set the entity's property instead of
manipulating the key.

Thinking out loud here... Maybe entity keys were intended to be immutable and
therefore should not allow any changes to their underlying value.  Perhaps their
properties should be read only.  I have not taken a look to see what references 
the
key objcts to see where in the .netTiers framework these objects are 
manipulated.

Original comment by rhet...@gmail.com on 26 Feb 2009 at 12:54

GoogleCodeExporter commented 9 years ago
I forgot exactly why I was trying to do this. I do remember that the table I was
using when I found this bug had a three-column composite PK. At one point in 
coding,
I had a key all populated with my three columns, and I simply wanted to assign 
it to
an entity for some reason.

I originally submitted this bug over a year ago to the CodeSmith forums, and no 
one
ever replied. I ran into an old unit test I wrote to test this bug and tried it 
again
since I just updated the project to 603. Same issue. :)

Original comment by codemonk...@gmail.com on 26 Feb 2009 at 5:08

GoogleCodeExporter commented 9 years ago

Original comment by rhet...@gmail.com on 26 Feb 2009 at 6:00

GoogleCodeExporter commented 9 years ago
I agree, I think ReadOnly is the way to go.

jeff

Original comment by jmhin...@gmail.com on 4 Mar 2009 at 4:08

GoogleCodeExporter commented 9 years ago
I think it would be more robust if setting the key simply overwrote the 
associated
fields in the entity. However, there are other bugs associated with reassigning
entity keys.

For instance, this unit test fails.

[Test]
public void TestEntityKeyReassignment()
{
    MyEntity entity1 = new MyEntity();
    entity1.MyEntityID = 1;

    MyEntity entity2 = new MyEntity();
    entity2.MyEntityID = 100;

    entity2.EntityId = entity1.EntityId;

    Assert.AreSame( entity1.EntityId.Entity, entity1, "They Entity property of the
EntityID of entity1 is not entity1!" );
}

So if did you reassign the key, what key is the original entity supposed to have
after you assign it elsewhere? Null? A new default instance? What would the 
entity's
MyEntityID property be? Zero? (I guess whatever the default is when you 
construct an
entity would be ok.)

EntityId assignment is doable. Each key would need to have a 
RemoveFromCurrentEntity
method. The setter of the Entity class would have to call this method on the key
before assigning the key to itself. And during that method, the key would call a
method on the original entity to reset itself back to the default key. (By 
making a
new key instance and assigning it to itself.) This would eliminate the key 
sharing
problem the above unit test exposes.

Think of it as a guy stealing another guy's girlfriend. The polite thing for 
the girl
to do is break-up with the first guy before telling the world she's dating the 
new
guy. :D

Currently, there is at least some attempt to handle key reassignemnt. For 
instance,
the Entity property of the EntityKey gets changed to the new entity when you 
assign
the key to that new entity. Someone at least tried to make the keys 
reassignable.

I think ReadOnly is an ok choice, but all of the existing code that attempts to
handle key reassignment should be removed. Don't just remove the setter.

Original comment by codemonk...@gmail.com on 4 Mar 2009 at 6:20

GoogleCodeExporter commented 9 years ago
You are absolutely right, we should remove any irrelevant code that implies 
that keys
are re assignable.  After we remove the setter what's left?

"Someone at least tried to make the keys reassignable."
Respectfully, I disagree.  I do not see enough evidence to support that 
hypothesis. 
It looks like cross referencing to me.

Entity identity is crucial to maintaining data integrity.  I could not advocate 
key
reassignment because identity, conceptually, belongs to a single entity, it can 
not
be shared, transferred, etc. to other entities.

Original comment by rhet...@gmail.com on 4 Mar 2009 at 6:50

GoogleCodeExporter commented 9 years ago
I'm not certain I know what you mean by "cross referencing". Clearly, yes, the 
key
gets cross-referenced by both entities.

However, "...the Entity property of the EntityKey gets changed to the new 
entity when
you assign the key to that new entity."

Why would anyone add that code if they didn't intend for you to assign an 
entity key
to an entity?

Original comment by codemonk...@gmail.com on 4 Mar 2009 at 6:55

GoogleCodeExporter commented 9 years ago
Also need to keep in mind that we use this EntityKey when we use EntityTracking 
and 
EntityFactory.  Not sure what impact on caching this would have when updating 
the 
key.

Original comment by jmhin...@gmail.com on 4 Mar 2009 at 12:06

GoogleCodeExporter commented 9 years ago

Original comment by bniemyjski on 25 May 2009 at 3:30

GoogleCodeExporter commented 9 years ago

Original comment by bniemyjski on 25 May 2009 at 4:23