Skip to content

Commit

Permalink
Merge pull request #14961 from transcom/B-22779
Browse files Browse the repository at this point in the history 
B 22779 - afero file cleanup
  • Loading branch information
@deandreJones
deandreJones authored Mar 5, 2025
2 parents 8bb93db + 2888df7 commit cac075e
Show file tree
Hide file tree
Showing 33 changed files with 856 additions and 293 deletions.
2 changes: 1 addition & 1 deletion cmd/generate-shipment-summary/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func main() {
noErr(err)
ppmGenerator, err := shipmentsummaryworksheet.NewSSWPPMGenerator(generator)
noErr(err)
ssw, info, err := ppmGenerator.FillSSWPDFForm(page1Data, page2Data, page3Data)
ssw, info, err := ppmGenerator.FillSSWPDFForm(page1Data, page2Data, page3Data, "")
noErr(err)
fmt.Println(ssw.Name()) // Should always return
fmt.Println(info.PageCount) // Page count should always be 2
Expand Down
2 changes: 1 addition & 1 deletion cmd/image-to-pdf/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func main() {
}

appCtx := appcontext.NewAppContext(nil, logger, nil)
path, err := generator.MergeImagesToPDF(appCtx, inputFiles)
path, err := generator.MergeImagesToPDF(appCtx, inputFiles, "")
if err != nil {
log.Fatal(err.Error())
}
Expand Down
14 changes: 13 additions & 1 deletion pkg/handlers/ghcapi/payment_request.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,16 +283,28 @@ func (h PaymentRequestBulkDownloadHandler) Handle(params paymentrequestop.BulkDo
return paymentrequestop.NewBulkDownloadBadRequest().WithPayload(errPayload), err
}

paymentRequestPacket, err := h.PaymentRequestBulkDownloadCreator.CreatePaymentRequestBulkDownload(appCtx, paymentRequestID)
paymentRequestPacket, requestPath, err := h.PaymentRequestBulkDownloadCreator.CreatePaymentRequestBulkDownload(appCtx, paymentRequestID)
if err != nil {
logger.Error("Error creating Payment Request Downloads Packet", zap.Error(err))

// need to cleanup any files created prior to the request failure
if err = h.PaymentRequestBulkDownloadCreator.CleanupPaymentRequestBulkDir(requestPath); err != nil {
logger.Error("Error cleaning up bulk payment request files", zap.Error(err))
}

errInstance := fmt.Sprintf("Instance: %s", h.GetTraceIDFromRequest(params.HTTPRequest))
errPayload := &ghcmessages.Error{Message: &errInstance}
return paymentrequestop.NewBulkDownloadInternalServerError().
WithPayload(errPayload), err
}

payload := io.NopCloser(paymentRequestPacket)

// we have copied the created files into the payload so we can remove them from memory
if err = h.PaymentRequestBulkDownloadCreator.CleanupPaymentRequestBulkDir(requestPath); err != nil {
logger.Error("Error deleting temp bulk payment request files", zap.Error(err))
}

filename := fmt.Sprintf("inline; filename=\"PaymentRequestBulkPacket-%s.pdf\"", time.Now().Format("01-02-2006_15-04-05"))

return paymentrequestop.NewBulkDownloadOK().WithContentDisposition(filename).WithPayload(payload), nil
Expand Down
37 changes: 35 additions & 2 deletions pkg/handlers/ghcapi/ppm_document.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,16 +177,33 @@ func (h showAOAPacketHandler) Handle(params ppmdocumentops.ShowAOAPacketParams)
return ppmdocumentops.NewShowAOAPacketBadRequest().WithPayload(errPayload), err
}

AOAPacket, err := h.AOAPacketCreator.CreateAOAPacket(appCtx, ppmShipmentID, false)
AOAPacket, packetPath, err := h.AOAPacketCreator.CreateAOAPacket(appCtx, ppmShipmentID, false)

if err != nil {
logger.Error("Error creating AOA", zap.Error(err))
errInstance := fmt.Sprintf("Instance: %s", h.GetTraceIDFromRequest(params.HTTPRequest))
errPayload := &ghcmessages.Error{Message: &errInstance}

// need to cleanup any files created prior to the packet creation failure
if err = h.AOAPacketCreator.CleanupAOAPacketDir(packetPath); err != nil {
logger.Error("Error: cleaning up temp AOA files", zap.Error(err))
}

return ppmdocumentops.NewShowAOAPacketInternalServerError().
WithPayload(errPayload), err
}

payload := io.NopCloser(AOAPacket)

// we have copied the created files into the payload so we can remove them from memory
if err = h.AOAPacketCreator.CleanupAOAPacketDir(packetPath); err != nil {
logger.Error("Error deleting temp AOA Packet directory", zap.Error(err))
errInstance := fmt.Sprintf("Instance: %s", h.GetTraceIDFromRequest(params.HTTPRequest))
errPayload := &ghcmessages.Error{Message: &errInstance}
return ppmdocumentops.NewShowAOAPacketInternalServerError().
WithPayload(errPayload), err
}

filename := fmt.Sprintf("inline; filename=\"AOA-%s.pdf\"", time.Now().Format("01-02-2006_15-04-05"))

return ppmdocumentops.NewShowAOAPacketOK().WithContentDisposition(filename).WithPayload(payload), nil
Expand All @@ -208,8 +225,14 @@ func (h ShowPaymentPacketHandler) Handle(params ppmdocumentops.ShowPaymentPacket
return handlers.ResponseForError(appCtx.Logger(), err), err
}

pdf, err := h.PaymentPacketCreator.GenerateDefault(appCtx, ppmShipmentID)
pdf, packetPath, err := h.PaymentPacketCreator.GenerateDefault(appCtx, ppmShipmentID)

if err != nil {
// need to cleanup any files created prior to the packet creation failure
if packetErr := h.PaymentPacketCreator.CleanupPaymentPacketDir(packetPath); packetErr != nil {
appCtx.Logger().Error("Error: cleaning up Payment Packet files", zap.Error(packetErr))
}

switch err.(type) {
case apperror.NotFoundError:
// this indicates ppm was not found
Expand All @@ -222,6 +245,16 @@ func (h ShowPaymentPacketHandler) Handle(params ppmdocumentops.ShowPaymentPacket
}

payload := io.NopCloser(pdf)

// we have copied the created files into the payload so we can remove them from memory
if err = h.PaymentPacketCreator.CleanupPaymentPacketDir(packetPath); err != nil {
appCtx.Logger().Error("Error: cleaning up Payment Packet files", zap.Error(err))
errInstance := fmt.Sprintf("Instance: %s", h.GetTraceIDFromRequest(params.HTTPRequest))
errPayload := &ghcmessages.Error{Message: &errInstance}
return ppmdocumentops.NewShowAOAPacketInternalServerError().
WithPayload(errPayload), err
}

filename := fmt.Sprintf("inline; filename=\"ppm_payment_packet-%s.pdf\"", time.Now().UTC().Format("2006-01-02T15:04:05.000Z"))

return ppmdocumentops.NewShowPaymentPacketOK().WithContentDisposition(filename).WithPayload(payload), nil
Expand Down
21 changes: 15 additions & 6 deletions pkg/handlers/ghcapi/ppm_document_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,8 @@ func (suite *HandlerSuite) TestShowAOAPacketHandler() {
AOAPacketCreator: &mockAOAPacketCreator,
}

mockAOAPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(nil, nil)
mockAOAPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(nil, "", nil)
mockAOAPacketCreator.On("CleanupAOAPacketDir", mock.AnythingOfType("string")).Return(nil)

// make the request
requestUser := factory.BuildUser(nil, nil, nil)
Expand Down Expand Up @@ -647,7 +648,8 @@ func (suite *HandlerSuite) TestShowAOAPacketHandler() {
AOAPacketCreator: &mockAOAPacketCreator,
}

mockAOAPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(nil, errors.New("Mock error"))
mockAOAPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(nil, "", errors.New("Mock error"))
mockAOAPacketCreator.On("CleanupAOAPacketDir", mock.AnythingOfType("string")).Return(nil)

// make the request
requestUser := factory.BuildUser(nil, nil, nil)
Expand Down Expand Up @@ -677,7 +679,8 @@ func (suite *HandlerSuite) TestShowAOAPacketHandler() {
AOAPacketCreator: &mockAOAPacketCreator,
}

mockAOAPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(nil, errors.New("Mock error"))
mockAOAPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(nil, "", errors.New("Mock error"))
mockAOAPacketCreator.On("CleanupAOAPacketDir", mock.AnythingOfType("string")).Return(nil)

// make the request
requestUser := factory.BuildUser(nil, nil, nil)
Expand Down Expand Up @@ -709,7 +712,9 @@ func (suite *HandlerSuite) TestShowPaymentPacketHandler() {

mockPaymentPacketCreator.On("GenerateDefault",
mock.AnythingOfType("*appcontext.appContext"),
mock.AnythingOfType("uuid.UUID")).Return(nil, nil)
mock.AnythingOfType("uuid.UUID")).Return(nil, "", nil)

mockPaymentPacketCreator.On("CleanupPaymentPacketDir", mock.AnythingOfType("string")).Return(nil)

// make the request
requestUser := factory.BuildUser(nil, nil, nil)
Expand All @@ -735,7 +740,9 @@ func (suite *HandlerSuite) TestShowPaymentPacketHandler() {

mockPaymentPacketCreator.On("GenerateDefault",
mock.AnythingOfType("*appcontext.appContext"),
mock.AnythingOfType("uuid.UUID")).Return(nil, errors.New("Mock error"))
mock.AnythingOfType("uuid.UUID")).Return(nil, "", errors.New("Mock error"))

mockPaymentPacketCreator.On("CleanupPaymentPacketDir", mock.AnythingOfType("string")).Return(nil)

// make the request
requestUser := factory.BuildUser(nil, nil, nil)
Expand All @@ -761,7 +768,9 @@ func (suite *HandlerSuite) TestShowPaymentPacketHandler() {

mockPaymentPacketCreator.On("GenerateDefault",
mock.AnythingOfType("*appcontext.appContext"),
mock.AnythingOfType("uuid.UUID")).Return(nil, apperror.NotFoundError{})
mock.AnythingOfType("uuid.UUID")).Return(nil, "", apperror.NotFoundError{})

mockPaymentPacketCreator.On("CleanupPaymentPacketDir", mock.AnythingOfType("string")).Return(nil)

// make the request
requestUser := factory.BuildUser(nil, nil, nil)
Expand Down
44 changes: 42 additions & 2 deletions pkg/handlers/internalapi/ppm_shipment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,16 +294,34 @@ func (h showAOAPacketHandler) Handle(params ppmops.ShowAOAPacketParams) middlewa
err.Error(), h.GetTraceIDFromRequest(params.HTTPRequest))), err
}

AOAPacket, err := h.AOAPacketCreator.CreateAOAPacket(appCtx, ppmShipmentID, false)
AOAPacket, packetPath, err := h.AOAPacketCreator.CreateAOAPacket(appCtx, ppmShipmentID, false)

if err != nil {
logger.Error("Error creating AOA", zap.Error(err))
aoaError := err.Error()

// need to cleanup any files created prior to the packet creation failure
if err = h.AOAPacketCreator.CleanupAOAPacketDir(packetPath); err != nil {
logger.Error("Error: cleaning up AOA files", zap.Error(err))
aoaError = aoaError + ": " + err.Error()
}

payload := payloads.InternalServerError(&aoaError, h.GetTraceIDFromRequest(params.HTTPRequest))
return ppmops.NewShowAOAPacketInternalServerError().
WithPayload(payload), err
}

payload := io.NopCloser(AOAPacket)

// we have copied the created files into the payload so we can remove them from memory
if err = h.AOAPacketCreator.CleanupAOAPacketDir(packetPath); err != nil {
logger.Error("Error: cleaning up AOA files", zap.Error(err))
aoaError := err.Error()
payload := payloads.InternalServerError(&aoaError, h.GetTraceIDFromRequest(params.HTTPRequest))
return ppmops.NewShowAOAPacketInternalServerError().
WithPayload(payload), err
}

filename := fmt.Sprintf("inline; filename=\"AOA-%s.pdf\"", time.Now().Format("01-02-2006_15-04-05"))

return ppmops.NewShowAOAPacketOK().WithContentDisposition(filename).WithPayload(payload), nil
Expand All @@ -325,8 +343,23 @@ func (h ShowPaymentPacketHandler) Handle(params ppmops.ShowPaymentPacketParams)
return handlers.ResponseForError(appCtx.Logger(), err), err
}

