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

AbstractAuditingEntity not working for update for entities with dto #1774

Closed
krishnakumarp opened this issue Jul 27, 2015 · 7 comments
Closed
Milestone

Comments

@krishnakumarp
Copy link

I tried to use the generated AbstractAuditingEntity to add audit fields to one of the application's entity. The update use case did not work correctly. It was failing with a constraint violation error that createdBy cannot be null. This property is marked as @NotNull in the generated class. But this field will be null in the update mode.

I'd to change this class to modify the definitions of createdBy and createdAt to fix this issue.

@CreatedBy
@Column(name = "created_by", nullable = false, length = 50, updatable = false)
@JsonIgnore
private String createdBy;

@CreatedDate
@Type(type = "org.jadira.usertype.dateandtime.joda.PersistentDateTime")
@Column(name = "created_date", nullable = false, updatable = false)
@JsonIgnore
private DateTime createdDate = DateTime.now();

If this is correct, please update the generator to work correctly.

@gzsombor
Copy link
Member

What cleared out the 'createdBy' field? It should be there, when it's retrieved from the database.

@krishnakumarp
Copy link
Author

I mask these fields out in dto layer. It is normal not to pass these audit fields to dto and back. You dont want to update created_* fields with entity updates. So these fields should be marked as updatable=false and should not have @NotNull

@gzsombor
Copy link
Member

Yes, sure, you dont want to put that fields into a DTO, and blindly update it. It's the drawback for using the same DTO for sending to the client, and receiving from the client.
But, as we don't generate that auditing DTO for that entity, we can't put that ignore flags inside the mapstruct based mapper.
I don't think it would be a wise decision, to mark the entity's created_ field as it can be null. We don't want to accidentally remove that information from the database.

@krishnakumarp
Copy link
Author

But we will not update it accidentally, once we mark it as , updatable = false. No?
While creating entity it is populated by AuditingEntityListener, so no chance of that being null.
Minimally, we could mark those fields as updatable = false.

@gzsombor
Copy link
Member

yes, that would make sense

@jdubois
Copy link
Member

jdubois commented Aug 8, 2015

So this is specific to using DTOs:

  • can you update the title of the ticket to reflect this? (I can't do it with the mobile app)
  • @gzsombor I'm not sure I understand the solution correctly, could you do a PR or write à small exemple?

@krishnakumarp krishnakumarp changed the title AbstractAuditingEntity not working for update AbstractAuditingEntity not working for update for entities with dto Aug 8, 2015
@jdubois
Copy link
Member

jdubois commented Oct 1, 2015

  • If you use DTOs, I fully agree you probably don't want to expose those fields
  • Then it's up to you, in your mapping, to update them correctly

I don't think we should change the existing behavior, then this has been opened for a long time and I don't find this issue very clear -> this is why I'm closing this

If you have something more detailed, with maybe a PR or some sample code that demonstrates your issue (including the mapping code), it would be most welcomed.

@jdubois jdubois closed this as completed Oct 1, 2015
@jdubois jdubois modified the milestone: 2.22.0 Oct 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants