Skip to content

Commit 8c678fe

Browse files
ShashwatDadhichkomalreddy3
ShashwatDadhich
authored and
komalreddy3
committed
fix: git material saved in transaction (#5040)
* git material flow added in transaction * wire refactored * code review comments incorporated * code review comments incorporated * code review comments incorporated
1 parent fe9df70 commit 8c678fe

19 files changed

+103
-72
lines changed

‎Wire.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,9 @@ func InitializeApp() (*App, error) {
226226
wire.Bind(new(router.PProfRouter), new(*router.PProfRouterImpl)),
227227
// ---- pprof end ----
228228

229+
sql.NewTransactionUtilImpl,
230+
wire.Bind(new(sql.TransactionWrapper), new(*sql.TransactionUtilImpl)),
231+
229232
trigger.NewPipelineRestHandler,
230233
wire.Bind(new(trigger.PipelineTriggerRestHandler), new(*trigger.PipelineTriggerRestHandlerImpl)),
231234
app.GetAppServiceConfig,

‎cmd/external-app/wire.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ func InitializeApp() (*App, error) {
103103
telemetry.NewPosthogClient,
104104
delete2.NewDeleteServiceImpl,
105105

106+
sql.NewTransactionUtilImpl,
107+
106108
pipelineConfig.NewMaterialRepositoryImpl,
107109
wire.Bind(new(pipelineConfig.MaterialRepository), new(*pipelineConfig.MaterialRepositoryImpl)),
108110
// appStatus

‎cmd/external-app/wire_gen.go

Lines changed: 6 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/sql/repository/imageTagging/ImageTaggingRepository.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,10 @@ type ImageTaggingRepositoryImpl struct {
8585
*sql.TransactionUtilImpl
8686
}
8787

88-
func NewImageTaggingRepositoryImpl(db *pg.DB) *ImageTaggingRepositoryImpl {
88+
func NewImageTaggingRepositoryImpl(db *pg.DB, TransactionUtilImpl *sql.TransactionUtilImpl) *ImageTaggingRepositoryImpl {
8989
return &ImageTaggingRepositoryImpl{
9090
dbConnection: db,
91-
TransactionUtilImpl: sql.NewTransactionUtilImpl(db),
91+
TransactionUtilImpl: TransactionUtilImpl,
9292
}
9393
}
9494

‎internal/sql/repository/pipelineConfig/CiPipelineRepository.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,11 @@ type CiPipelineRepositoryImpl struct {
150150
*sql.TransactionUtilImpl
151151
}
152152

153-
func NewCiPipelineRepositoryImpl(dbConnection *pg.DB, logger *zap.SugaredLogger) *CiPipelineRepositoryImpl {
153+
func NewCiPipelineRepositoryImpl(dbConnection *pg.DB, logger *zap.SugaredLogger, TransactionUtilImpl *sql.TransactionUtilImpl) *CiPipelineRepositoryImpl {
154154
return &CiPipelineRepositoryImpl{
155155
dbConnection: dbConnection,
156156
logger: logger,
157-
TransactionUtilImpl: sql.NewTransactionUtilImpl(dbConnection),
157+
TransactionUtilImpl: TransactionUtilImpl,
158158
}
159159
}
160160

‎internal/sql/repository/pipelineConfig/MaterialRepository.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,16 @@ type GitMaterial struct {
5252

5353
type MaterialRepository interface {
5454
MaterialExists(url string) (bool, error)
55-
SaveMaterial(material *GitMaterial) error
56-
UpdateMaterial(material *GitMaterial) error
55+
SaveMaterial(tx *pg.Tx, material *GitMaterial) error
56+
UpdateMaterial(tx *pg.Tx, material *GitMaterial) error
5757
Update(materials []*GitMaterial) error
5858
FindByAppId(appId int) ([]*GitMaterial, error)
5959
FindById(Id int) (*GitMaterial, error)
6060
FindByAppIdAndGitMaterialId(appId, id int) (*GitMaterial, error)
6161
UpdateMaterialScmId(material *GitMaterial) error
6262
FindByAppIdAndCheckoutPath(appId int, checkoutPath string) (*GitMaterial, error)
6363
FindByGitProviderId(gitProviderId int) (materials []*GitMaterial, err error)
64-
MarkMaterialDeleted(material *GitMaterial) error
64+
MarkMaterialDeleted(tx *pg.Tx, material *GitMaterial) error
6565
FindNumberOfAppsWithGitRepo(appIds []int) (int, error)
6666
FindByAppIds(appIds []int) ([]*GitMaterial, error)
6767
}
@@ -113,12 +113,12 @@ func (repo MaterialRepositoryImpl) MaterialExists(url string) (bool, error) {
113113
return exists, err
114114
}
115115

116-
func (repo MaterialRepositoryImpl) SaveMaterial(material *GitMaterial) error {
117-
return repo.dbConnection.Insert(material)
116+
func (repo MaterialRepositoryImpl) SaveMaterial(tx *pg.Tx, material *GitMaterial) error {
117+
return tx.Insert(material)
118118
}
119119

120-
func (repo MaterialRepositoryImpl) UpdateMaterial(material *GitMaterial) error {
121-
return repo.dbConnection.Update(material)
120+
func (repo MaterialRepositoryImpl) UpdateMaterial(tx *pg.Tx, material *GitMaterial) error {
121+
return tx.Update(material)
122122
}
123123

124124
func (repo MaterialRepositoryImpl) UpdateMaterialScmId(material *GitMaterial) error {
@@ -164,9 +164,9 @@ func (repo MaterialRepositoryImpl) FindByGitProviderId(gitProviderId int) (mater
164164
return materials, err
165165
}
166166

167-
func (repo MaterialRepositoryImpl) MarkMaterialDeleted(material *GitMaterial) error {
167+
func (repo MaterialRepositoryImpl) MarkMaterialDeleted(tx *pg.Tx, material *GitMaterial) error {
168168
material.Active = false
169-
return repo.dbConnection.Update(material)
169+
return tx.Update(material)
170170
}
171171

172172
func (repo MaterialRepositoryImpl) FindNumberOfAppsWithGitRepo(appIds []int) (int, error) {

‎pkg/chartRepo/repository/ChartsRepository.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,10 @@ type ChartRepository interface {
5858
sql.TransactionWrapper
5959
}
6060

61-
func NewChartRepository(dbConnection *pg.DB) *ChartRepositoryImpl {
61+
func NewChartRepository(dbConnection *pg.DB, TransactionUtilImpl *sql.TransactionUtilImpl) *ChartRepositoryImpl {
6262
return &ChartRepositoryImpl{
6363
dbConnection: dbConnection,
64-
TransactionUtilImpl: sql.NewTransactionUtilImpl(dbConnection),
64+
TransactionUtilImpl: TransactionUtilImpl,
6565
}
6666
}
6767

‎pkg/cluster/repository/EphemeralContainersRepository.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ type EphemeralContainersRepository interface {
4040
FindContainerByName(clusterID int, namespace, podName, name string) (*EphemeralContainerBean, error)
4141
}
4242

43-
func NewEphemeralContainersRepositoryImpl(db *pg.DB) *EphemeralContainersRepositoryImpl {
43+
func NewEphemeralContainersRepositoryImpl(db *pg.DB, transactionUtilImpl *sql.TransactionUtilImpl) *EphemeralContainersRepositoryImpl {
4444
return &EphemeralContainersRepositoryImpl{
4545
dbConnection: db,
46-
TransactionUtilImpl: sql.NewTransactionUtilImpl(db),
46+
TransactionUtilImpl: transactionUtilImpl,
4747
}
4848
}
4949

‎pkg/genericNotes/repository/GenericNoteHistoryRepository.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,7 @@ type GenericNoteHistoryRepository interface {
3636
FindHistoryByNoteId(id []int) ([]GenericNoteHistory, error)
3737
}
3838

39-
func NewGenericNoteHistoryRepositoryImpl(dbConnection *pg.DB) *GenericNoteHistoryRepositoryImpl {
40-
TransactionUtilImpl := sql.NewTransactionUtilImpl(dbConnection)
39+
func NewGenericNoteHistoryRepositoryImpl(dbConnection *pg.DB, TransactionUtilImpl *sql.TransactionUtilImpl) *GenericNoteHistoryRepositoryImpl {
4140
return &GenericNoteHistoryRepositoryImpl{
4241
dbConnection: dbConnection,
4342
TransactionUtilImpl: TransactionUtilImpl,

‎pkg/genericNotes/repository/GenericNoteRepository.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ type GenericNoteRepository interface {
5151
GetDescriptionFromAppIds(appIds []int) ([]*GenericNote, error)
5252
}
5353

54-
func NewGenericNoteRepositoryImpl(dbConnection *pg.DB) *GenericNoteRepositoryImpl {
55-
TransactionUtilImpl := sql.NewTransactionUtilImpl(dbConnection)
54+
func NewGenericNoteRepositoryImpl(dbConnection *pg.DB, TransactionUtilImpl *sql.TransactionUtilImpl) *GenericNoteRepositoryImpl {
5655
return &GenericNoteRepositoryImpl{
5756
dbConnection: dbConnection,
5857
TransactionUtilImpl: TransactionUtilImpl,

‎pkg/infraConfig/infraConfigRepository.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ type InfraConfigRepositoryImpl struct {
2424
*sql.TransactionUtilImpl
2525
}
2626

27-
func NewInfraProfileRepositoryImpl(dbConnection *pg.DB) *InfraConfigRepositoryImpl {
27+
func NewInfraProfileRepositoryImpl(dbConnection *pg.DB, TransactionUtilImpl *sql.TransactionUtilImpl) *InfraConfigRepositoryImpl {
2828
return &InfraConfigRepositoryImpl{
2929
dbConnection: dbConnection,
30-
TransactionUtilImpl: sql.NewTransactionUtilImpl(dbConnection),
30+
TransactionUtilImpl: TransactionUtilImpl,
3131
}
3232
}
3333

‎pkg/pipeline/CiCdPipelineOrchestrator.go

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ type CiCdPipelineOrchestratorImpl struct {
130130
genericNoteService genericNotes.GenericNoteService
131131
customTagService CustomTagService
132132
chartService chart.ChartService
133+
transactionManager sql.TransactionWrapper
133134
}
134135

135136
func NewCiCdPipelineOrchestrator(
@@ -156,7 +157,7 @@ func NewCiCdPipelineOrchestrator(
156157
configMapService ConfigMapService,
157158
customTagService CustomTagService,
158159
genericNoteService genericNotes.GenericNoteService,
159-
chartService chart.ChartService) *CiCdPipelineOrchestratorImpl {
160+
chartService chart.ChartService, transactionManager sql.TransactionWrapper) *CiCdPipelineOrchestratorImpl {
160161
return &CiCdPipelineOrchestratorImpl{
161162
appRepository: pipelineGroupRepository,
162163
logger: logger,
@@ -183,6 +184,7 @@ func NewCiCdPipelineOrchestrator(
183184
genericNoteService: genericNoteService,
184185
customTagService: customTagService,
185186
chartService: chartService,
187+
transactionManager: transactionManager,
186188
}
187189
}
188190

@@ -1271,6 +1273,11 @@ func (impl CiCdPipelineOrchestratorImpl) DeleteApp(appId int, userId int32) erro
12711273
}
12721274

12731275
func (impl CiCdPipelineOrchestratorImpl) CreateMaterials(createMaterialRequest *bean.CreateMaterialDTO) (*bean.CreateMaterialDTO, error) {
1276+
tx, err := impl.transactionManager.StartTx()
1277+
if err != nil {
1278+
return nil, err
1279+
}
1280+
defer tx.Rollback()
12741281
existingMaterials, err := impl.materialRepository.FindByAppId(createMaterialRequest.AppId)
12751282
if err != nil {
12761283
impl.logger.Errorw("err", "err", err)
@@ -1295,7 +1302,7 @@ func (impl CiCdPipelineOrchestratorImpl) CreateMaterials(createMaterialRequest *
12951302
var materials []*bean.GitMaterial
12961303
for _, inputMaterial := range createMaterialRequest.Material {
12971304
inputMaterial.UpdateSanitisedGitRepoUrl()
1298-
m, err := impl.createMaterial(inputMaterial, createMaterialRequest.AppId, createMaterialRequest.UserId)
1305+
m, err := impl.createMaterial(tx, inputMaterial, createMaterialRequest.AppId, createMaterialRequest.UserId)
12991306
inputMaterial.Id = m.Id
13001307
if err != nil {
13011308
return nil, err
@@ -1307,12 +1314,22 @@ func (impl CiCdPipelineOrchestratorImpl) CreateMaterials(createMaterialRequest *
13071314
impl.logger.Errorw("error in updating to sensor", "err", err)
13081315
return nil, err
13091316
}
1317+
err = impl.transactionManager.CommitTx(tx)
1318+
if err != nil {
1319+
impl.logger.Errorw("error in committing tx Create material", "err", err, "materials", materials)
1320+
return nil, err
1321+
}
13101322
impl.logger.Debugw("all materials are ", "materials", materials)
13111323
return createMaterialRequest, nil
13121324
}
13131325

13141326
func (impl CiCdPipelineOrchestratorImpl) UpdateMaterial(updateMaterialDTO *bean.UpdateMaterialDTO) (*bean.UpdateMaterialDTO, error) {
1315-
updatedMaterial, err := impl.updateMaterial(updateMaterialDTO)
1327+
tx, err := impl.transactionManager.StartTx()
1328+
if err != nil {
1329+
return nil, err
1330+
}
1331+
defer tx.Rollback()
1332+
updatedMaterial, err := impl.updateMaterial(tx, updateMaterialDTO)
13161333
if err != nil {
13171334
impl.logger.Errorw("err", "err", err)
13181335
return nil, err
@@ -1323,6 +1340,11 @@ func (impl CiCdPipelineOrchestratorImpl) UpdateMaterial(updateMaterialDTO *bean.
13231340
impl.logger.Errorw("error in updating to git-sensor", "err", err)
13241341
return nil, err
13251342
}
1343+
err = impl.transactionManager.CommitTx(tx)
1344+
if err != nil {
1345+
impl.logger.Errorw("error in committing tx Update material", "err", err)
1346+
return nil, err
1347+
}
13261348
return updateMaterialDTO, nil
13271349
}
13281350

@@ -1456,7 +1478,7 @@ func (impl CiCdPipelineOrchestratorImpl) validateCheckoutPathsForMultiGit(allPat
14561478
return nil
14571479
}
14581480

1459-
func (impl CiCdPipelineOrchestratorImpl) updateMaterial(updateMaterialDTO *bean.UpdateMaterialDTO) (*pipelineConfig.GitMaterial, error) {
1481+
func (impl CiCdPipelineOrchestratorImpl) updateMaterial(tx *pg.Tx, updateMaterialDTO *bean.UpdateMaterialDTO) (*pipelineConfig.GitMaterial, error) {
14601482
existingMaterials, err := impl.materialRepository.FindByAppId(updateMaterialDTO.AppId)
14611483
if err != nil {
14621484
impl.logger.Errorw("err", "err", err)
@@ -1497,19 +1519,19 @@ func (impl CiCdPipelineOrchestratorImpl) updateMaterial(updateMaterialDTO *bean.
14971519
currentMaterial.FilterPattern = updateMaterialDTO.Material.FilterPattern
14981520
currentMaterial.AuditLog = sql.AuditLog{UpdatedBy: updateMaterialDTO.UserId, CreatedBy: currentMaterial.CreatedBy, UpdatedOn: time.Now(), CreatedOn: currentMaterial.CreatedOn}
14991521

1500-
err = impl.materialRepository.UpdateMaterial(currentMaterial)
1522+
err = impl.materialRepository.UpdateMaterial(tx, currentMaterial)
15011523

15021524
if err != nil {
15031525
impl.logger.Errorw("error in updating material", "material", currentMaterial, "err", err)
15041526
return nil, err
15051527
}
15061528

1507-
err = impl.gitMaterialHistoryService.CreateMaterialHistory(currentMaterial)
1529+
err = impl.gitMaterialHistoryService.CreateMaterialHistory(tx, currentMaterial)
15081530

15091531
return currentMaterial, nil
15101532
}
15111533

1512-
func (impl CiCdPipelineOrchestratorImpl) createMaterial(inputMaterial *bean.GitMaterial, appId int, userId int32) (*pipelineConfig.GitMaterial, error) {
1534+
func (impl CiCdPipelineOrchestratorImpl) createMaterial(tx *pg.Tx, inputMaterial *bean.GitMaterial, appId int, userId int32) (*pipelineConfig.GitMaterial, error) {
15131535
basePath := path.Base(inputMaterial.Url)
15141536
basePath = strings.TrimSuffix(basePath, ".git")
15151537
material := &pipelineConfig.GitMaterial{
@@ -1523,12 +1545,12 @@ func (impl CiCdPipelineOrchestratorImpl) createMaterial(inputMaterial *bean.GitM
15231545
FilterPattern: inputMaterial.FilterPattern,
15241546
AuditLog: sql.AuditLog{UpdatedBy: userId, CreatedBy: userId, UpdatedOn: time.Now(), CreatedOn: time.Now()},
15251547
}
1526-
err := impl.materialRepository.SaveMaterial(material)
1548+
err := impl.materialRepository.SaveMaterial(tx, material)
15271549
if err != nil {
15281550
impl.logger.Errorw("error in saving material", "material", material, "err", err)
15291551
return nil, err
15301552
}
1531-
err = impl.gitMaterialHistoryService.CreateMaterialHistory(material)
1553+
err = impl.gitMaterialHistoryService.CreateMaterialHistory(tx, material)
15321554
return material, err
15331555
}
15341556

‎pkg/pipeline/CiMaterialConfigService.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig"
2323
"github.com/devtron-labs/devtron/pkg/bean"
2424
"github.com/devtron-labs/devtron/pkg/pipeline/history"
25+
"github.com/devtron-labs/devtron/pkg/sql"
2526
"github.com/go-pg/pg"
2627
"github.com/juju/errors"
2728
"go.uber.org/zap"
@@ -52,6 +53,7 @@ type CiMaterialConfigServiceImpl struct {
5253
gitMaterialHistoryService history.GitMaterialHistoryService
5354
pipelineRepository pipelineConfig.PipelineRepository
5455
ciPipelineMaterialRepository pipelineConfig.CiPipelineMaterialRepository
56+
transactionManager sql.TransactionWrapper
5557
}
5658

5759
func NewCiMaterialConfigServiceImpl(
@@ -62,7 +64,7 @@ func NewCiMaterialConfigServiceImpl(
6264
ciPipelineRepository pipelineConfig.CiPipelineRepository,
6365
gitMaterialHistoryService history.GitMaterialHistoryService,
6466
pipelineRepository pipelineConfig.PipelineRepository,
65-
ciPipelineMaterialRepository pipelineConfig.CiPipelineMaterialRepository) *CiMaterialConfigServiceImpl {
67+
ciPipelineMaterialRepository pipelineConfig.CiPipelineMaterialRepository, transactionManager sql.TransactionWrapper) *CiMaterialConfigServiceImpl {
6668

6769
return &CiMaterialConfigServiceImpl{
6870
logger: logger,
@@ -73,6 +75,7 @@ func NewCiMaterialConfigServiceImpl(
7375
gitMaterialHistoryService: gitMaterialHistoryService,
7476
pipelineRepository: pipelineRepository,
7577
ciPipelineMaterialRepository: ciPipelineMaterialRepository,
78+
transactionManager: transactionManager,
7679
}
7780
}
7881

@@ -122,21 +125,21 @@ func (impl *CiMaterialConfigServiceImpl) DeleteMaterial(request *bean.UpdateMate
122125
existingMaterial.UpdatedOn = time.Now()
123126
existingMaterial.UpdatedBy = request.UserId
124127

125-
err = impl.materialRepo.MarkMaterialDeleted(existingMaterial)
128+
tx, err := impl.transactionManager.StartTx()
126129
if err != nil {
127-
impl.logger.Errorw("error in deleting git material", "gitMaterial", existingMaterial)
128130
return err
129131
}
130132

131-
err = impl.gitMaterialHistoryService.MarkMaterialDeletedAndCreateHistory(existingMaterial)
133+
defer tx.Rollback()
132134

133-
dbConnection := impl.pipelineRepository.GetConnection()
134-
tx, err := dbConnection.Begin()
135+
err = impl.materialRepo.MarkMaterialDeleted(tx, existingMaterial)
135136
if err != nil {
137+
impl.logger.Errorw("error in deleting git material", "gitMaterial", existingMaterial)
136138
return err
137139
}
138-
// Rollback tx on error.
139-
defer tx.Rollback()
140+
141+
err = impl.gitMaterialHistoryService.MarkMaterialDeletedAndCreateHistory(tx, existingMaterial)
142+
140143
var materials []*pipelineConfig.CiPipelineMaterial
141144
for _, pipeline := range pipelines {
142145
materialDbObject, err := impl.ciPipelineMaterialRepository.GetByPipelineIdAndGitMaterialId(pipeline.Id, request.Material.Id)

0 commit comments

Comments
 (0)