pdf, err := h.PaymentPacketCreator.GenerateDefault(appCtx, ppmShipmentID)
pdf, packetPath, err := h.PaymentPacketCreator.GenerateDefault(appCtx, ppmShipmentID)

defer func() {
// if a panic occurred we need to cleanup the files
if r := recover(); r != nil {
if packetErr := h.PaymentPacketCreator.CleanupPaymentPacketDir(packetPath); packetErr != nil {
appCtx.Logger().Error("Panic: cleaning up Payment Packet files", zap.Error(packetErr))
}
}
}()

if err != nil {
// need to cleanup any files created prior to the packet creation failure
if packetErr := h.PaymentPacketCreator.CleanupPaymentPacketDir(packetPath); packetErr != nil {
appCtx.Logger().Error("Error: cleaning up Payment Packet files", zap.Error(packetErr))
}

switch err.(type) {
case apperror.ForbiddenError:
// this indicates user does not have access to PPM
Expand All @@ -343,6 +376,13 @@ func (h ShowPaymentPacketHandler) Handle(params ppmops.ShowPaymentPacketParams)
}

payload := io.NopCloser(pdf)

// we have copied the created files into the payload so we can remove them from memory
if err = h.PaymentPacketCreator.CleanupPaymentPacketDir(packetPath); err != nil {
appCtx.Logger().Error(fmt.Sprintf("internalapi.DownPaymentPacket InternalServerError failed to clean up payment packet files for ppmShipmentID:%s", ppmShipmentID.String()), zap.Error(err))
return ppmops.NewShowPaymentPacketInternalServerError(), err
}

filename := fmt.Sprintf("inline; filename=\"ppm_payment_packet-%s.pdf\"", time.Now().UTC().Format("2006-01-02T15:04:05.000Z"))

return ppmops.NewShowPaymentPacketOK().WithContentDisposition(filename).WithPayload(payload), nil
Expand Down
24 changes: 17 additions & 7 deletions pkg/handlers/internalapi/ppm_shipment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1002,7 +1002,8 @@ func (suite *HandlerSuite) TestShowAOAPacketHandler() {
}

mockAOAPacketCreator.On("VerifyAOAPacketInternal", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID")).Return(nil)
mockAOAPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(nil, nil)
mockAOAPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(nil, "", nil)
mockAOAPacketCreator.On("CleanupAOAPacketDir", mock.AnythingOfType("string")).Return(nil)

// make the request
requestUser := factory.BuildUser(nil, nil, nil)
Expand Down Expand Up @@ -1038,7 +1039,8 @@ func (suite *HandlerSuite) TestShowAOAPacketHandler() {
}

mockAOAPacketCreator.On("VerifyAOAPacketInternal", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID")).Return(nil)
mockAOAPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(nil, errors.New("Mock error"))
mockAOAPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(nil, "", errors.New("Mock error"))
mockAOAPacketCreator.On("CleanupAOAPacketDir", mock.AnythingOfType("string")).Return(nil)

// make the request
requestUser := factory.BuildUser(nil, nil, nil)
Expand Down Expand Up @@ -1099,7 +1101,8 @@ func (suite *HandlerSuite) TestShowAOAPacketHandler() {
}

mockAOAPacketCreator.On("VerifyAOAPacketInternal", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID")).Return(nil)
mockAOAPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(nil, errors.New("Mock error"))
mockAOAPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(nil, "", errors.New("Mock error"))
mockAOAPacketCreator.On("CleanupAOAPacketDir", mock.AnythingOfType("string")).Return(nil)

// make the request
requestUser := factory.BuildUser(nil, nil, nil)
Expand Down Expand Up @@ -1131,7 +1134,9 @@ func (suite *HandlerSuite) TestShowPaymentPacketHandler() {

mockPaymentPacketCreator.On("GenerateDefault",
mock.AnythingOfType("*appcontext.appContext"),
mock.AnythingOfType("uuid.UUID")).Return(nil, nil)
mock.AnythingOfType("uuid.UUID")).Return(nil, "", nil)

mockPaymentPacketCreator.On("CleanupPaymentPacketDir", mock.AnythingOfType("string")).Return(nil)

// make the request
requestUser := factory.BuildUser(nil, nil, nil)
Expand All @@ -1157,8 +1162,9 @@ func (suite *HandlerSuite) TestShowPaymentPacketHandler() {

mockPaymentPacketCreator.On("GenerateDefault",
mock.AnythingOfType("*appcontext.appContext"),
mock.AnythingOfType("uuid.UUID")).Return(nil, errors.New("Mock error"))
mock.AnythingOfType("uuid.UUID")).Return(nil, "", errors.New("Mock error"))

mockPaymentPacketCreator.On("CleanupPaymentPacketDir", mock.AnythingOfType("string")).Return(nil)
// make the request
requestUser := factory.BuildUser(nil, nil, nil)
ppmshipmentid := ppmShipment.ID
Expand All @@ -1183,7 +1189,9 @@ func (suite *HandlerSuite) TestShowPaymentPacketHandler() {

mockPaymentPacketCreator.On("GenerateDefault",
mock.AnythingOfType("*appcontext.appContext"),
mock.AnythingOfType("uuid.UUID")).Return(nil, apperror.NotFoundError{})
mock.AnythingOfType("uuid.UUID")).Return(nil, "", apperror.NotFoundError{})

mockPaymentPacketCreator.On("CleanupPaymentPacketDir", mock.AnythingOfType("string")).Return(nil)

// make the request
requestUser := factory.BuildUser(nil, nil, nil)
Expand All @@ -1209,7 +1217,9 @@ func (suite *HandlerSuite) TestShowPaymentPacketHandler() {

mockPaymentPacketCreator.On("GenerateDefault",
mock.AnythingOfType("*appcontext.appContext"),
mock.AnythingOfType("uuid.UUID")).Return(nil, apperror.ForbiddenError{})
mock.AnythingOfType("uuid.UUID")).Return(nil, "", apperror.ForbiddenError{})

mockPaymentPacketCreator.On("CleanupPaymentPacketDir", mock.AnythingOfType("string")).Return(nil)

// make the request
requestUser := factory.BuildUser(nil, nil, nil)
Expand Down
20 changes: 20 additions & 0 deletions pkg/handlers/internalapi/uploads.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,13 +335,26 @@ func (h CreatePPMUploadHandler) Handle(params ppmop.CreatePPMUploadParams) middl

// Ensure weight receipt PDF is not corrupted
if err != nil || pdfInfo.PageCount != weightEstimatePages {
cleanupErr := h.WeightTicketGenerator.CleanupFile(aFile)

if cleanupErr != nil {
appCtx.Logger().Warn("failed to cleanup weight ticket file", zap.Error(cleanupErr), zap.String("verrs", verrs.Error()))
}

return ppmop.NewCreatePPMUploadInternalServerError(), rollbackErr
}

// we already generated an afero file so we can skip that process the wrapper method does
newUserUpload, verrs, createErr = h.UserUploader.CreateUserUploadForDocument(appCtx, &document.ID, appCtx.Session().UserID, uploaderpkg.File{File: aFile}, uploaderpkg.AllowedTypesPPMDocuments)

if verrs.HasAny() || createErr != nil {
appCtx.Logger().Error("failed to create new user upload", zap.Error(createErr), zap.String("verrs", verrs.Error()))
cleanupErr := h.WeightTicketGenerator.CleanupFile(aFile)

if cleanupErr != nil {
appCtx.Logger().Warn("failed to cleanup weight ticket file", zap.Error(cleanupErr), zap.String("verrs", verrs.Error()))
return ppmop.NewCreatePPMUploadInternalServerError(), rollbackErr
}
switch createErr.(type) {
case uploaderpkg.ErrUnsupportedContentType:
return ppmop.NewCreatePPMUploadUnprocessableEntity().WithPayload(payloads.ValidationError(createErr.Error(), uuid.Nil, verrs)), createErr
Expand All @@ -356,6 +369,13 @@ func (h CreatePPMUploadHandler) Handle(params ppmop.CreatePPMUploadParams) middl
}
}

err = h.WeightTicketGenerator.CleanupFile(aFile)

if err != nil {
appCtx.Logger().Warn("failed to cleanup weight ticket file", zap.Error(err), zap.String("verrs", verrs.Error()))
return ppmop.NewCreatePPMUploadInternalServerError(), rollbackErr
}

url, err = h.UserUploader.PresignedURL(appCtx, newUserUpload)

if err != nil {
Expand Down
Loading

0 comments on commit cac075e

Please sign in to comment.