Wrong Primary key returned in AuditEntityFieldSet(IEntityCore entity, int fieldIndex, object originalValue)

Posts   
 
    
Ammit
User
Posts: 59
Joined: 19-May-2006
# Posted on: 03-Nov-2007 10:14:12   

HI

I am using LLBLGen Runtime version 2.5.0.0 File Version 2.5.7.1005 with self servicing , DB is Sql Server 2005.

I am implemening feature for audit logging. I have implemented in slightly different way due to some XYZ reasons..

I am using AuditInsertOfNewEntity(IEntityCore entity) call to record new entity creation log. i can not use Set

code inside my AuditInsertOfNewEntity to log new entity generation log


 //Looping through list of Entity Fields and setting newly created AuditDetail entities
                    for (int jj = 0; jj < ((IEntity)entity).Fields.Count; jj++)
                    {
                        AuditDetailEntity auditDetail = new AuditDetailEntity();
                        auditDetail.ColumnNane = ((IEntity)entity).Fields[jj].Name;
                        auditDetail.OldValue = null;
                        
                        //Ignore NULL entities Do not log them
                        if (entity.GetCurrentFieldValue(jj) == null)
                        {
                            auditDetail.NewValue = null;
                        }
                        else
                        {
                            auditDetail.NewValue = entity.GetCurrentFieldValue(jj).ToString();
                            auditEntry.AuditDetails.Add(auditDetail);
                        }
                    }
                    transaction.AuditEntryEntities.Add(auditEntry);

I am using AuditEntityFieldSet(IEntityCore entity, int fieldIndex, object originalValue) to log the Update log



// avoid to audit into AuditTransactionEntity (this would create an overflow effect). This is necessary if this auditor is injected into 
                    // all entities, thus also in the AuditTransactionEntity
                    if (entity is AuditTransactionEntity)
                    {
                        throw new InvalidInjectionException("Can not Inject AuditTransactionEntity.");
                    }
                    //This will be used to track down Updated Entities
                    if (!entity.IsNew)
                    {
                        AuditDetailEntity auditDetail = new AuditDetailEntity();
                        // used to store the change experimented by a field.
                        string originalValueAsString = string.Empty;
                        string currentValue = string.Empty;

                        // sets VALUE OR NULL for originalValue and uses it as string.
                        if (originalValue != null)
                        {
                            originalValueAsString = originalValue.ToString();
                        }
                        else
                        {
                            originalValueAsString = null;
                        }

                        // sets VALUE OR NULL for currentValue and uses it as string.
                        if (entity.GetCurrentFieldValue(fieldIndex) != null)
                        {
                            currentValue = entity.GetCurrentFieldValue(fieldIndex).ToString();
                        }
                        else
                        {
                            currentValue = null;
                        }
                        auditDetail.ColumnNane = ((IEntity)entity).Fields[fieldIndex].Name;
                        auditDetail.OldValue = originalValueAsString;
                        auditDetail.NewValue = currentValue;
                        int count = findEntity(entity.LLBLGenProEntityName, transaction.AuditEntryEntities, GetPrimaryKeyInfoFromEntity(entity));
                        
                        //Create a new entity
                        if(count == -1)
                        {
                            AuditEntryEntity auditEntry = new AuditEntryEntity();
                            auditEntry.AuditEntryTypeId = (int)AuditEntryTypeId.Update;
                            auditEntry.EntityType = entity.LLBLGenProEntityName;
                            auditEntry.EntityId = long.Parse(AuditorHelper.GetPrimaryKeyInfoFromEntity(entity));
                            auditEntry.AuditDetails.Add(auditDetail);
                            transaction.AuditEntryEntities.Add(auditEntry);
                        }
                        //Entry Already Exists insest the AuditDetails at given count
                        else
                        {
                            transaction.AuditEntryEntities[count].AuditDetails.Add(auditDetail);
                        }
                    }


code to get primary key is taken from your samples that goes as follows


