WIP: Introduce gormsupport.Versioning to auto increment a model's Version on update and set to 0 on create

[kwk]

TL;DR

This fictional code:

type MyModel struct {
  Version int
  Name string
  // more fields
}

model := MyModel{}

// Increment version of model that we're about to save
oldVersion := model.Version
model.Version = model.Version + 1

// Check the model we're trying to update has the correct version.
db.Where("Version = ?", oldVersion).Save(&model)

// Check the model we're trying to delete has the correct version.
db.Where("Version = ?", oldVersion).Delete(&model)

can be simplified to just:

type MyModel struct {
  gormsupport.Versioning
  Name string
  // more fields
}
model := MyModel{}
db.Save(&model)
db.Delete(&model)

This is

• less error prone,
• unclutters the code and
• nicely automated.

About

The change is best explained with a comment from the code.

// Versioning can be embedded into model structs that want to have a Version
// column which will
// 
// * automatically be incremented before each UPDATE,
// * set to 0 on CREATE,
// * and checked for compatibility on each UPDATE and DELETE.
//
// For the first creation of a model the initial version will always be
// overwritten with 0 nomatter what the user specified in the model itself. The
// model itself is not changed in any cases, just the DB query for INSERT and
// UPDATE is touched.
//
// We also add
//
//    AND version=<VERSION-OF-THE-MODEL>
//
// to the WHERE conditions of the UPDATE part.
type Versioning struct {
	Version int `json:"version"`
}

At first sight it doesn't look like this PR is helping much but we can easily forget to increment a Version field on UPDATE or to set it to 0 on INSERT. If you embed gormsupport.Versioning in your struct this cannot happen because Gorm will call the BeforeUpdate/BeforeCreate function of any model object if it exists.

We make sure that upon creation the version will be 0, no matter what the user specified. This is done with the BeforeCreate callback.

What models already embed the Versioning struct?

• workitem.WorkItemStorage
• workitem.WorkItemType
• link.WorkItemLinkCategory
• link.WorkItemLinkType
• link.WorkItemLink
• space.Space
• spacetemplate.SpaceTemplate
  • Won't use gormsupport.Versioning here because we update the spacetemplates quite often during tests and currently this does not respect any versioning whatsoever. Will have to look into this in another PR.
• area.Area
• label.Label
• query.Query

This adds the gormsupport.Versioning struct as discussed in https://chat.openshift.io/developers/pl/js3udrouwidfjqwjx7zr7n6aga

[baijum]

Looks good to me. Are you working on the item mentioned in the description? Also how about changing other places where version is used?

[codecov-io]

Codecov Report

Merging #2263 into master will decrease coverage by 0.1%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2263      +/-   ##
==========================================
- Coverage   69.52%   69.41%   -0.11%     
==========================================
  Files         176      176              
  Lines       16670    16397     -273     
==========================================
- Hits        11589    11382     -207     
+ Misses       3957     3922      -35     
+ Partials     1124     1093      -31

Impacted Files	Coverage Δ
workitem/link/category_repository.go	33.33% <ø> (+0.36%)	⬆️
workitem/workitem_repository.go	68.23% <ø> (+0.77%)	⬆️
workitem/link/category.go	100% <100%> (ø)	⬆️
workitem/workitem_storage.go	70.83% <100%> (ø)	⬆️
gormsupport/versioning.go	100% <100%> (ø)
controller/user.go	0% <0%> (-39.03%)	⬇️
workitem/expression_compiler.go	82.05% <0%> (-1.46%)	⬇️
workitem/simple_type.go	81.95% <0%> (-0.72%)	⬇️
migration/migration.go	61.43% <0%> (-0.13%)	⬇️
controller/iteration.go	78.27% <0%> (-0.06%)	⬇️
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8fab53...5735c9b. Read the comment docs.

[kwk]

Looks good to me. Are you working on the item mentioned in the description? Also how about changing other places where version is used?

@baijum I have worked on the items mentioned in the description and need to finish test fixups. Not doing that today but on Friday maybe.

[baijum]

LGTM

[michaelkleinhenz]

What is the story/task reference for this PR?

[kwk]

What is the story/task reference for this PR?

There is none. This is cleanup work and stabilization work. Do we have an issue for this already? I don't want to open an issue for all the cleanups I'm doing.