Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with Auditing #8

Closed
markchipman opened this issue May 26, 2016 · 13 comments
Closed

Problem with Auditing #8

markchipman opened this issue May 26, 2016 · 13 comments

Comments

@markchipman
Copy link

@markchipman markchipman commented May 26, 2016

I've got a pretty funky thing going on here...

First what works -- creating new records and deleting from my dbcontext will yield correct entries into the both the AuditEntries and AuditEntryProperties tables. This is good and expected.

NOTE that the data from the class that is being modified is indeed saving all field values into the database table correctly. Again this is very good... it's just the saving of the audit data that has an issue...

What doesn't work -- When I update a recordset, only the primary key value is being saved to the AuditEntryProperties table and NO OTHER changed fields are being saved in that table. Oddly, the PK doesn't change yet it does show up as a new row in the table, but I think it should not be showing up there anyways (I have this setting: AuditManager.DefaultConfiguration.IgnorePropertyUnchanged = true). At the end of this write-up you'll see what I'm getting in the AuditEntryProperties table... you can see that a new record is added, then subsequently the same record is edited (the changed entity properties ARE NOT being written for some reason), and lastly I deleted the record and that works as well. You can tell what properties got updated from the 'oldvalue' column of the delete set of records.

Here is my setup:

I have about 50 fields in my table, but this is a jist of what it is defined as mostly...

CREATE TABLE [dbo].[ClientPricing](
[RecId] [int] IDENTITY(1,1) NOT NULL,
[CustId] nvarchar NOT NULL,
[CustomerName] nvarchar NULL,
[StandardOrderPrice] [decimal](18, 2) NULL,
[StandardLinePrice] [decimal](18, 2) NULL,
[StandardUnitPrice] [decimal](18, 2) NULL,
[MassMailOrderPrice] [decimal](18, 2) NULL,
[MassMailLinePrice] [decimal](18, 2) NULL,
[MassMailUnitPrice] [decimal](18, 2) NULL,
[BulkOrderPrice] [decimal](18, 2) NULL,
[BulkLinePrice] [decimal](18, 2) NULL,
[BulkUnitPrice] [decimal](18, 2) NULL,
[PalletPrice] [decimal](18, 2) NULL,
[CasePrice] [decimal](18, 2) NULL,
[OuterPrice] [decimal](18, 2) NULL,
[InnerPrice] [decimal](18, 2) NULL,
[EachPrice] [decimal](18, 2) NULL ......

And this is what the DBContext class looks like...

    public WebApplication4Context() : base("name=WebApplication4Context")
    {
    }

    public DbSet<ClientPricing> ClientPricing { get; set; }
    public DbSet<AuditEntry> AuditEntries { get; set; }
    public DbSet<AuditEntryProperty> AuditEntryProperties { get; set; }

    protected override void OnModelCreating(DbModelBuilder modelBuilder)
    {
        // See: Entity Framework Code First Automatic Migration & Existing Tables ==> https://blog.rajsoftware.com/2014/07/12/entity-framework-code-first-automatic-migration-existing-tables/
        modelBuilder.Conventions.Remove<PluralizingTableNameConvention>();

        base.OnModelCreating(modelBuilder); // DON'T REMOVE THIS!!!

        AuditManager.DefaultConfiguration.AutoSavePreAction = (context, audit) =>
        {
            // ADD "Where(x => x.AuditEntryID == 0)" to allow multiple SaveChanges with same Audit
            (context as WebApplication4Context).AuditEntries.AddRange(audit.Entries);
        };

        AuditManager.DefaultConfiguration.Exclude(x => true); // Exclude ALL
        AuditManager.DefaultConfiguration.Include<IAuditable>();
        AuditManager.DefaultConfiguration.IgnorePropertyUnchanged = true;
    }

    public override int SaveChanges()
    {
        var currentUser = System.Threading.Thread.CurrentPrincipal.Identity.Name;

        var audit = new Audit();
        audit.CreatedBy = string.IsNullOrEmpty(currentUser) ? "System" : currentUser;
        audit.PreSaveChanges(this);
        var rowsAffected = base.SaveChanges();
        audit.PostSaveChanges();

        if (audit.Configuration.AutoSavePreAction != null)
        {
            audit.Configuration.AutoSavePreAction(this, audit);
            base.SaveChanges();
        }

        return rowsAffected;
    }

    public override Task<int> SaveChangesAsync()
    {
        return SaveChangesAsync(CancellationToken.None);
    }

    public override async Task<int> SaveChangesAsync(CancellationToken cancellationToken)
    {
        var currentUser = System.Threading.Thread.CurrentPrincipal.Identity.Name;

        var audit = new Audit();

        // Access to all auditing information


        audit.CreatedBy = string.IsNullOrEmpty(currentUser) ? "System" : currentUser;
        audit.PreSaveChanges(this);

        var entries = audit.Entries;
        foreach (var entry in entries)
        {
            foreach (var property in entry.Properties)
            {
            }
        }

        var rowsAffected = await base.SaveChangesAsync(cancellationToken).ConfigureAwait(false);
        audit.PostSaveChanges();

        if (audit.Configuration.AutoSavePreAction != null)
        {
            audit.Configuration.AutoSavePreAction(this, audit);
            await base.SaveChangesAsync(cancellationToken).ConfigureAwait(false);
        }

        // Access to all auditing information
        entries = audit.Entries;
        foreach (var entry in entries)
        {
            foreach (var property in entry.Properties)
            {
            }
        }

        return rowsAffected;
    }
}

}

And here's the first part of my ClientPricing class... (Note that I'm inheriting from IAuditable as referenced above in the OnModelCreating method to only allow auditing on specified classes)

public partial class ClientPricing : IAuditable
{
    [Key]
    public int RecId { get; set; }
    public string CustId { get; set; }
    public string CustomerName { get; set; }
    public Nullable<decimal> StandardOrderPrice { get; set; }
    public Nullable<decimal> StandardLinePrice { get; set; }
    public Nullable<decimal> StandardUnitPrice { get; set; }
    public Nullable<decimal> MassMailOrderPrice { get; set; }
    public Nullable<decimal> MassMailLinePrice { get; set; }
    public Nullable<decimal> MassMailUnitPrice { get; set; }
    public Nullable<decimal> BulkOrderPrice { get; set; }
    public Nullable<decimal> BulkLinePrice { get; set; }
    public Nullable<decimal> BulkUnitPrice { get; set; }
    public Nullable<decimal> PalletPrice { get; set; }
    public Nullable<decimal> CasePrice { get; set; }
    public Nullable<decimal> OuterPrice { get; set; }
    public Nullable<decimal> InnerPrice { get; set; }
    public Nullable<decimal> EachPrice { get; set; }

Lastly, here's the table data...
AuditEntryID EntitySetName EntityTypeName State StateName CreatedBy CreatedDate
1 ClientPricing ClientPricing 2 EntityModified System 2016-05-25 18:11:58.163
2 ClientPricing ClientPricing 2 EntityModified System 2016-05-25 18:14:43.117
3 ClientPricing ClientPricing 0 EntityAdded System 2016-05-25 18:18:05.197
4 ClientPricing ClientPricing 2 EntityModified System 2016-05-25 18:19:59.657
5 ClientPricing ClientPricing 1 EntityDeleted System 2016-05-25 18:21:18.400

AuditEntryPropertyID AuditEntryID RelationName PropertyName OldValue NewValue
1 1 NULL RecId 3002 3002
2 2 NULL RecId 3002 3002
3 3 NULL RecId NULL 3003
4 3 NULL CustId NULL mynewcustomer
5 3 NULL CustomerName NULL blah
6 3 NULL StandardOrderPrice NULL 2
7 3 NULL StandardLinePrice NULL 3
8 3 NULL StandardUnitPrice NULL 4
9 3 NULL MassMailOrderPrice NULL
10 3 NULL MassMailLinePrice NULL
11 3 NULL MassMailUnitPrice NULL
12 3 NULL BulkOrderPrice NULL
13 3 NULL BulkLinePrice NULL
14 3 NULL BulkUnitPrice NULL
15 3 NULL PalletPrice NULL
16 3 NULL CasePrice NULL
17 3 NULL OuterPrice NULL
18 3 NULL InnerPrice NULL
19 3 NULL EachPrice NULL
20 3 NULL ExpeditedOrderFlatFee NULL
21 3 NULL ExpeditedOrderPercentFee NULL
22 3 NULL WhiteGloveFlatFee NULL
23 3 NULL WhiteGlovePercentFee NULL
24 3 NULL ShippingSerialNumberFee NULL
25 3 NULL TierCalculationFlag NULL
26 3 NULL TierCalculationPeriod NULL
27 3 NULL RoutingGuideSetup NULL
28 3 NULL RoutingGuideMaintenance NULL
29 3 NULL ThirdPartyShippingFee NULL
30 3 NULL PackagingPrice NULL
31 3 NULL CubicStoragePrice NULL
32 3 NULL StorageLocationPeriod NULL
33 3 NULL LargeLocationPrice NULL
34 3 NULL SmallLocationPrice NULL
35 3 NULL PickType1LocationPrice NULL
36 3 NULL PickType2LocationPrice NULL
37 3 NULL PickType3LocationPrice NULL
38 3 NULL PickType4LocationPrice NULL
39 3 NULL PickType5LocationPrice NULL
40 3 NULL ReturnOrderPrice NULL
41 3 NULL ReturnLinePrice NULL
42 3 NULL ReturnUnitPrice NULL
43 3 NULL RMAOrderPrice NULL
44 3 NULL RMALinePrice NULL
45 3 NULL RMAUnitPrice NULL
46 3 NULL ReturnSerialNumberFee NULL
47 3 NULL PricePerReceipt NULL
48 3 NULL PricePerUnitReceived NULL
49 3 NULL AccountManager NULL
50 3 NULL Salesman NULL
51 3 NULL PaymentTerms NULL
52 3 NULL International NULL
53 3 NULL MonthlyServicesCharge NULL
54 3 NULL MonthlyWebServiceCharge NULL
55 3 NULL Receive20ftContainerPrice NULL
56 3 NULL Receive40ftContainerPrice NULL
57 3 NULL Receive53ftContainerPrice NULL
58 4 NULL RecId 3003 3003
59 5 NULL RecId 3003 NULL
60 5 NULL CustId mynewcustomer NULL
61 5 NULL CustomerName blahblah NULL
62 5 NULL StandardOrderPrice 7.00 NULL
63 5 NULL StandardLinePrice 8.00 NULL
64 5 NULL StandardUnitPrice 9.00 NULL
65 5 NULL MassMailOrderPrice 1.00 NULL
66 5 NULL MassMailLinePrice 2.00 NULL
67 5 NULL MassMailUnitPrice 3.00 NULL
68 5 NULL BulkOrderPrice NULL
69 5 NULL BulkLinePrice NULL
70 5 NULL BulkUnitPrice NULL
71 5 NULL PalletPrice NULL
72 5 NULL CasePrice NULL
73 5 NULL OuterPrice NULL
74 5 NULL InnerPrice NULL
75 5 NULL EachPrice NULL
76 5 NULL ExpeditedOrderFlatFee NULL
77 5 NULL ExpeditedOrderPercentFee NULL
78 5 NULL WhiteGloveFlatFee NULL
79 5 NULL WhiteGlovePercentFee NULL
80 5 NULL ShippingSerialNumberFee NULL
81 5 NULL TierCalculationFlag NULL
82 5 NULL TierCalculationPeriod NULL
83 5 NULL RoutingGuideSetup NULL
84 5 NULL RoutingGuideMaintenance NULL
85 5 NULL ThirdPartyShippingFee NULL
86 5 NULL PackagingPrice NULL
87 5 NULL CubicStoragePrice NULL
88 5 NULL StorageLocationPeriod NULL
89 5 NULL LargeLocationPrice NULL
90 5 NULL SmallLocationPrice NULL
91 5 NULL PickType1LocationPrice NULL
92 5 NULL PickType2LocationPrice NULL
93 5 NULL PickType3LocationPrice NULL
94 5 NULL PickType4LocationPrice NULL
95 5 NULL PickType5LocationPrice NULL
96 5 NULL ReturnOrderPrice NULL
97 5 NULL ReturnLinePrice NULL
98 5 NULL ReturnUnitPrice NULL
99 5 NULL RMAOrderPrice NULL
100 5 NULL RMALinePrice NULL
101 5 NULL RMAUnitPrice NULL
102 5 NULL ReturnSerialNumberFee NULL
103 5 NULL PricePerReceipt NULL
104 5 NULL PricePerUnitReceived NULL
105 5 NULL AccountManager NULL
106 5 NULL Salesman NULL
107 5 NULL PaymentTerms NULL
108 5 NULL International NULL
109 5 NULL MonthlyServicesCharge NULL
110 5 NULL MonthlyWebServiceCharge NULL
111 5 NULL Receive20ftContainerPrice NULL
112 5 NULL Receive40ftContainerPrice NULL
113 5 NULL Receive53ftContainerPrice NULL

@zzzprojects
Copy link
Collaborator

@zzzprojects zzzprojects commented May 26, 2016

Hello Mark,

Primary Key property will always show in the audit no matter the IgnorePropertyUnchanged because it's the only way to retrieve on which rows the modification is performed. If the customer becomes inactive, you must also know who the customer is who has been affected by the change.

As for the bug, I can clearly see what you are saying. I tried using the same class/code as you but for me, it worked as expected (property modified was audited).

Can you provide additional information to help us to reproduce this issue:

  • Entity Framework Version
  • EF+ Version
  • .NET Framework Version
  • Database Provider (SQL Server?)

Best Regards,

Jonathan

@markchipman
Copy link
Author

@markchipman markchipman commented May 26, 2016

Hi Jonathan... when I get into the office in a bit, let me send you the
entire sample project as I had to create it to try to determine / isolate
the issue. I'm using EF6 and SQL Server 2012 The Framework version is
4.5.1. I'll also send the database recreation scripts to make it painless.

I appreciate your assistance... and I'm glad you can see what I was trying
to explain as it is peculiar.

Thanks,

Mark Chipman

On Thu, May 26, 2016 at 8:04 AM, ZZZ Projects notifications@github.com
wrote:

Hello Mark,

Primary Key property will always show in the audit no matter the
IgnorePropertyUnchanged because it's the only way to retrieve on which rows
the modification is performed. If the customer becomes inactive, you must
also know who the customer is who has been affected by the change.

As for the bug, I can clearly see what you are saying. I tried using the
same class/code as you but for me, it worked as expected (property modified
was audited).

Can you provide additional information to help us to reproduce this issue:

  • Entity Framework Version
  • EF+ Version
  • .NET Framework Version
  • Database Provider (SQL Server?)

Best Regards,

Jonathan


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#8 (comment)