public static string GetPrimaryKeyInfoFromEntity(IEntityCore entity)
        {
            try
            {
                // gets primary key fields
                List<IEntityField> pkFields = ((IEntity)entity).PrimaryKeyFields;

                if (pkFields.Count > 1)
                {
                    throw new InvalidPrimaryKeyException(pkFields.Count +" PrimaryKeys found for "+entity.LLBLGenProEntityName);
                }
                // collect PK fields if the entity isn't new
                string strPKEntityInfo = string.Empty;
                if (!entity.IsNew)
                {
                    return pkFields[0].CurrentValue.ToString();
                }

                // returns PK info
                return strPKEntityInfo;
            }
            catch(Exception ex)
            {
                throw ex;
            }
        }

things are working fine except in one scenario.. Consider this

1) I am creating entity X code goes like this. creating entity for the first time i wond log these changes as in my AuditEntityFieldSet implementation i am checking for if (!entity.IsNew) because i want to record them in inserNet call i have impelemtned...

X.a = 10; X.b = 15; X.c = 25;

but now if i have following line of code in place [which i actually do have]

X.b = 100;

I fall in to AuditEntityFieldSet and if (!entity.IsNew) is true as somehoe LLBLGen treats this as an existing entity though i havent done X.save() yet. i can still live with this as this is a genuine update nad it does give me all the operations being performed on an entity... but funny part of the store is for thios update when i try to find primary key for the same to log them it gives me back PriMary Key of some other existing record for entity X in the DB. due to that this change gets recorded against that entity instead of this newly created entity and that sort of screws lots of things off on our end.

Can you please let me know what should i do to overcom this issue?

I tried few alternates to overcome this issue as well e.g. I disabled this AuditEntityFieldSet and tried capturing changes in UpdateEntity call but AuditUpdateOfExistingEntity(IEntityCore entity) there is no way i can figure out the OLD Value of the entity and log it .. as i observed in AuditEntityFieldSet call LLBLGen does send me the Original Value as object.. I can not take this work around as well...

Can you please let me know why Proimarykey finction is giving me wrong primary key? if it is by design of a potential bug then can you also tell me the alternate solution to address this problem?

Thanks Ammit

daelmo avatar
daelmo
Support Team
Posts: 8245
Joined: 28-Nov-2005
# Posted on: 04-Nov-2007 20:06:18   

Ammit wrote:

HI



// avoid to audit into AuditTransactionEntity (this would create an overflow effect). This is necessary if this auditor is injected into 
                    // all entities, thus also in the AuditTransactionEntity
                    if (entity is AuditTransactionEntity)
                    {
                        throw new InvalidInjectionException("Can not Inject AuditTransactionEntity.");
                    }
...

You don't need to throw an exception, just ignore that entity (use RETURN or an IF block.

Ammit wrote:

code to get primary key is taken from your samples that goes as follows


public static string GetPrimaryKeyInfoFromEntity(IEntityCore entity)
        {
            try
            {
                // gets primary key fields
                List<IEntityField> pkFields = ((IEntity)entity).PrimaryKeyFields;

                if (pkFields.Count > 1)
                {
                    throw new InvalidPrimaryKeyException(pkFields.Count +" PrimaryKeys found for "+entity.LLBLGenProEntityName);
                }
                // collect PK fields if the entity isn't new
                string strPKEntityInfo = string.Empty;
                if (!entity.IsNew)
                {
                    return pkFields[0].CurrentValue.ToString();
                }

                // returns PK info
                return strPKEntityInfo;
            }
            catch(Exception ex)
            {
                throw ex;
            }
        }

Correct if I'm wrong but if the entity is new, that method will return always "" (string.Empy).

Ammit wrote:

int count = findEntity(entity.LLBLGenProEntityName, transaction.AuditEntryEntities, GetPrimaryKeyInfoFromEntity(entity));

                    //Create a new entity
                    if(count == -1)
                    {

Could you post the code snippet of the _findEntity _method?

Also, could you post your complete Auditor class, coz I'm lost in some parts of the code.

Regards.

David Elizondo | LLBLGen Support Team
Ammit
User
Posts: 59
Joined: 19-May-2006
# Posted on: 05-Nov-2007 05:54:50   

Hi Nick,

Throwing InvalidInjectionException is by design and not by mistake. We have some internal arrengements which forces me to throw such exceptions.. so i think we cna ignore this for now..

And primary key function returns me Primary key of some other record... that is what the problem is i am also expecting "" but it returns me primary key of some other entity. that is why i am stuck

I am attaching code file along with this email I am not attacheing the actual auditor implementation beacuase it just calls this static method i have written e.g. public override void AuditEntityFieldSet(IEntityCore entity, int fieldIndex, object originalValue) { AuditorHelper.AuditEntityFieldSet(entity,fieldIndex,originalValue); }

i am sending you the implementation of this AuditorHelper class that handles everything..

daelmo wrote:

Ammit wrote:

HI



// avoid to audit into AuditTransactionEntity (this would create an overflow effect). This is necessary if this auditor is injected into 
                    // all entities, thus also in the AuditTransactionEntity
                    if (entity is AuditTransactionEntity)
                    {
                        throw new InvalidInjectionException("Can not Inject AuditTransactionEntity.");
                    }
...

You don't need to throw an exception, just ignore that entity (use RETURN or an IF block.

Ammit wrote:

code to get primary key is taken from your samples that goes as follows


public static string GetPrimaryKeyInfoFromEntity(IEntityCore entity)
        {
            try
            {
                // gets primary key fields
                List<IEntityField> pkFields = ((IEntity)entity).PrimaryKeyFields;

                if (pkFields.Count > 1)
                {
                    throw new InvalidPrimaryKeyException(pkFields.Count +" PrimaryKeys found for "+entity.LLBLGenProEntityName);
                }
                // collect PK fields if the entity isn't new
                string strPKEntityInfo = string.Empty;
                if (!entity.IsNew)
                {
                    return pkFields[0].CurrentValue.ToString();
                }

                // returns PK info
                return strPKEntityInfo;
            }
            catch(Exception ex)
            {
                throw ex;
            }
        }

Correct if I'm wrong but if the entity is new, that method will return always "" (string.Empy).

Ammit wrote:

int count = findEntity(entity.LLBLGenProEntityName, transaction.AuditEntryEntities, GetPrimaryKeyInfoFromEntity(entity));

                    //Create a new entity
                    if(count == -1)
                    {

Could you post the code snippet of the _findEntity _method?

Also, could you post your complete Auditor class, coz I'm lost in some parts of the code.

Regards.

Attachments
Filename File size Added on Approval
AuditorHelper.cs 14,852 05-Nov-2007 05:55.52 Approved
Walaa avatar
Walaa
Support Team
Posts: 14950
Joined: 21-Aug-2005
# Posted on: 05-Nov-2007 10:04:18   

Would you please attach a simple small repro solution based on a common database like Northwind?

Thanks for your co-operation.

Ammit
User
Posts: 59
Joined: 19-May-2006
# Posted on: 05-Nov-2007 10:11:29   

Walaa wrote:

Would you please attach a simple small repro solution based on a common database like Northwind?

Thanks for your co-operation.

I will try doing it if i happen to find time today, Could you reproduce the issue at your end? I think i have written a very good description of how to reproduce the problem and even gave you my source file I will definitely try providing you sample with NorthWind sometime later...may be tomorrow... Meanwhile if you can try reproducing the issue on your end.,, it will be big help for me...

Craete any entity. before save reassign some other valoue to that entity.. and try saving it..

Thanks Ammit

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 05-Nov-2007 11:41:24   

You're using a static method, though INSIDE the static method you're storing entity objects. This means that all threads are using the same in-memory store. This can lead to concurrency issues (threading issues). I'm not sure what the app does, but if it's a webapp, you do have multiple threads and therefore multiple threads calling the same code.

So rule of thumb: store the audit entities in the auditor object itself OR use a lock mechanism but that's hurting performance and not necessary as you can store the entities in the auditor.

Frans Bouma | Lead developer LLBLGen Pro
Ammit
User
Posts: 59
Joined: 19-May-2006
# Posted on: 05-Nov-2007 11:57:07   

Otis wrote:

You're using a static method, though INSIDE the static method you're storing entity objects. This means that all threads are using the same in-memory store. This can lead to concurrency issues (threading issues). I'm not sure what the app does, but if it's a webapp, you do have multiple threads and therefore multiple threads calling the same code.

So rule of thumb: store the audit entities in the auditor object itself OR use a lock mechanism but that's hurting performance and not necessary as you can store the entities in the auditor.

Hey Otic nice to point this out..but that has been take care of If you read the code i have attached it says

Base.Transaction transaction = Base.Transaction.Current; where ihave mechanism of returning this Transaction object back.. This code will get executed in a web method call and thie transaction object is cached per thread id structure so there is no way i am going to have issues ... TransactionObject has these AuditEntiry[] that further consists AuditDetail.. Looks like i am safe there..

Thanks for pointing this to me thought

Can you please throw some more light on the issue i have raised? Is it my code, is it DB or is it the LLBLGen methods , returing me wrong Primary Key?

sparcler
User
Posts: 7
Joined: 25-Oct-2007
# Posted on: 06-Nov-2007 13:47:46   

Otis: so you are saying that a function as the one below whould not work as intended in a web environment because of multiple threads and concurrency?


public class StatusHelper
{
   public static void SetStatus(int newStatus)
   {
       StatusEntity status = new StatusEntity();

       status.Value = newStatus;
       status.Save();
   }
}

Ammit
User
Posts: 59
Joined: 19-May-2006
# Posted on: 06-Nov-2007 15:15:26   

thats is what i am keen on knowing as well BTW i am stillw aiting for the response on wrong Primary key being returned.. i am done with my basic test cases and coding.. so i may send you a samplke with northwind tomorrow.. BTW if you can very quickly check the same scenario i have been asking about , then it will be a great help

thanks Ammit

sparcler wrote:

Otis: so you are saying that a function as the one below whould not work as intended in a web environment because of multiple threads and concurrency?


public class StatusHelper
{
   public static void SetStatus(int newStatus)
   {
       StatusEntity status = new StatusEntity();

       status.Value = newStatus;
       status.Save();
   }
}

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 06-Nov-2007 17:17:27   

sparcler wrote:

Otis: so you are saying that a function as the one below whould not work as intended in a web environment because of multiple threads and concurrency?


public class StatusHelper
{
   public static void SetStatus(int newStatus)
   {
       StatusEntity status = new StatusEntity();

       status.Value = newStatus;
       status.Save();
   }
}

Yes it will work because objects created INSIDE a static method which aren't stored outside the method are thread safe. So every thread ending up in SetStatus will create a new status entity and doesn't run into threading issues.

However if you store 'status' somewhere globally, and other threads can access that, it might be a threading issue.

Ammit: you still are liable to threading issues if you don't use a lock. Every thread ends up in the same code. So if you call a method which will return to you an object that's already alive, that call needs to be atomic or it will be a problem in some cases.

That's also why I suggested to you to keep the elements stored in teh auditor and not in a shared instance. For code it's not important: you can also place the static methods in the auditor class, it's not really a benefit to have static code in your situation.

(also keep an eye on the catch (exception ex) { throw ex; } statements: these destroy your call stack, as it will originate at the line of throw ex; not on the line where ex was thrown. So if you use that kind of code, simply remove try/catch completely. )

Frans Bouma | Lead developer LLBLGen Pro
Ammit
User
Posts: 59
Joined: 19-May-2006
# Posted on: 06-Nov-2007 17:22:06   

Perfetct that is why i have those helper methods declared as static and the data persistance is happening in thread safe manner BTW can someone quickly confirm that it is issue with my code or something thati have missed during LLBLGen auditing implementation i have sent code and in the previois posts i have illustrated well in detail where and how the problem is arising?\

thanks Ammit

Otis wrote:

sparcler wrote:

Otis: so you are saying that a function as the one below whould not work as intended in a web environment because of multiple threads and concurrency?


public class StatusHelper
{
   public static void SetStatus(int newStatus)
   {
       StatusEntity status = new StatusEntity();

       status.Value = newStatus;
       status.Save();
   }
}

Yes it will work because objects created INSIDE a static method which aren't stored outside the method are thread safe. So every thread ending up in SetStatus will create a new status entity and doesn't run into threading issues.

However if you store 'status' somewhere globally, and other threads can access that, it might be a threading issue.

Ammit: you still are liable to threading issues if you don't use a lock. Every thread ends up in the same code. So if you call a method which will return to you an object that's already alive, that call needs to be atomic or it will be a problem in some cases.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 06-Nov-2007 17:25:16   

Ammit wrote:

Perfetct that is why i have those helper methods declared as static and the data persistance is happening in thread safe manner BTW can someone quickly confirm that it is issue with my code or something thati have missed during LLBLGen auditing implementation i have sent code and in the previois posts i have illustrated well in detail where and how the problem is arising?\

I'm not sure you're not entirely safe. It's IMHO not an advantage to have the audit code declared static, because it's working on stateful data. That's a red flag: static code should be operating on stateless data, but an auditor is a recorder, it records data, so the auditor is the place where the data is stored and therefore the audit code is better placed there.

So I want to ask you: place the code back in the auditor and try again.

I don't fully understand your last sentence: did you mail a project/code ?

Frans Bouma | Lead developer LLBLGen Pro
Ammit
User
Posts: 59
Joined: 19-May-2006
# Posted on: 06-Nov-2007 20:33:48   

Otis wrote:

Ammit wrote:

Perfetct that is why i have those helper methods declared as static and the data persistance is happening in thread safe manner BTW can someone quickly confirm that it is issue with my code or something thati have missed during LLBLGen auditing implementation i have sent code and in the previois posts i have illustrated well in detail where and how the problem is arising?\

I'm not sure you're not entirely safe. It's IMHO not an advantage to have the audit code declared static, because it's working on stateful data. That's a red flag: static code should be operating on stateless data, but an auditor is a recorder, it records data, so the auditor is the place where the data is stored and therefore the audit code is better placed there.

So I want to ask you: place the code back in the auditor and try again.

I don't fully understand your last sentence: did you mail a project/code ?

Yes i have attached the my code in one of my earlier posts. can you please go through the issue i have raised earlier and see if something is wrong in the implementation or it is by design that i am getting some wrong primary key for newly created entity?

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 06-Nov-2007 21:54:39   

Ammit wrote:

Otis wrote:

Ammit wrote:

Perfetct that is why i have those helper methods declared as static and the data persistance is happening in thread safe manner BTW can someone quickly confirm that it is issue with my code or something thati have missed during LLBLGen auditing implementation i have sent code and in the previois posts i have illustrated well in detail where and how the problem is arising?\

I'm not sure you're not entirely safe. It's IMHO not an advantage to have the audit code declared static, because it's working on stateful data. That's a red flag: static code should be operating on stateless data, but an auditor is a recorder, it records data, so the auditor is the place where the data is stored and therefore the audit code is better placed there.

So I want to ask you: place the code back in the auditor and try again.

I don't fully understand your last sentence: did you mail a project/code ?

Yes i have attached the my code in one of my earlier posts. can you please go through the issue i have raised earlier and see if something is wrong in the implementation or it is by design that i am getting some wrong primary key for newly created entity?

The AditorHelper code you mean?

I already looked at it, and that's why I gave my advice. It's not by design you get wrong data back of course, however as you use static methods which refer to shared stores without a lock, you will likely run into a threading issue, so hence my advice to you to move the code to the auditor instead and drop the static methods (not necessary anyway), and see if that solves your problem.

Frans Bouma | Lead developer LLBLGen Pro