  • Mark

@markchipman
Copy link
Author

@markchipman markchipman commented May 26, 2016

Jonathan:

I sent the sample app solution to you via email. Come to think of it, let me send you also the actual data that I'm using for that table... I will export it to an excel spreadsheet that will be easy to reimport. Let me know if you have any ideas.

Thanks,

Mark

@zzzprojects
Copy link
Collaborator

@zzzprojects zzzprojects commented May 27, 2016

Thank you,

A Google drive request access has been sent. I will check it ASAP when I will be able to download these files.

@markchipman
Copy link
Author

@markchipman markchipman commented May 27, 2016

You should have access now.

Mark

On Fri, May 27, 2016 at 6:40 AM, ZZZ Projects notifications@github.com
wrote:

Thank you,

A Google drive request access has been sent. I will check it ASAP when I
will be able to download these files.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#8 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AEoKEGSvTLwevZemRa0bOvpd_zpWgNtBks5qFuYqgaJpZM4InGp_
.

  • Mark

@zzzprojects
Copy link
Collaborator

@zzzprojects zzzprojects commented May 27, 2016

Thank You,

We have found the issue. The bug is not in our library, but the code used to set an entity to the modified state.

To enable modified properties, you can use the following code.

if (ModelState.IsValid)
{
    db.ClientPricing.AddOrUpdate(clientPricing);
    // db.Entry(clientPricing).State = EntityState.Modified;

    // You can ensure a modify is used by uncommenting this code
    // if (db.Entry(clientPricing).State == EntityState.Added) throw new Exception("Oops! error message");

    db.SaveChanges();
    return RedirectToAction("Index");
}
return View(clientPricing);

Why this is happening?
The ClientPricing is created from a new object using all HttpPost values. When you force the state to "Modified", the context is not aware of the original value and use the current value instead. So every property has the original value == current value and our auditing only log the key since all other values are equals.

By using the method AddOrUpdate, a database round trip is performed to check if the entity exists or not and GET the original value from the database. This time, when we compare the original value with the current value, we can find which property has been modified.

We will soon improve our Wiki and add some case like this since you are not the first one reporting this issue.

Let me know if you need a better explanation of this issue.

@zzzprojects zzzprojects self-assigned this May 27, 2016
@markchipman
Copy link
Author

@markchipman markchipman commented May 27, 2016

Thank you so much for your very quick resolution!

It's great that you have discovered the issue. Yes I agree that a
mention in the Wiki on how to correctly set the object state will be
helpful to everyone for updates to records.

You might mention that in order to use AddOrUpdate, a "using System.Data.Entity.Migrations;" declaration needs to be added.

One other suggestion is to include these two lines so that others can see
how to easily set the 'createdby' field to the currently logged-in user or
a default...

var currentUser = System.Threading.Thread.CurrentPrincipal.Identity.Name;
audit.CreatedBy = string.IsNullOrEmpty(currentUser) ? "System" :
currentUser;

Thanks again for your help on the issue and this great library.

  • Mark

@zzzprojects
Copy link
Collaborator

@zzzprojects zzzprojects commented Sep 13, 2016

Hello Mark,

We just released the version 1.4.0 and added your suggestion about currentUser (which should have been added long time ago!!!)

If you test it, let me know if this is working to allow us to close this issue.

@zzzprojects
Copy link
Collaborator

@zzzprojects zzzprojects commented Nov 12, 2016

Closing: Fixed in 1.4.0

@Borja204
Copy link

@Borja204 Borja204 commented Sep 5, 2018

I know this one is closed but I just faced the same issue today and found a solution to avoid using the AddOrUpdate Method:

var originalE = _db.Entities.Find(entity.Id); _db.Entry(originalE ).CurrentValues.SetValues(entity); _db.SaveChanges();

Regards!

@JonathanMagnan
Copy link
Member

@JonathanMagnan JonathanMagnan commented Sep 5, 2018

Hello @Borja204 ,

Are you sure this solution work? Normally the find method will look if the entity exists in the ChangeTracker or get it from the database.

You avoid the AddOrUpdate, but it's pretty much the same. As AddOrUpdate, you will perform a database round-trip since the entity should not already exist in your context.

Best Regards,

Jonathan

@Borja204
Copy link

@Borja204 Borja204 commented Sep 5, 2018

Hello @JonathanMagnan,

Ill check it again tomorrow but I did some tests earlier today and it apareantly worked (not a lot of tests since I started using this functionality of your library today).

Im out of the office atm but tomorrow I'll continue with the audit functionality in the app so Ill check it again and give you a more accurate response.

Regards,

@edihasaj
Copy link

@edihasaj edihasaj commented Mar 21, 2020

The easiest way to do for me was:

`
var cadastralAreaDb = await _context.CadastralAreas.FindAsync(id).ConfigureAwait(false);

var audit = new Audit
{
CreatedBy = "Edi"
};

_context.Entry(cadastralAreaDb).CurrentValues.SetValues(cadastralArea);
await _context.SaveChangesAsync(audit).ConfigureAwait(false);
`

This is tested in ASP NET Core 3.1

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

No branches or pull requests

4 participants