diff --git a/cmd/generate-shipment-summary/main.go b/cmd/generate-shipment-summary/main.go index d9776cd4150..80c2a366970 100644 --- a/cmd/generate-shipment-summary/main.go +++ b/cmd/generate-shipment-summary/main.go @@ -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 diff --git a/cmd/image-to-pdf/main.go b/cmd/image-to-pdf/main.go index 307706e5db5..1c42c60e586 100644 --- a/cmd/image-to-pdf/main.go +++ b/cmd/image-to-pdf/main.go @@ -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()) } diff --git a/pkg/handlers/ghcapi/payment_request.go b/pkg/handlers/ghcapi/payment_request.go index 622d38b1abc..9279bad80fb 100644 --- a/pkg/handlers/ghcapi/payment_request.go +++ b/pkg/handlers/ghcapi/payment_request.go @@ -283,9 +283,15 @@ 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(). @@ -293,6 +299,12 @@ func (h PaymentRequestBulkDownloadHandler) Handle(params paymentrequestop.BulkDo } 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 diff --git a/pkg/handlers/ghcapi/ppm_document.go b/pkg/handlers/ghcapi/ppm_document.go index ee92f7e5d29..c355d065a7e 100644 --- a/pkg/handlers/ghcapi/ppm_document.go +++ b/pkg/handlers/ghcapi/ppm_document.go @@ -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 @@ -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 @@ -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 diff --git a/pkg/handlers/ghcapi/ppm_document_test.go b/pkg/handlers/ghcapi/ppm_document_test.go index 576a792f43c..26f12dbbe91 100644 --- a/pkg/handlers/ghcapi/ppm_document_test.go +++ b/pkg/handlers/ghcapi/ppm_document_test.go @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) diff --git a/pkg/handlers/internalapi/ppm_shipment.go b/pkg/handlers/internalapi/ppm_shipment.go index 33480ae9830..b8199b63468 100644 --- a/pkg/handlers/internalapi/ppm_shipment.go +++ b/pkg/handlers/internalapi/ppm_shipment.go @@ -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 @@ -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 @@ -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 diff --git a/pkg/handlers/internalapi/ppm_shipment_test.go b/pkg/handlers/internalapi/ppm_shipment_test.go index b9e20809025..5f311e8b437 100644 --- a/pkg/handlers/internalapi/ppm_shipment_test.go +++ b/pkg/handlers/internalapi/ppm_shipment_test.go @@ -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) @@ -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) @@ -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) @@ -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) @@ -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 @@ -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) @@ -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) diff --git a/pkg/handlers/internalapi/uploads.go b/pkg/handlers/internalapi/uploads.go index 4d248598ed6..ce65ec91bc0 100644 --- a/pkg/handlers/internalapi/uploads.go +++ b/pkg/handlers/internalapi/uploads.go @@ -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 @@ -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 { diff --git a/pkg/handlers/primeapi/move_task_order.go b/pkg/handlers/primeapi/move_task_order.go index d82f2b72fb8..96d03dbf4c8 100644 --- a/pkg/handlers/primeapi/move_task_order.go +++ b/pkg/handlers/primeapi/move_task_order.go @@ -330,9 +330,17 @@ func (h DownloadMoveOrderHandler) Handle(params movetaskorderops.DownloadMoveOrd moveOrderUploadType = services.MoveOrderAmendmentUpload } - outputFile, err := h.PrimeDownloadMoveUploadPDFGenerator.GenerateDownloadMoveUserUploadPDF(appCtx, moveOrderUploadType, move, true) + dirName := uuid.Must(uuid.NewV4()).String() + outputFile, err := h.PrimeDownloadMoveUploadPDFGenerator.GenerateDownloadMoveUserUploadPDF(appCtx, moveOrderUploadType, move, true, dirName) if err != nil { + + cleanupErr := h.PrimeDownloadMoveUploadPDFGenerator.CleanupFile(outputFile) + + if cleanupErr != nil { + appCtx.Logger().Warn("primeapi.DownloadMoveOrder warn", zap.Error(cleanupErr)) + } + switch e := err.(type) { case apperror.UnprocessableEntityError: appCtx.Logger().Warn("primeapi.DownloadMoveOrder warn", zap.Error(err)) @@ -347,6 +355,14 @@ func (h DownloadMoveOrderHandler) Handle(params movetaskorderops.DownloadMoveOrd payload := io.NopCloser(outputFile) + err = h.PrimeDownloadMoveUploadPDFGenerator.CleanupFile(outputFile) + + if err != nil { + appCtx.Logger().Error("primeapi.DownloadMoveOrder error", zap.Error(err)) + return movetaskorderops.NewDownloadMoveOrderInternalServerError().WithPayload( + payloads.InternalServerError(nil, h.GetTraceIDFromRequest(params.HTTPRequest))), err + } + // Build fileName in format: Customer-{type}-for-MTO-{locator}-{TIMESTAMP}.pdf // example: // Customer-ORDERS,AMENDMENTS-for-MTO-PPMSIT-2024-01-11T17-02.pdf (all) diff --git a/pkg/handlers/primeapi/move_task_order_test.go b/pkg/handlers/primeapi/move_task_order_test.go index 1a945ca44ef..58a58a2cb07 100644 --- a/pkg/handlers/primeapi/move_task_order_test.go +++ b/pkg/handlers/primeapi/move_task_order_test.go @@ -10,6 +10,7 @@ import ( "github.com/gobuffalo/validate/v3" "github.com/gofrs/uuid" "github.com/pkg/errors" + "github.com/spf13/afero" "github.com/stretchr/testify/mock" "github.com/transcom/mymove/pkg/apperror" @@ -2030,6 +2031,8 @@ func (suite *HandlerSuite) TestUpdateMTOPostCounselingInfo() { func (suite *HandlerSuite) TestDownloadMoveOrderHandler() { uri := "/moves/%s/documents" paramTypeAll := "ALL" + fs := afero.NewMemMapFs() + suite.Run("Successful DownloadMoveOrder - 200", func() { mockMoveSearcher := mocks.MoveSearcher{} mockOrderFetcher := mocks.OrderFetcher{} @@ -2057,12 +2060,19 @@ func (suite *HandlerSuite) TestDownloadMoveOrderHandler() { }), ).Return(moves, 1, nil) + outputFile, err := fs.Create("testFile") + suite.NoError(err) + // mock to return nil Error mockPrimeDownloadMoveUploadPDFGenerator.On("GenerateDownloadMoveUserUploadPDF", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("services.MoveOrderUploadType"), mock.AnythingOfType("models.Move"), - mock.AnythingOfType("bool")).Return(nil, nil) + mock.AnythingOfType("bool"), + mock.AnythingOfType("string")).Return(outputFile, nil) + + mockPrimeDownloadMoveUploadPDFGenerator.On("CleanupFile", + mock.AnythingOfType("*mem.File")).Return(nil) // make the request requestUser := factory.BuildUser(nil, nil, nil) @@ -2107,12 +2117,19 @@ func (suite *HandlerSuite) TestDownloadMoveOrderHandler() { }), ).Return(moves, 1, nil) + outputFile, err := fs.Create("testFile") + suite.NoError(err) + // mock to return nil Error mockPrimeDownloadMoveUploadPDFGenerator.On("GenerateDownloadMoveUserUploadPDF", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("services.MoveOrderUploadType"), mock.AnythingOfType("models.Move"), - mock.AnythingOfType("bool")).Return(nil, errors.New("error")) + mock.AnythingOfType("bool"), + mock.AnythingOfType("string")).Return(outputFile, errors.New("error")) + + mockPrimeDownloadMoveUploadPDFGenerator.On("CleanupFile", + mock.AnythingOfType("*mem.File")).Return(nil) // make the request requestUser := factory.BuildUser(nil, nil, nil) @@ -2292,12 +2309,19 @@ func (suite *HandlerSuite) TestDownloadMoveOrderHandler() { }), ).Return(moves, 1, nil) - // mock to return nil Errro + outputFile, err := fs.Create("testFile") + suite.NoError(err) + + // mock to return nil Error mockPrimeDownloadMoveUploadPDFGenerator.On("GenerateDownloadMoveUserUploadPDF", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("services.MoveOrderUploadType"), mock.AnythingOfType("models.Move"), - mock.AnythingOfType("bool")).Return(nil, apperror.NewUnprocessableEntityError("test")) + mock.AnythingOfType("bool"), + mock.AnythingOfType("string")).Return(outputFile, apperror.NewUnprocessableEntityError("test")) + + mockPrimeDownloadMoveUploadPDFGenerator.On("CleanupFile", + mock.AnythingOfType("*mem.File")).Return(nil) // make the request requestUser := factory.BuildUser(nil, nil, nil) @@ -2338,12 +2362,19 @@ func (suite *HandlerSuite) TestDownloadMoveOrderHandler() { }), ).Return(moves, 1, nil) - // mock to return nil Errro + outputFile, err := fs.Create("testFile") + suite.NoError(err) + + // mock to return nil Error mockPrimeDownloadMoveUploadPDFGenerator.On("GenerateDownloadMoveUserUploadPDF", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("services.MoveOrderUploadType"), mock.AnythingOfType("models.Move"), - mock.AnythingOfType("bool")).Return(nil, errors.New("test")) + mock.AnythingOfType("bool"), + mock.AnythingOfType("string")).Return(outputFile, errors.New("test")) + + mockPrimeDownloadMoveUploadPDFGenerator.On("CleanupFile", + mock.AnythingOfType("*mem.File")).Return(nil) // make the request requestUser := factory.BuildUser(nil, nil, nil) @@ -2385,12 +2416,19 @@ func (suite *HandlerSuite) TestDownloadMoveOrderHandler() { }), ).Return(moves, 1, nil) - // mock to return nil Errro + outputFile, err := fs.Create("testFile") + suite.NoError(err) + + // mock to return nil Error mockPrimeDownloadMoveUploadPDFGenerator.On("GenerateDownloadMoveUserUploadPDF", mock.AnythingOfType("*appcontext.appContext"), services.MoveOrderUploadAll, //Verify ALL enum is used mock.AnythingOfType("models.Move"), - mock.AnythingOfType("bool")).Return(nil, errors.New("test")) + mock.AnythingOfType("bool"), + mock.AnythingOfType("string")).Return(outputFile, errors.New("test")) + + mockPrimeDownloadMoveUploadPDFGenerator.On("CleanupFile", + mock.AnythingOfType("*mem.File")).Return(nil) // make the request requestUser := factory.BuildUser(nil, nil, nil) @@ -2432,12 +2470,19 @@ func (suite *HandlerSuite) TestDownloadMoveOrderHandler() { }), ).Return(moves, 1, nil) - // mock to return nil Errro + outputFile, err := fs.Create("testFile") + suite.NoError(err) + + // mock to return nil Error mockPrimeDownloadMoveUploadPDFGenerator.On("GenerateDownloadMoveUserUploadPDF", mock.AnythingOfType("*appcontext.appContext"), services.MoveOrderUpload, //Verify Order only enum is used mock.AnythingOfType("models.Move"), - mock.AnythingOfType("bool")).Return(nil, errors.New("test")) + mock.AnythingOfType("bool"), + mock.AnythingOfType("string")).Return(outputFile, errors.New("test")) + + mockPrimeDownloadMoveUploadPDFGenerator.On("CleanupFile", + mock.AnythingOfType("*mem.File")).Return(nil) // make the request requestUser := factory.BuildUser(nil, nil, nil) @@ -2456,7 +2501,7 @@ func (suite *HandlerSuite) TestDownloadMoveOrderHandler() { suite.Assertions.IsType(&movetaskorderops.DownloadMoveOrderInternalServerError{}, downloadMoveOrderResponse) }) - suite.Run("DownloadMoveOrder: Orders Only - service returns unprocess entity - 422", func() { + suite.Run("DownloadMoveOrder: Amendments Only - service returns unprocess entity - 422", func() { mockMoveSearcher := mocks.MoveSearcher{} mockOrderFetcher := mocks.OrderFetcher{} mockPrimeDownloadMoveUploadPDFGenerator := mocks.PrimeDownloadMoveUploadPDFGenerator{} @@ -2480,12 +2525,19 @@ func (suite *HandlerSuite) TestDownloadMoveOrderHandler() { }), ).Return(moves, 1, nil) - // mock to return nil Errro + outputFile, err := fs.Create("testFile") + suite.NoError(err) + + // mock to return nil Error mockPrimeDownloadMoveUploadPDFGenerator.On("GenerateDownloadMoveUserUploadPDF", mock.AnythingOfType("*appcontext.appContext"), services.MoveOrderAmendmentUpload, //Verify Amendment only enum is used mock.AnythingOfType("models.Move"), - mock.AnythingOfType("bool")).Return(nil, errors.New("test")) + mock.AnythingOfType("bool"), + mock.AnythingOfType("string")).Return(outputFile, errors.New("test")) + + mockPrimeDownloadMoveUploadPDFGenerator.On("CleanupFile", + mock.AnythingOfType("*mem.File")).Return(nil) // make the request requestUser := factory.BuildUser(nil, nil, nil) diff --git a/pkg/paperwork/generator.go b/pkg/paperwork/generator.go index 54b3750860b..57596e469b7 100644 --- a/pkg/paperwork/generator.go +++ b/pkg/paperwork/generator.go @@ -19,7 +19,6 @@ import ( "github.com/pdfcpu/pdfcpu/pkg/api" "github.com/pdfcpu/pdfcpu/pkg/pdfcpu" "github.com/pdfcpu/pdfcpu/pkg/pdfcpu/model" - "github.com/pdfcpu/pdfcpu/pkg/pdfcpu/types" "github.com/pkg/errors" "github.com/spf13/afero" "go.uber.org/zap" @@ -149,15 +148,45 @@ type inputFile struct { ContentType string } -func (g *Generator) newTempFile() (afero.File, error) { - outputFile, err := g.fs.TempFile(g.workDir, "temp") +// get the working directory path +func (g *Generator) GetWorkDir() string { + return g.workDir +} + +// creates the directory if it does not exist and creates a new file in that directory +func (g *Generator) newTempFileInDir(dirName string) (afero.File, error) { + dirPath := g.workDir + + if dirName != "" { + dirPath = dirPath + "/" + dirName + } + + // Check if directory exists + exists, err := afero.DirExists(g.fs, dirPath) + + if err != nil { + return nil, err + } + + if !exists { + // Create a directory with permissions 0755 (read/write/execute for owner, read/execute for group/others) + err := g.fs.Mkdir(dirPath, 0755) + + if err != nil { + return nil, err + } + } + + outputFile, err := g.fs.TempFile(dirPath, "temp") + if err != nil { return nil, errors.WithStack(err) } + return outputFile, nil } -func (g *Generator) newTempFileWithName(fileName string) (afero.File, error) { +func (g *Generator) newTempFileWithNameInDir(dirName string, fileName string) (afero.File, error) { name := "temp" if fileName != "" { @@ -166,7 +195,29 @@ func (g *Generator) newTempFileWithName(fileName string) (afero.File, error) { name = fileName[:extensionIndex] + strings.Replace(fileName[extensionIndex:], ".", "*.", 1) } - outputFile, err := g.fs.TempFile(g.workDir, name) + dirPath := g.workDir + + if dirPath != "" { + dirPath = dirPath + "/" + dirName + } + + // Check if directory exists + exists, err := afero.DirExists(g.fs, dirPath) + + if err != nil { + return nil, err + } + + if !exists { + // Create a directory with permissions 0755 (read/write/execute for owner, read/execute for group/others) + err := g.fs.Mkdir(dirPath, 0755) + + if err != nil { + return nil, err + } + } + + outputFile, err := g.fs.TempFile(dirPath, name) if err != nil { return nil, errors.WithStack(err) } @@ -184,7 +235,7 @@ func (g *Generator) FileSystem() *afero.Afero { } // Add bookmarks into a single PDF -func (g *Generator) AddPdfBookmarks(inputFile afero.File, bookmarks []pdfcpu.Bookmark) (afero.File, error) { +func (g *Generator) AddPdfBookmarks(inputFile afero.File, bookmarks []pdfcpu.Bookmark, dirName string) (afero.File, error) { buf := new(bytes.Buffer) replace := true @@ -193,7 +244,7 @@ func (g *Generator) AddPdfBookmarks(inputFile afero.File, bookmarks []pdfcpu.Boo return nil, errors.Wrap(err, "error pdfcpu.api.AddBookmarks") } - tempFile, err := g.newTempFile() + tempFile, err := g.newTempFileInDir(dirName) if err != nil { return nil, err } @@ -238,13 +289,13 @@ func (g *Generator) GetPdfFileInfoByContents(file afero.File) (*pdfcpu.PDFInfo, } // CreateMergedPDFUpload converts Uploads to PDF and merges them into a single PDF -func (g *Generator) CreateMergedPDFUpload(appCtx appcontext.AppContext, uploads models.Uploads) (afero.File, error) { - pdfs, err := g.ConvertUploadsToPDF(appCtx, uploads, true) +func (g *Generator) CreateMergedPDFUpload(appCtx appcontext.AppContext, uploads models.Uploads, dirName string) (afero.File, error) { + pdfs, err := g.ConvertUploadsToPDF(appCtx, uploads, true, dirName) if err != nil { return nil, errors.Wrap(err, "Error while converting uploads") } - mergedPdf, err := g.MergePDFFiles(appCtx, pdfs) + mergedPdf, err := g.MergePDFFiles(appCtx, pdfs, dirName) if err != nil { return nil, errors.Wrap(err, "Error while merging PDFs") } @@ -253,7 +304,7 @@ func (g *Generator) CreateMergedPDFUpload(appCtx appcontext.AppContext, uploads } // ConvertUploadsToPDF turns a slice of Uploads into a slice of paths to converted PDF files -func (g *Generator) ConvertUploadsToPDF(appCtx appcontext.AppContext, uploads models.Uploads, doRotation bool) ([]string, error) { +func (g *Generator) ConvertUploadsToPDF(appCtx appcontext.AppContext, uploads models.Uploads, doRotation bool, dirName string) ([]string, error) { // tempfile paths to be returned pdfs := make([]string, 0) @@ -269,12 +320,12 @@ func (g *Generator) ConvertUploadsToPDF(appCtx appcontext.AppContext, uploads mo var pdf string var err error if doRotation { - pdf, err = g.PDFFromImages(appCtx, images) + pdf, err = g.PDFFromImages(appCtx, images, dirName) if err != nil { return nil, errors.Wrap(err, "Converting images") } } else { - pdf, err = g.PDFFromImagesNoRotation(appCtx, images) + pdf, err = g.PDFFromImagesNoRotation(appCtx, images, dirName) if err != nil { return nil, errors.Wrap(err, "Converting images") } @@ -295,7 +346,7 @@ func (g *Generator) ConvertUploadsToPDF(appCtx appcontext.AppContext, uploads mo } }() - outputFile, err := g.newTempFile() + outputFile, err := g.newTempFileInDir(dirName) if err != nil { return nil, errors.Wrap(err, "Creating temp file") @@ -321,12 +372,12 @@ func (g *Generator) ConvertUploadsToPDF(appCtx appcontext.AppContext, uploads mo var err error if doRotation { - pdf, err = g.PDFFromImages(appCtx, images) + pdf, err = g.PDFFromImages(appCtx, images, dirName) if err != nil { return nil, errors.Wrap(err, "Converting remaining images to pdf") } } else { - pdf, err = g.PDFFromImagesNoRotation(appCtx, images) + pdf, err = g.PDFFromImagesNoRotation(appCtx, images, dirName) if err != nil { return nil, errors.Wrap(err, "Converting remaining images to pdf") } @@ -348,20 +399,14 @@ func (g *Generator) ConvertUploadsToPDF(appCtx appcontext.AppContext, uploads mo return pdfs, nil } -func (g *Generator) ConvertUploadToPDF(appCtx appcontext.AppContext, upload models.Upload) (string, error) { +func (g *Generator) ConvertUploadToPDF(appCtx appcontext.AppContext, upload models.Upload, dirName string) (string, error) { download, err := g.uploader.Download(appCtx, &upload) if err != nil { return "nil", errors.Wrap(err, "Downloading file from upload") } - defer func() { - if downloadErr := download.Close(); downloadErr != nil { - appCtx.Logger().Debug("Failed to close file", zap.Error(downloadErr)) - } - }() - - outputFile, err := g.newTempFile() + outputFile, err := g.newTempFileInDir(dirName) if err != nil { return "nil", errors.Wrap(err, "Creating temp file") @@ -380,7 +425,14 @@ func (g *Generator) ConvertUploadToPDF(appCtx appcontext.AppContext, upload mode images := make([]inputFile, 0) images = append(images, inputFile{Path: path, ContentType: upload.ContentType}) - return g.PDFFromImages(appCtx, images) + + defer func() { + if downloadErr := download.Close(); downloadErr != nil { + appCtx.Logger().Debug("Failed to close file", zap.Error(downloadErr)) + } + }() + + return g.PDFFromImages(appCtx, images, dirName) } // convert between image MIME types and the values expected by gofpdf @@ -390,7 +442,7 @@ var contentTypeToImageType = map[string]string{ } // ReduceUnusedSpace reduces unused space -func ReduceUnusedSpace(_ appcontext.AppContext, file afero.File, g *Generator, contentType string) (imgFile afero.File, width float64, height float64, err error) { +func ReduceUnusedSpace(_ appcontext.AppContext, file afero.File, g *Generator, contentType string, dirName string) (imgFile afero.File, width float64, height float64, err error) { // Figure out if the image should be rotated by calculating height and width of image. pic, _, decodeErr := image.Decode(file) if decodeErr != nil { @@ -402,7 +454,7 @@ func ReduceUnusedSpace(_ appcontext.AppContext, file afero.File, g *Generator, c // If the image is landscape, then turn it to portrait orientation if w > h { - newFile, newTemplateFileErr := g.newTempFile() + newFile, newTemplateFileErr := g.newTempFileInDir(dirName) if newTemplateFileErr != nil { return nil, 0.0, 0.0, errors.Wrap(newTemplateFileErr, "Creating temp file for image rotation") } @@ -444,7 +496,7 @@ func ReduceUnusedSpace(_ appcontext.AppContext, file afero.File, g *Generator, c // // The files at those paths will be tempfiles that will need to be cleaned // up by the caller. -func (g *Generator) PDFFromImages(appCtx appcontext.AppContext, images []inputFile) (string, error) { +func (g *Generator) PDFFromImages(appCtx appcontext.AppContext, images []inputFile, dirName string) (string, error) { // These constants are based on A4 page size, which we currently default to. horizontalMargin := 0.0 topMargin := 0.0 @@ -461,7 +513,7 @@ func (g *Generator) PDFFromImages(appCtx appcontext.AppContext, images []inputFi appCtx.Logger().Debug("generating PDF from image files", zap.Any("images", images)) - outputFile, err := g.newTempFile() + outputFile, err := g.newTempFileInDir(dirName) if err != nil { return "", err } @@ -489,7 +541,7 @@ func (g *Generator) PDFFromImages(appCtx appcontext.AppContext, images []inputFi if img.ContentType == uploader.FileTypePNG { appCtx.Logger().Debug("Converting png to 8-bit") // gofpdf isn't able to process 16-bit PNGs, so to be safe we convert all PNGs to an 8-bit color depth - newFile, newTemplateFileErr := g.newTempFile() + newFile, newTemplateFileErr := g.newTempFileInDir(dirName) if newTemplateFileErr != nil { return "", errors.Wrap(newTemplateFileErr, "Creating temp file for png conversion") } @@ -511,7 +563,7 @@ func (g *Generator) PDFFromImages(appCtx appcontext.AppContext, images []inputFi } } - optimizedFile, w, h, rotateErr := ReduceUnusedSpace(appCtx, file, g, img.ContentType) + optimizedFile, w, h, rotateErr := ReduceUnusedSpace(appCtx, file, g, img.ContentType, dirName) if rotateErr != nil { return "", errors.Wrapf(rotateErr, "Rotating image if in landscape orientation") } @@ -564,7 +616,7 @@ func (g *Generator) PDFFromImages(appCtx appcontext.AppContext, images []inputFi // // The files at those paths will be tempfiles that will need to be cleaned // up by the caller. -func (g *Generator) PDFFromImagesNoRotation(appCtx appcontext.AppContext, images []inputFile) (string, error) { +func (g *Generator) PDFFromImagesNoRotation(appCtx appcontext.AppContext, images []inputFile, dirName string) (string, error) { // These constants are based on A4 page size, which we currently default to. horizontalMargin := 0.0 topMargin := 0.0 @@ -581,7 +633,7 @@ func (g *Generator) PDFFromImagesNoRotation(appCtx appcontext.AppContext, images appCtx.Logger().Debug("generating PDF from image files", zap.Any("images", images)) - outputFile, err := g.newTempFile() + outputFile, err := g.newTempFileInDir(dirName) if err != nil { return "", err } @@ -609,7 +661,7 @@ func (g *Generator) PDFFromImagesNoRotation(appCtx appcontext.AppContext, images if img.ContentType == uploader.FileTypePNG { appCtx.Logger().Debug("Converting png to 8-bit") // gofpdf isn't able to process 16-bit PNGs, so to be safe we convert all PNGs to an 8-bit color depth - newFile, newTemplateFileErr := g.newTempFile() + newFile, newTemplateFileErr := g.newTempFileInDir(dirName) if newTemplateFileErr != nil { return "", errors.Wrap(newTemplateFileErr, "Creating temp file for png conversion") } @@ -669,13 +721,15 @@ func (g *Generator) PDFFromImagesNoRotation(appCtx appcontext.AppContext, images } // MergePDFFiles Merges a slice of paths to PDF files into a single PDF -func (g *Generator) MergePDFFiles(_ appcontext.AppContext, paths []string) (afero.File, error) { +func (g *Generator) MergePDFFiles(_ appcontext.AppContext, paths []string, dirName string) (afero.File, error) { var err error - mergedFile, err := g.newTempFile() + mergedFile, err := g.newTempFileInDir(dirName) if err != nil { return mergedFile, err } + defer mergedFile.Close() + var files []io.ReadSeeker for _, p := range paths { f, fileOpenErr := g.fs.Open(p) @@ -702,7 +756,7 @@ func (g *Generator) MergePDFFiles(_ appcontext.AppContext, paths []string) (afer // The content type of the image is inferred from its extension. If this proves to // be insufficient, storage.DetectContentType and contentTypeToImageType above can // be used. -func (g *Generator) MergeImagesToPDF(appCtx appcontext.AppContext, paths []string) (string, error) { +func (g *Generator) MergeImagesToPDF(appCtx appcontext.AppContext, paths []string, dirName string) (string, error) { // path and type for each image images := make([]inputFile, 0) @@ -714,10 +768,10 @@ func (g *Generator) MergeImagesToPDF(appCtx appcontext.AppContext, paths []strin }) } - return g.PDFFromImages(appCtx, images) + return g.PDFFromImages(appCtx, images, dirName) } -func (g *Generator) FillPDFForm(jsonData []byte, templateReader io.ReadSeeker, fileName string) (SSWWorksheet afero.File, err error) { +func (g *Generator) FillPDFForm(jsonData []byte, templateReader io.ReadSeeker, fileName string, dirName string) (SSWWorksheet afero.File, err error) { var conf = g.pdfConfig // Change type to reader readJSON := strings.NewReader(string(jsonData)) @@ -728,7 +782,7 @@ func (g *Generator) FillPDFForm(jsonData []byte, templateReader io.ReadSeeker, f return nil, formerr } - tempFile, err := g.newTempFileWithName(fileName) // Will use g.newTempFileWithName for proper memory usage, saves the new temp file with the fileName + tempFile, err := g.newTempFileWithNameInDir(dirName, fileName) // Will use g.newTempFileWithName for proper memory usage, saves the new temp file with the fileName if err != nil { return nil, err } @@ -750,7 +804,7 @@ func (g *Generator) FillPDFForm(jsonData []byte, templateReader io.ReadSeeker, f // LockPDFForm takes in a PDF Form readseeker, reads all form fields, and locks them // This is primarily for the SSW, but needs to be done separately from filling as only one process (filling, locking, merging, etc) // may be completed at a time. -func (g *Generator) LockPDFForm(templateReader io.ReadSeeker, fileName string) (SSWWorksheet afero.File, err error) { +func (g *Generator) LockPDFForm(templateReader io.ReadSeeker, fileName string, dirName string) (SSWWorksheet afero.File, err error) { var conf = g.pdfConfig buf := new(bytes.Buffer) // Reads all form fields on document as []form.Field @@ -771,7 +825,7 @@ func (g *Generator) LockPDFForm(templateReader io.ReadSeeker, fileName string) ( return nil, err } - tempFile, err := g.newTempFileWithName(fileName) // Will use g.newTempFileWithName for proper memory usage, saves the new temp file with the fileName + tempFile, err := g.newTempFileWithNameInDir(dirName, fileName) // Will use g.newTempFileWithName for proper memory usage, saves the new temp file with the fileName if err != nil { return nil, err } @@ -792,11 +846,11 @@ func (g *Generator) LockPDFForm(templateReader io.ReadSeeker, fileName string) ( } // MergePDFFiles Merges a slice of paths to PDF files into a single PDF -func (g *Generator) MergePDFFilesByContents(_ appcontext.AppContext, fileReaders []io.ReadSeeker) (afero.File, error) { +func (g *Generator) MergePDFFilesByContents(_ appcontext.AppContext, fileReaders []io.ReadSeeker, dirName string) (afero.File, error) { var err error // Create a merged file - mergedFile, err := g.newTempFile() + mergedFile, err := g.newTempFileInDir(dirName) if err != nil { return nil, err } @@ -815,66 +869,3 @@ func (g *Generator) MergePDFFilesByContents(_ appcontext.AppContext, fileReaders return mergedFile, nil } - -// Pdfcpu does not nil check watermarks as of version 0.9.1 -// This map allows us to preemptively nil check before calling the package -func createMapOfOnlyWatermarkedPages(m map[int][]*model.Watermark) map[int][]*model.Watermark { - validMap := make(map[int][]*model.Watermark) - for page, wms := range m { - // Skip entries where the slice is nil or empty - if len(wms) == 0 { - continue - } - - // Filter out nil pointers from the slice - validWms := []*model.Watermark{} - for _, wm := range wms { - if wm != nil { - validWms = append(validWms, wm) - } - } - - // Only add the page to the valid map if the filtered slice is not empty - if len(validWms) > 0 { - validMap[page] = validWms - } - } - return validMap -} - -func (g *Generator) AddWatermarks(inputFile afero.File, m map[int][]*model.Watermark) (afero.File, error) { - // Preemptive nil check for the map and its contents - watermarkMap := createMapOfOnlyWatermarkedPages(m) - if watermarkMap[0] == nil { - return nil, fmt.Errorf("no watermarks provided for generation") - } - - buf := new(bytes.Buffer) - err := api.AddWatermarksSliceMap(inputFile, buf, watermarkMap, g.pdfConfig) - if err != nil { - return nil, err - } - - tempFile, err := g.newTempFile() - if err != nil { - return nil, err - } - - // copy byte[] to temp file - _, err = io.Copy(tempFile, buf) - if err != nil { - return nil, errors.Wrap(err, "error io.Copy on byte[] to temp") - } - - // Reload the file from memstore - pdfWithWatermarks, err := g.fs.Open(tempFile.Name()) - if err != nil { - return nil, errors.Wrap(err, "error g.fs.Open on reload from memstore") - } - - return pdfWithWatermarks, nil -} - -func (g *Generator) CreateTextWatermark(text, desc string, onTop, update bool, u types.DisplayUnit) (*model.Watermark, error) { - return api.TextWatermark(text, desc, onTop, update, u) -} diff --git a/pkg/paperwork/generator_test.go b/pkg/paperwork/generator_test.go index 551adaf274d..021534d9a17 100644 --- a/pkg/paperwork/generator_test.go +++ b/pkg/paperwork/generator_test.go @@ -99,7 +99,7 @@ func (suite *PaperworkSuite) TestPDFFromImages() { suite.FatalNil(err) } - generatedPath, err := generator.PDFFromImages(suite.AppContextForTest(), images) + generatedPath, err := generator.PDFFromImages(suite.AppContextForTest(), images, "testDir") suite.FatalNil(err, "failed to generate pdf") aferoFile, err := generator.fs.Open(generatedPath) suite.FatalNil(err, "afero failed to open pdf") @@ -125,7 +125,6 @@ func (suite *PaperworkSuite) TestPDFFromImages() { suite.FatalNil(err, "Failed to read and validate the PDF file") // Temporarily assert 2 pages of PDF while optimization is disabled suite.Equal(2, pageCtx.PageCount, "Expected 2 pages in the PDF") - // TODO: // Uncomment the remainder of this test as it is the best form of validation for the PDF generation // BUTTTT it requires optimization to be enabled in pdfcpu. We had to disable it due to an overflow bug @@ -172,7 +171,7 @@ func (suite *PaperworkSuite) TestPDFFromImagesNoRotation() { suite.FatalNil(err) } - generatedPath, err := generator.PDFFromImagesNoRotation(suite.AppContextForTest(), images) + generatedPath, err := generator.PDFFromImagesNoRotation(suite.AppContextForTest(), images, "testDir") suite.FatalNil(err, "failed to generate pdf") aferoFile, err := generator.fs.Open(generatedPath) suite.FatalNil(err, "afero failed to open pdf") @@ -198,7 +197,6 @@ func (suite *PaperworkSuite) TestPDFFromImagesNoRotation() { suite.FatalNil(err, "Failed to read and validate the PDF file") // Temporarily assert 2 pages of PDF while optimization is disabled suite.Equal(2, pageCtx.PageCount, "Expected 2 pages in the PDF") - // TODO: Uncomment these lines below when the above lines are deleted // err = api.ExtractImagesFile(f.Name(), tmpDir, []string{"-2"}, generator.pdfConfig) // suite.FatalNil(err) @@ -242,7 +240,7 @@ func (suite *PaperworkSuite) TestPDFFromImages16BitPNG() { _, err = suite.openLocalFile(images[0].Path, generator.fs) suite.FatalNil(err) - generatedPath, err := generator.PDFFromImages(suite.AppContextForTest(), images) + generatedPath, err := generator.PDFFromImages(suite.AppContextForTest(), images, "testDir") suite.FatalNil(err, "failed to generate pdf") suite.NotEmpty(generatedPath, "got an empty path to the generated file") } @@ -263,7 +261,7 @@ func (suite *PaperworkSuite) TestPDFFromImagesRotation() { _, err = suite.openLocalFile(images[1].Path, generator.fs) suite.FatalNil(err) - generatedPath, err := generator.PDFFromImages(suite.AppContextForTest(), images) + generatedPath, err := generator.PDFFromImages(suite.AppContextForTest(), images, "testDir") suite.FatalNil(err, "failed to generate pdf") suite.NotEmpty(generatedPath, "got an empty path to the generated file") } @@ -273,7 +271,7 @@ func (suite *PaperworkSuite) TestGenerateUploadsPDF() { uploads, err := models.UploadsFromUserUploads(suite.DB(), order.UploadedOrders.UserUploads) suite.FatalNil(err) - paths, err := generator.ConvertUploadsToPDF(suite.AppContextForTest(), uploads, true) + paths, err := generator.ConvertUploadsToPDF(suite.AppContextForTest(), uploads, true, "testDir") suite.FatalNil(err) suite.Equal(3, len(paths), "wrong number of paths returned") @@ -284,7 +282,7 @@ func (suite *PaperworkSuite) TestCreateMergedPDF() { uploads, err := models.UploadsFromUserUploads(suite.DB(), order.UploadedOrders.UserUploads) suite.FatalNil(err) - file, err := generator.CreateMergedPDFUpload(suite.AppContextForTest(), uploads) + file, err := generator.CreateMergedPDFUpload(suite.AppContextForTest(), uploads, "testDir") suite.FatalNil(err) // Read merged file and verify page count @@ -312,7 +310,7 @@ func (suite *PaperworkSuite) TestCreateMergedPDFByContents() { files = append(files, file1) files = append(files, file2) - file, err := generator.MergePDFFilesByContents(suite.AppContextForTest(), files) + file, err := generator.MergePDFFilesByContents(suite.AppContextForTest(), files, "testDir") suite.FatalNil(err) // Read merged file and verify page count @@ -330,7 +328,7 @@ func (suite *PaperworkSuite) TestCleanup() { uploads, err := models.UploadsFromUserUploads(suite.DB(), order.UploadedOrders.UserUploads) suite.FatalNil(err) - _, err = generator.CreateMergedPDFUpload(suite.AppContextForTest(), uploads) + _, err = generator.CreateMergedPDFUpload(suite.AppContextForTest(), uploads, "testDir") suite.FatalNil(err) //RA Summary: gosec - errcheck - Unchecked return value diff --git a/pkg/services/mocks/AOAPacketCreator.go b/pkg/services/mocks/AOAPacketCreator.go index b1f23ed19dc..a2e76b10802 100644 --- a/pkg/services/mocks/AOAPacketCreator.go +++ b/pkg/services/mocks/AOAPacketCreator.go @@ -15,8 +15,44 @@ type AOAPacketCreator struct { mock.Mock } +// CleanupAOAPacketDir provides a mock function with given fields: dirName +func (_m *AOAPacketCreator) CleanupAOAPacketDir(dirName string) error { + ret := _m.Called(dirName) + + if len(ret) == 0 { + panic("no return value specified for CleanupAOAPacketDir") + } + + var r0 error + if rf, ok := ret.Get(0).(func(string) error); ok { + r0 = rf(dirName) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// CleanupAOAPacketFile provides a mock function with given fields: packetFile, closeFile +func (_m *AOAPacketCreator) CleanupAOAPacketFile(packetFile afero.File, closeFile bool) error { + ret := _m.Called(packetFile, closeFile) + + if len(ret) == 0 { + panic("no return value specified for CleanupAOAPacketFile") + } + + var r0 error + if rf, ok := ret.Get(0).(func(afero.File, bool) error); ok { + r0 = rf(packetFile, closeFile) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // CreateAOAPacket provides a mock function with given fields: appCtx, ppmShipmentID, isPaymentPacket -func (_m *AOAPacketCreator) CreateAOAPacket(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID, isPaymentPacket bool) (afero.File, error) { +func (_m *AOAPacketCreator) CreateAOAPacket(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID, isPaymentPacket bool) (afero.File, string, error) { ret := _m.Called(appCtx, ppmShipmentID, isPaymentPacket) if len(ret) == 0 { @@ -24,8 +60,9 @@ func (_m *AOAPacketCreator) CreateAOAPacket(appCtx appcontext.AppContext, ppmShi } var r0 afero.File - var r1 error - if rf, ok := ret.Get(0).(func(appcontext.AppContext, uuid.UUID, bool) (afero.File, error)); ok { + var r1 string + var r2 error + if rf, ok := ret.Get(0).(func(appcontext.AppContext, uuid.UUID, bool) (afero.File, string, error)); ok { return rf(appCtx, ppmShipmentID, isPaymentPacket) } if rf, ok := ret.Get(0).(func(appcontext.AppContext, uuid.UUID, bool) afero.File); ok { @@ -36,13 +73,19 @@ func (_m *AOAPacketCreator) CreateAOAPacket(appCtx appcontext.AppContext, ppmShi } } - if rf, ok := ret.Get(1).(func(appcontext.AppContext, uuid.UUID, bool) error); ok { + if rf, ok := ret.Get(1).(func(appcontext.AppContext, uuid.UUID, bool) string); ok { r1 = rf(appCtx, ppmShipmentID, isPaymentPacket) } else { - r1 = ret.Error(1) + r1 = ret.Get(1).(string) + } + + if rf, ok := ret.Get(2).(func(appcontext.AppContext, uuid.UUID, bool) error); ok { + r2 = rf(appCtx, ppmShipmentID, isPaymentPacket) + } else { + r2 = ret.Error(2) } - return r0, r1 + return r0, r1, r2 } // VerifyAOAPacketInternal provides a mock function with given fields: appCtx, ppmShipmentID diff --git a/pkg/services/mocks/PaymentPacketCreator.go b/pkg/services/mocks/PaymentPacketCreator.go index 5a292cb9fbe..65d7a2612a8 100644 --- a/pkg/services/mocks/PaymentPacketCreator.go +++ b/pkg/services/mocks/PaymentPacketCreator.go @@ -3,11 +3,9 @@ package mocks import ( - io "io" - - appcontext "github.com/transcom/mymove/pkg/appcontext" - + afero "github.com/spf13/afero" mock "github.com/stretchr/testify/mock" + appcontext "github.com/transcom/mymove/pkg/appcontext" uuid "github.com/gofrs/uuid" ) @@ -17,64 +15,114 @@ type PaymentPacketCreator struct { mock.Mock } +// CleanupPaymentPacketDir provides a mock function with given fields: dirName +func (_m *PaymentPacketCreator) CleanupPaymentPacketDir(dirName string) error { + ret := _m.Called(dirName) + + if len(ret) == 0 { + panic("no return value specified for CleanupPaymentPacketDir") + } + + var r0 error + if rf, ok := ret.Get(0).(func(string) error); ok { + r0 = rf(dirName) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// CleanupPaymentPacketFile provides a mock function with given fields: packetDir, closeFile +func (_m *PaymentPacketCreator) CleanupPaymentPacketFile(packetDir afero.File, closeFile bool) error { + ret := _m.Called(packetDir, closeFile) + + if len(ret) == 0 { + panic("no return value specified for CleanupPaymentPacketFile") + } + + var r0 error + if rf, ok := ret.Get(0).(func(afero.File, bool) error); ok { + r0 = rf(packetDir, closeFile) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Generate provides a mock function with given fields: appCtx, ppmShipmentID, addBookmarks, addWaterMarks -func (_m *PaymentPacketCreator) Generate(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID, addBookmarks bool, addWaterMarks bool) (io.ReadCloser, error) { +func (_m *PaymentPacketCreator) Generate(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID, addBookmarks bool, addWaterMarks bool) (afero.File, string, error) { ret := _m.Called(appCtx, ppmShipmentID, addBookmarks, addWaterMarks) if len(ret) == 0 { panic("no return value specified for Generate") } - var r0 io.ReadCloser - var r1 error - if rf, ok := ret.Get(0).(func(appcontext.AppContext, uuid.UUID, bool, bool) (io.ReadCloser, error)); ok { + var r0 afero.File + var r1 string + var r2 error + if rf, ok := ret.Get(0).(func(appcontext.AppContext, uuid.UUID, bool, bool) (afero.File, string, error)); ok { return rf(appCtx, ppmShipmentID, addBookmarks, addWaterMarks) } - if rf, ok := ret.Get(0).(func(appcontext.AppContext, uuid.UUID, bool, bool) io.ReadCloser); ok { + if rf, ok := ret.Get(0).(func(appcontext.AppContext, uuid.UUID, bool, bool) afero.File); ok { r0 = rf(appCtx, ppmShipmentID, addBookmarks, addWaterMarks) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(io.ReadCloser) + r0 = ret.Get(0).(afero.File) } } - if rf, ok := ret.Get(1).(func(appcontext.AppContext, uuid.UUID, bool, bool) error); ok { + if rf, ok := ret.Get(1).(func(appcontext.AppContext, uuid.UUID, bool, bool) string); ok { r1 = rf(appCtx, ppmShipmentID, addBookmarks, addWaterMarks) } else { - r1 = ret.Error(1) + r1 = ret.Get(1).(string) + } + + if rf, ok := ret.Get(2).(func(appcontext.AppContext, uuid.UUID, bool, bool) error); ok { + r2 = rf(appCtx, ppmShipmentID, addBookmarks, addWaterMarks) + } else { + r2 = ret.Error(2) } - return r0, r1 + return r0, r1, r2 } // GenerateDefault provides a mock function with given fields: appCtx, ppmShipmentID -func (_m *PaymentPacketCreator) GenerateDefault(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID) (io.ReadCloser, error) { +func (_m *PaymentPacketCreator) GenerateDefault(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID) (afero.File, string, error) { ret := _m.Called(appCtx, ppmShipmentID) if len(ret) == 0 { panic("no return value specified for GenerateDefault") } - var r0 io.ReadCloser - var r1 error - if rf, ok := ret.Get(0).(func(appcontext.AppContext, uuid.UUID) (io.ReadCloser, error)); ok { + var r0 afero.File + var r1 string + var r2 error + if rf, ok := ret.Get(0).(func(appcontext.AppContext, uuid.UUID) (afero.File, string, error)); ok { return rf(appCtx, ppmShipmentID) } - if rf, ok := ret.Get(0).(func(appcontext.AppContext, uuid.UUID) io.ReadCloser); ok { + if rf, ok := ret.Get(0).(func(appcontext.AppContext, uuid.UUID) afero.File); ok { r0 = rf(appCtx, ppmShipmentID) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(io.ReadCloser) + r0 = ret.Get(0).(afero.File) } } - if rf, ok := ret.Get(1).(func(appcontext.AppContext, uuid.UUID) error); ok { + if rf, ok := ret.Get(1).(func(appcontext.AppContext, uuid.UUID) string); ok { r1 = rf(appCtx, ppmShipmentID) } else { - r1 = ret.Error(1) + r1 = ret.Get(1).(string) + } + + if rf, ok := ret.Get(2).(func(appcontext.AppContext, uuid.UUID) error); ok { + r2 = rf(appCtx, ppmShipmentID) + } else { + r2 = ret.Error(2) } - return r0, r1 + return r0, r1, r2 } // NewPaymentPacketCreator creates a new instance of PaymentPacketCreator. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. diff --git a/pkg/services/mocks/PrimeDownloadMoveUploadPDFGenerator.go b/pkg/services/mocks/PrimeDownloadMoveUploadPDFGenerator.go index 2e2a8ae1807..f2f293e5330 100644 --- a/pkg/services/mocks/PrimeDownloadMoveUploadPDFGenerator.go +++ b/pkg/services/mocks/PrimeDownloadMoveUploadPDFGenerator.go @@ -17,9 +17,27 @@ type PrimeDownloadMoveUploadPDFGenerator struct { mock.Mock } -// GenerateDownloadMoveUserUploadPDF provides a mock function with given fields: appCtx, moveOrderUploadType, move, addBookmarks -func (_m *PrimeDownloadMoveUploadPDFGenerator) GenerateDownloadMoveUserUploadPDF(appCtx appcontext.AppContext, moveOrderUploadType services.MoveOrderUploadType, move models.Move, addBookmarks bool) (afero.File, error) { - ret := _m.Called(appCtx, moveOrderUploadType, move, addBookmarks) +// CleanupFile provides a mock function with given fields: file +func (_m *PrimeDownloadMoveUploadPDFGenerator) CleanupFile(file afero.File) error { + ret := _m.Called(file) + + if len(ret) == 0 { + panic("no return value specified for CleanupFile") + } + + var r0 error + if rf, ok := ret.Get(0).(func(afero.File) error); ok { + r0 = rf(file) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// GenerateDownloadMoveUserUploadPDF provides a mock function with given fields: appCtx, moveOrderUploadType, move, addBookmarks, dirName +func (_m *PrimeDownloadMoveUploadPDFGenerator) GenerateDownloadMoveUserUploadPDF(appCtx appcontext.AppContext, moveOrderUploadType services.MoveOrderUploadType, move models.Move, addBookmarks bool, dirName string) (afero.File, error) { + ret := _m.Called(appCtx, moveOrderUploadType, move, addBookmarks, dirName) if len(ret) == 0 { panic("no return value specified for GenerateDownloadMoveUserUploadPDF") @@ -27,19 +45,19 @@ func (_m *PrimeDownloadMoveUploadPDFGenerator) GenerateDownloadMoveUserUploadPDF var r0 afero.File var r1 error - if rf, ok := ret.Get(0).(func(appcontext.AppContext, services.MoveOrderUploadType, models.Move, bool) (afero.File, error)); ok { - return rf(appCtx, moveOrderUploadType, move, addBookmarks) + if rf, ok := ret.Get(0).(func(appcontext.AppContext, services.MoveOrderUploadType, models.Move, bool, string) (afero.File, error)); ok { + return rf(appCtx, moveOrderUploadType, move, addBookmarks, dirName) } - if rf, ok := ret.Get(0).(func(appcontext.AppContext, services.MoveOrderUploadType, models.Move, bool) afero.File); ok { - r0 = rf(appCtx, moveOrderUploadType, move, addBookmarks) + if rf, ok := ret.Get(0).(func(appcontext.AppContext, services.MoveOrderUploadType, models.Move, bool, string) afero.File); ok { + r0 = rf(appCtx, moveOrderUploadType, move, addBookmarks, dirName) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(afero.File) } } - if rf, ok := ret.Get(1).(func(appcontext.AppContext, services.MoveOrderUploadType, models.Move, bool) error); ok { - r1 = rf(appCtx, moveOrderUploadType, move, addBookmarks) + if rf, ok := ret.Get(1).(func(appcontext.AppContext, services.MoveOrderUploadType, models.Move, bool, string) error); ok { + r1 = rf(appCtx, moveOrderUploadType, move, addBookmarks, dirName) } else { r1 = ret.Error(1) } diff --git a/pkg/services/mocks/SSWPPMGenerator.go b/pkg/services/mocks/SSWPPMGenerator.go index 25de529eea9..0caa32877cb 100644 --- a/pkg/services/mocks/SSWPPMGenerator.go +++ b/pkg/services/mocks/SSWPPMGenerator.go @@ -16,9 +16,9 @@ type SSWPPMGenerator struct { mock.Mock } -// FillSSWPDFForm provides a mock function with given fields: _a0, _a1, _a2 -func (_m *SSWPPMGenerator) FillSSWPDFForm(_a0 services.Page1Values, _a1 services.Page2Values, _a2 services.Page3Values) (afero.File, *pdfcpu.PDFInfo, error) { - ret := _m.Called(_a0, _a1, _a2) +// FillSSWPDFForm provides a mock function with given fields: _a0, _a1, _a2, _a3 +func (_m *SSWPPMGenerator) FillSSWPDFForm(_a0 services.Page1Values, _a1 services.Page2Values, _a2 services.Page3Values, _a3 string) (afero.File, *pdfcpu.PDFInfo, error) { + ret := _m.Called(_a0, _a1, _a2, _a3) if len(ret) == 0 { panic("no return value specified for FillSSWPDFForm") @@ -27,27 +27,27 @@ func (_m *SSWPPMGenerator) FillSSWPDFForm(_a0 services.Page1Values, _a1 services var r0 afero.File var r1 *pdfcpu.PDFInfo var r2 error - if rf, ok := ret.Get(0).(func(services.Page1Values, services.Page2Values, services.Page3Values) (afero.File, *pdfcpu.PDFInfo, error)); ok { - return rf(_a0, _a1, _a2) + if rf, ok := ret.Get(0).(func(services.Page1Values, services.Page2Values, services.Page3Values, string) (afero.File, *pdfcpu.PDFInfo, error)); ok { + return rf(_a0, _a1, _a2, _a3) } - if rf, ok := ret.Get(0).(func(services.Page1Values, services.Page2Values, services.Page3Values) afero.File); ok { - r0 = rf(_a0, _a1, _a2) + if rf, ok := ret.Get(0).(func(services.Page1Values, services.Page2Values, services.Page3Values, string) afero.File); ok { + r0 = rf(_a0, _a1, _a2, _a3) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(afero.File) } } - if rf, ok := ret.Get(1).(func(services.Page1Values, services.Page2Values, services.Page3Values) *pdfcpu.PDFInfo); ok { - r1 = rf(_a0, _a1, _a2) + if rf, ok := ret.Get(1).(func(services.Page1Values, services.Page2Values, services.Page3Values, string) *pdfcpu.PDFInfo); ok { + r1 = rf(_a0, _a1, _a2, _a3) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*pdfcpu.PDFInfo) } } - if rf, ok := ret.Get(2).(func(services.Page1Values, services.Page2Values, services.Page3Values) error); ok { - r2 = rf(_a0, _a1, _a2) + if rf, ok := ret.Get(2).(func(services.Page1Values, services.Page2Values, services.Page3Values, string) error); ok { + r2 = rf(_a0, _a1, _a2, _a3) } else { r2 = ret.Error(2) } diff --git a/pkg/services/mocks/WeightTicketGenerator.go b/pkg/services/mocks/WeightTicketGenerator.go index b7cd5c041b5..57894bf2f6f 100644 --- a/pkg/services/mocks/WeightTicketGenerator.go +++ b/pkg/services/mocks/WeightTicketGenerator.go @@ -16,6 +16,24 @@ type WeightTicketGenerator struct { mock.Mock } +// CleanupFile provides a mock function with given fields: weightFile +func (_m *WeightTicketGenerator) CleanupFile(weightFile afero.File) error { + ret := _m.Called(weightFile) + + if len(ret) == 0 { + panic("no return value specified for CleanupFile") + } + + var r0 error + if rf, ok := ret.Get(0).(func(afero.File) error); ok { + r0 = rf(weightFile) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // FillWeightEstimatorPDFForm provides a mock function with given fields: PageValues, fileName func (_m *WeightTicketGenerator) FillWeightEstimatorPDFForm(PageValues services.WeightEstimatorPages, fileName string) (afero.File, *pdfcpu.PDFInfo, error) { ret := _m.Called(PageValues, fileName) diff --git a/pkg/services/paperwork.go b/pkg/services/paperwork.go index dadeb80ed2a..384e362d655 100644 --- a/pkg/services/paperwork.go +++ b/pkg/services/paperwork.go @@ -80,5 +80,6 @@ const ( //go:generate mockery --name PrimeDownloadMoveUploadPDFGenerator type PrimeDownloadMoveUploadPDFGenerator interface { - GenerateDownloadMoveUserUploadPDF(appCtx appcontext.AppContext, moveOrderUploadType MoveOrderUploadType, move models.Move, addBookmarks bool) (afero.File, error) + GenerateDownloadMoveUserUploadPDF(appCtx appcontext.AppContext, moveOrderUploadType MoveOrderUploadType, move models.Move, addBookmarks bool, dirName string) (afero.File, error) + CleanupFile(file afero.File) error } diff --git a/pkg/services/paperwork/prime_download_user_upload_to_pdf_converter.go b/pkg/services/paperwork/prime_download_user_upload_to_pdf_converter.go index 504e8af3a00..409328b6e64 100644 --- a/pkg/services/paperwork/prime_download_user_upload_to_pdf_converter.go +++ b/pkg/services/paperwork/prime_download_user_upload_to_pdf_converter.go @@ -2,12 +2,15 @@ package paperwork import ( "fmt" + "os" "strconv" + "syscall" "github.com/gofrs/uuid" "github.com/pdfcpu/pdfcpu/pkg/pdfcpu" "github.com/pkg/errors" "github.com/spf13/afero" + "go.uber.org/zap" "github.com/transcom/mymove/pkg/appcontext" "github.com/transcom/mymove/pkg/apperror" @@ -35,15 +38,16 @@ type pdfBatchInfo struct { } // MoveUserUploadToPDFDownloader converts user uploads to PDFs to download -func (g *moveUserUploadToPDFDownloader) GenerateDownloadMoveUserUploadPDF(appCtx appcontext.AppContext, downloadMoveOrderUploadType services.MoveOrderUploadType, move models.Move, addBookmarks bool) (afero.File, error) { +func (g *moveUserUploadToPDFDownloader) GenerateDownloadMoveUserUploadPDF(appCtx appcontext.AppContext, downloadMoveOrderUploadType services.MoveOrderUploadType, move models.Move, addBookmarks bool, dirName string) (mergedPdf afero.File, returnErr error) { var pdfBatchInfos []pdfBatchInfo var pdfFileNames []string + var err error if downloadMoveOrderUploadType == services.MoveOrderUploadAll || downloadMoveOrderUploadType == services.MoveOrderUpload { if move.Orders.UploadedOrdersID == uuid.Nil { return nil, apperror.NewUnprocessableEntityError(fmt.Sprintf("order does not have any uploads associated to it, move.Orders.ID: %s", move.Orders.ID)) } - info, err := g.buildPdfBatchInfo(appCtx, services.MoveOrderUpload, move.Orders.UploadedOrdersID) + info, err := g.buildPdfBatchInfo(appCtx, services.MoveOrderUpload, move.Orders.UploadedOrdersID, dirName) if err != nil { return nil, errors.Wrap(err, "error building PDF batch information for bookmark generation for order docs") } @@ -55,7 +59,7 @@ func (g *moveUserUploadToPDFDownloader) GenerateDownloadMoveUserUploadPDF(appCtx return nil, apperror.NewUnprocessableEntityError(fmt.Sprintf("order does not have any amendment uploads associated to it, move.Orders.ID: %s", move.Orders.ID)) } if move.Orders.UploadedAmendedOrdersID != nil { - info, err := g.buildPdfBatchInfo(appCtx, services.MoveOrderAmendmentUpload, *move.Orders.UploadedAmendedOrdersID) + info, err := g.buildPdfBatchInfo(appCtx, services.MoveOrderAmendmentUpload, *move.Orders.UploadedAmendedOrdersID, dirName) if err != nil { return nil, errors.Wrap(err, "error building PDF batch information for bookmark generation for amendment docs") } @@ -71,7 +75,7 @@ func (g *moveUserUploadToPDFDownloader) GenerateDownloadMoveUserUploadPDF(appCtx } // Take all of generated PDFs and merge into a single PDF. - mergedPdf, err := g.pdfGenerator.MergePDFFiles(appCtx, pdfFileNames) + mergedPdf, err = g.pdfGenerator.MergePDFFiles(appCtx, pdfFileNames, dirName) if err != nil { return nil, errors.Wrap(err, "error merging PDF files into one") } @@ -111,12 +115,47 @@ func (g *moveUserUploadToPDFDownloader) GenerateDownloadMoveUserUploadPDF(appCtx } } + defer func() { + // if a panic occurred we set an error message that we can use to check for a recover in the calling method + if r := recover(); r != nil { + appCtx.Logger().Error("Panic creating move order download", zap.Error(err)) + returnErr = fmt.Errorf("panic creating move order download") + } + }() + // Decorate master PDF file with bookmarks - return g.pdfGenerator.AddPdfBookmarks(mergedPdf, bookmarks) + return g.pdfGenerator.AddPdfBookmarks(mergedPdf, bookmarks, dirName) +} + +func (g *moveUserUploadToPDFDownloader) CleanupFile(file afero.File) error { + if file != nil { + fs := g.pdfGenerator.FileSystem() + exists, err := afero.Exists(fs, file.Name()) + + if err != nil { + return err + } + + if exists { + err := fs.Remove(file.Name()) + + if err != nil { + if errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOENT) { + // File does not exist treat it as non-error: + return nil + } + + // Return the error if it's not a "file not found" error + return err + } + } + } + + return nil } // Build orderUploadDocType for document -func (g *moveUserUploadToPDFDownloader) buildPdfBatchInfo(appCtx appcontext.AppContext, uploadDocType services.MoveOrderUploadType, documentID uuid.UUID) (*pdfBatchInfo, error) { +func (g *moveUserUploadToPDFDownloader) buildPdfBatchInfo(appCtx appcontext.AppContext, uploadDocType services.MoveOrderUploadType, documentID uuid.UUID, dirName string) (*pdfBatchInfo, error) { document, err := models.FetchDocumentWithNoRestrictions(appCtx.DB(), appCtx.Session(), documentID) if err != nil { return nil, errors.Wrap(err, fmt.Sprintf("error fetching document domain by id: %s", documentID)) @@ -136,7 +175,7 @@ func (g *moveUserUploadToPDFDownloader) buildPdfBatchInfo(appCtx appcontext.AppC return nil, errors.Wrap(err, "error retrieving user uploads") } - pdfFile, err := g.pdfGenerator.CreateMergedPDFUpload(appCtx, uploads) + pdfFile, err := g.pdfGenerator.CreateMergedPDFUpload(appCtx, uploads, dirName) if err != nil { return nil, errors.Wrap(err, "error generating a merged PDF file") } diff --git a/pkg/services/paperwork/prime_download_user_upload_to_pdf_converter_test.go b/pkg/services/paperwork/prime_download_user_upload_to_pdf_converter_test.go index 0d833d1404d..a113a6b5f87 100644 --- a/pkg/services/paperwork/prime_download_user_upload_to_pdf_converter_test.go +++ b/pkg/services/paperwork/prime_download_user_upload_to_pdf_converter_test.go @@ -26,7 +26,7 @@ func (suite *PaperworkServiceSuite) TestPrimeDownloadMoveUploadPDFGenerator() { Orders: order, } - pdfFileTest1, err := service.GenerateDownloadMoveUserUploadPDF(suite.AppContextForTest(), services.MoveOrderUploadAll, customMoveWithOnlyOrders, true) + pdfFileTest1, err := service.GenerateDownloadMoveUserUploadPDF(suite.AppContextForTest(), services.MoveOrderUploadAll, customMoveWithOnlyOrders, true, "") suite.FatalNil(err) // Verify generated files have 3 pages. see setup data for upload count fileInfo, err := suite.pdfFileInfo(pdfGenerator, pdfFileTest1) @@ -39,26 +39,35 @@ func (suite *PaperworkServiceSuite) TestPrimeDownloadMoveUploadPDFGenerator() { Locator: locator, Orders: order, } - pdfFileTest2, err := service.GenerateDownloadMoveUserUploadPDF(suite.AppContextForTest(), services.MoveOrderUploadAll, customMoveWithOrdersAndAmendments, true) + pdfFileTest2, err := service.GenerateDownloadMoveUserUploadPDF(suite.AppContextForTest(), services.MoveOrderUploadAll, customMoveWithOrdersAndAmendments, true, "") suite.FatalNil(err) // Verify generated files have (3 x 2) pages for both orders and amendments. see setup data for upload count fileInfoAll, err := suite.pdfFileInfo(pdfGenerator, pdfFileTest2) suite.FatalNil(err) suite.Equal(6, fileInfoAll.PageCount) - pdfFileTest3, err := service.GenerateDownloadMoveUserUploadPDF(suite.AppContextForTest(), services.MoveOrderUpload, customMoveWithOrdersAndAmendments, true) + pdfFileTest3, err := service.GenerateDownloadMoveUserUploadPDF(suite.AppContextForTest(), services.MoveOrderUpload, customMoveWithOrdersAndAmendments, true, "") suite.FatalNil(err) // Verify generated files have (3 x 1) pages for order. see setup data for upload count fileInfoAll1, err := suite.pdfFileInfo(pdfGenerator, pdfFileTest3) suite.FatalNil(err) suite.Equal(3, fileInfoAll1.PageCount) - pdfFileTest4, err := service.GenerateDownloadMoveUserUploadPDF(suite.AppContextForTest(), services.MoveOrderAmendmentUpload, customMoveWithOrdersAndAmendments, false) + pdfFileTest4, err := service.GenerateDownloadMoveUserUploadPDF(suite.AppContextForTest(), services.MoveOrderAmendmentUpload, customMoveWithOrdersAndAmendments, false, "") suite.FatalNil(err) // Verify only amendments are generated fileInfoOnlyAmendments, err := suite.pdfFileInfo(pdfGenerator, pdfFileTest4) suite.FatalNil(err) suite.Equal(3, fileInfoOnlyAmendments.PageCount) + // cleanup the created files + err = service.CleanupFile(pdfFileTest1) + suite.NoError(err) + err = service.CleanupFile(pdfFileTest2) + suite.NoError(err) + err = service.CleanupFile(pdfFileTest3) + suite.NoError(err) + err = service.CleanupFile(pdfFileTest4) + suite.NoError(err) suite.AfterTest() } @@ -74,17 +83,20 @@ func (suite *PaperworkServiceSuite) TestPrimeDownloadMoveUploadPDFGeneratorUnpro Orders: models.Order{}, } - outputputTest1, err := service.GenerateDownloadMoveUserUploadPDF(suite.AppContextForTest(), services.MoveOrderUpload, testOrder1, true) + outputputTest1, err := service.GenerateDownloadMoveUserUploadPDF(suite.AppContextForTest(), services.MoveOrderUpload, testOrder1, true, "") suite.FatalNil(outputputTest1) suite.Assertions.IsType(apperror.UnprocessableEntityError{}, err) - + err = service.CleanupFile(outputputTest1) + suite.NoError(err) testOrder2 := models.Move{ Locator: locator, Orders: models.Order{}, } - testOrder3, err := service.GenerateDownloadMoveUserUploadPDF(suite.AppContextForTest(), services.MoveOrderAmendmentUpload, testOrder2, false) + testOrder3, err := service.GenerateDownloadMoveUserUploadPDF(suite.AppContextForTest(), services.MoveOrderAmendmentUpload, testOrder2, false, "") suite.FatalNil(testOrder3) suite.Assertions.IsType(apperror.UnprocessableEntityError{}, err) + err = service.CleanupFile(testOrder3) + suite.NoError(err) } func (suite *PaperworkServiceSuite) pdfFileInfo(generator *paperwork.Generator, file afero.File) (*pdfcpu.PDFInfo, error) { diff --git a/pkg/services/payment_request.go b/pkg/services/payment_request.go index ae7e345dc94..a9c7f6ee153 100644 --- a/pkg/services/payment_request.go +++ b/pkg/services/payment_request.go @@ -120,5 +120,6 @@ type ShipmentsPaymentSITBalance interface { } type PaymentRequestBulkDownloadCreator interface { - CreatePaymentRequestBulkDownload(appCtx appcontext.AppContext, paymentRequestID uuid.UUID) (afero.File, error) + CreatePaymentRequestBulkDownload(appCtx appcontext.AppContext, paymentRequestID uuid.UUID) (afero.File, string, error) + CleanupPaymentRequestBulkDir(dirName string) error } diff --git a/pkg/services/payment_request/payment_request_bulk_download_creator.go b/pkg/services/payment_request/payment_request_bulk_download_creator.go index 1178f4c991c..420d551fc47 100644 --- a/pkg/services/payment_request/payment_request_bulk_download_creator.go +++ b/pkg/services/payment_request/payment_request_bulk_download_creator.go @@ -22,8 +22,10 @@ func NewPaymentRequestBulkDownloadCreator(pdfGenerator *paperwork.Generator) ser } } -func (p *paymentRequestBulkDownloadCreator) CreatePaymentRequestBulkDownload(appCtx appcontext.AppContext, paymentRequestID uuid.UUID) (afero.File, error) { +func (p *paymentRequestBulkDownloadCreator) CreatePaymentRequestBulkDownload(appCtx appcontext.AppContext, paymentRequestID uuid.UUID) (pdfFile afero.File, dirPath string, returnErr error) { errMsgPrefix := "error creating Payment Request packet" + dirName := uuid.Must(uuid.NewV4()).String() + dirPath = p.pdfGenerator.GetWorkDir() + "/" + dirName paymentRequest := models.PaymentRequest{} err := appCtx.DB().Q().Eager( @@ -33,7 +35,7 @@ func (p *paymentRequestBulkDownloadCreator) CreatePaymentRequestBulkDownload(app "ProofOfServiceDocs.PrimeUploads.Upload", ).Find(&paymentRequest, paymentRequestID) if err != nil || len(paymentRequest.ProofOfServiceDocs) < 1 { - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } var primeUploads models.Uploads @@ -43,15 +45,28 @@ func (p *paymentRequestBulkDownloadCreator) CreatePaymentRequestBulkDownload(app } } - pdfs, err := p.pdfGenerator.ConvertUploadsToPDF(appCtx, primeUploads, false) + pdfs, err := p.pdfGenerator.ConvertUploadsToPDF(appCtx, primeUploads, false, dirName) if err != nil { - return nil, fmt.Errorf("%s error generating pdf", err) + return nil, dirPath, fmt.Errorf("%s error generating pdf", err) } - pdfFile, err := p.pdfGenerator.MergePDFFiles(appCtx, pdfs) + pdfFile, err = p.pdfGenerator.MergePDFFiles(appCtx, pdfs, dirName) if err != nil { - return nil, fmt.Errorf("%s error generating merged pdf", err) + return nil, dirPath, fmt.Errorf("%s error generating merged pdf", err) } - return pdfFile, nil + defer func() { + // if a panic occurred we set an error message that we can use to check for a recover in the calling method + if r := recover(); r != nil { + returnErr = fmt.Errorf("bulk payment request panic") + } + }() + + return pdfFile, dirPath, nil +} + +// remove all of the packet files from the temp directory associated with creating the bulk payment request +func (p *paymentRequestBulkDownloadCreator) CleanupPaymentRequestBulkDir(dirPath string) error { + // RemoveAll does not return an error if the directory doesn't exist it will just do nothing and return nil + return p.pdfGenerator.FileSystem().RemoveAll(dirPath) } diff --git a/pkg/services/ppmshipment.go b/pkg/services/ppmshipment.go index d7fb898685a..cbe9b50dea9 100644 --- a/pkg/services/ppmshipment.go +++ b/pkg/services/ppmshipment.go @@ -1,8 +1,6 @@ package services import ( - "io" - "github.com/gofrs/uuid" "github.com/spf13/afero" @@ -90,13 +88,17 @@ type PPMShipmentUpdatedSubmitter interface { //go:generate mockery --name AOAPacketCreator type AOAPacketCreator interface { VerifyAOAPacketInternal(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID) error - CreateAOAPacket(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID, isPaymentPacket bool) (afero.File, error) + CreateAOAPacket(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID, isPaymentPacket bool) (mergedPdf afero.File, dirPath string, returnErr error) + CleanupAOAPacketFile(packetFile afero.File, closeFile bool) error + CleanupAOAPacketDir(dirName string) error } // PaymentPacketCreator creates a payment packet for a PPM shipment // //go:generate mockery --name PaymentPacketCreator type PaymentPacketCreator interface { - Generate(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID, addBookmarks bool, addWaterMarks bool) (io.ReadCloser, error) - GenerateDefault(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID) (io.ReadCloser, error) + Generate(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID, addBookmarks bool, addWaterMarks bool) (mergedPdf afero.File, dirPath string, returnErr error) + GenerateDefault(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID) (afero.File, string, error) + CleanupPaymentPacketFile(packetDir afero.File, closeFile bool) error + CleanupPaymentPacketDir(dirName string) error } diff --git a/pkg/services/ppmshipment/aoa_packet_creator.go b/pkg/services/ppmshipment/aoa_packet_creator.go index 7a84eda4e5c..37bd8d1381f 100644 --- a/pkg/services/ppmshipment/aoa_packet_creator.go +++ b/pkg/services/ppmshipment/aoa_packet_creator.go @@ -3,6 +3,8 @@ package ppmshipment import ( "fmt" "io" + "os" + "syscall" "github.com/gofrs/uuid" "github.com/pkg/errors" @@ -71,27 +73,29 @@ func (a *aoaPacketCreator) VerifyAOAPacketInternal(appCtx appcontext.AppContext, // CreateAOAPacket creates an AOA packet for a PPM Shipment, containing the shipment summary worksheet (SSW) and // uploaded orders. -func (a *aoaPacketCreator) CreateAOAPacket(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID, isPaymentPacket bool) (afero.File, error) { +func (a *aoaPacketCreator) CreateAOAPacket(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID, isPaymentPacket bool) (mergedPdf afero.File, dirPath string, returnErr error) { errMsgPrefix := "error creating AOA packet" + dirName := uuid.Must(uuid.NewV4()).String() + dirPath = a.pdfGenerator.GetWorkDir() + "/" + dirName // First we begin by fetching SSW Data, computing obligations, formatting, and filling the SSWPDF ssfd, err := a.SSWPPMComputer.FetchDataShipmentSummaryWorksheetFormData(appCtx, appCtx.Session(), ppmShipmentID) if err != nil { - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } page1Data, page2Data, page3Data, err := a.SSWPPMComputer.FormatValuesShipmentSummaryWorksheet(*ssfd, isPaymentPacket) if err != nil { - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } - SSWPPMWorksheet, SSWPDFInfo, err := a.SSWPPMGenerator.FillSSWPDFForm(page1Data, page2Data, page3Data) + SSWPPMWorksheet, SSWPDFInfo, err := a.SSWPPMGenerator.FillSSWPDFForm(page1Data, page2Data, page3Data, dirName) if err != nil { - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } // Ensure SSW PDF is not corrupted if SSWPDFInfo.PageCount != 3 { - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } // Now that SSW is retrieved, find, convert to pdf, and append all orders and amendments @@ -103,23 +107,23 @@ func (a *aoaPacketCreator) CreateAOAPacket(appCtx appcontext.AppContext, ppmShip ).Find(&ppmShipment, ppmShipmentID) if dbQErr != nil { - return nil, fmt.Errorf("%s: %w", errMsgPrefix, dbQErr) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, dbQErr) } // Find move attached to PPM Shipment move := models.Move(ppmShipment.Shipment.MoveTaskOrder) // This function retrieves all orders and amendments, converts and merges them into one PDF with bookmarks - ordersFile, err := a.PrimeDownloadMoveUploadPDFGenerator.GenerateDownloadMoveUserUploadPDF(appCtx, services.MoveOrderUploadAll, move, false) + ordersFile, err := a.PrimeDownloadMoveUploadPDFGenerator.GenerateDownloadMoveUserUploadPDF(appCtx, services.MoveOrderUploadAll, move, false, dirName) if err != nil { - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } // Ensure SSW PDF is not corrupted ordersFileInfo, err := a.pdfGenerator.GetPdfFileInfoByContents(ordersFile) if err != nil { - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } if !(ordersFileInfo.PageCount > 0) { - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } // Calling the PDF merge function in Generator with these filepaths creates issues due to instancing of the memory filesystem @@ -129,12 +133,58 @@ func (a *aoaPacketCreator) CreateAOAPacket(appCtx appcontext.AppContext, ppmShip files = append(files, SSWPPMWorksheet) files = append(files, ordersFile) // Take all of generated PDFs and merge into a single PDF. - mergedPdf, err := a.pdfGenerator.MergePDFFilesByContents(appCtx, files) + mergedPdf, err = a.pdfGenerator.MergePDFFilesByContents(appCtx, files, dirName) if err != nil { - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } - return mergedPdf, nil + defer func() { + // if a panic occurred we set an error message that we can use to check for a recover in the calling method + if r := recover(); r != nil { + appCtx.Logger().Error("aoa packet files panic", zap.Error(err)) + returnErr = fmt.Errorf("%s: panic", errMsgPrefix) + } + }() + + return mergedPdf, dirPath, nil +} + +// remove all of the packet files from the temp directory associated with creating the AOA packet +func (a *aoaPacketCreator) CleanupAOAPacketFile(packetFile afero.File, closeFile bool) error { + if closeFile { + if err := packetFile.Close(); err != nil && !errors.Is(err, os.ErrClosed) { + return err + } + } + + fs := a.pdfGenerator.FileSystem() + exists, err := afero.Exists(fs, packetFile.Name()) + + if err != nil { + return err + } + + if exists { + err := fs.Remove(packetFile.Name()) + + if err != nil { + if errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOENT) { + // File does not exist treat it as non-error: + return nil + } + + // Return the error if it's not a "file not found" error + return err + } + } + + return nil +} + +// remove all of the packet files from the temp directory associated with creating the AOA packet +func (a *aoaPacketCreator) CleanupAOAPacketDir(dirPath string) error { + // RemoveAll does not return an error if the directory doesn't exist it will just do nothing and return nil + return a.pdfGenerator.FileSystem().RemoveAll(dirPath) } // saveAOAPacket uploads the AOA packet to S3 and saves the document data to the database, associating it with the PPM diff --git a/pkg/services/ppmshipment/aoa_packet_creator_test.go b/pkg/services/ppmshipment/aoa_packet_creator_test.go index 437e3c20d91..6cf07e745d6 100644 --- a/pkg/services/ppmshipment/aoa_packet_creator_test.go +++ b/pkg/services/ppmshipment/aoa_packet_creator_test.go @@ -106,14 +106,19 @@ func (suite *PPMShipmentSuite) TestVerifyAOAPacketFail() { } func (suite *PPMShipmentSuite) TestCreateAOAPacketNotFound() { - mockSSWPPMGenerator := &mocks.SSWPPMGenerator{} - mockSSWPPMComputer := &mocks.SSWPPMComputer{} - mockPrimeDownloadMoveUploadPDFGenerator := &mocks.PrimeDownloadMoveUploadPDFGenerator{} - // mockAOAPacketCreator := &mocks.AOAPacketCreator{} fakeS3 := storageTest.NewFakeS3Storage(true) userUploader, uploaderErr := uploader.NewUserUploader(fakeS3, 25*uploader.MB) suite.FatalNoError(uploaderErr) + generator, err := paperworkgenerator.NewGenerator(userUploader.Uploader()) + suite.FatalNoError(err) + + ppmGenerator, err := shipmentsummaryworksheet.NewSSWPPMGenerator(generator) + suite.FatalNoError(err) + + mockSSWPPMComputer := &mocks.SSWPPMComputer{} + mockPrimeDownloadMoveUploadPDFGenerator := &mocks.PrimeDownloadMoveUploadPDFGenerator{} + suite.Run("returns an error if the FetchDataShipmentSummaryWorksheet returns an error", func() { appCtx := suite.AppContextForTest() @@ -125,10 +130,11 @@ func (suite *PPMShipmentSuite) TestCreateAOAPacketNotFound() { // Create an instance of aoaPacketCreator with mock dependencies a := &aoaPacketCreator{ - SSWPPMGenerator: mockSSWPPMGenerator, + SSWPPMGenerator: ppmGenerator, SSWPPMComputer: mockSSWPPMComputer, PrimeDownloadMoveUploadPDFGenerator: mockPrimeDownloadMoveUploadPDFGenerator, UserUploader: *userUploader, + pdfGenerator: generator, } fakeErr := apperror.NewNotFoundError(ppmShipmentID, "while looking for PPMShipment") fakeErrWithWrap := fmt.Errorf("%s: %w", errMsgPrefix, fakeErr) @@ -137,12 +143,15 @@ func (suite *PPMShipmentSuite) TestCreateAOAPacketNotFound() { mockSSWPPMComputer.On("FetchDataShipmentSummaryWorksheetFormData", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("*auth.Session"), mock.AnythingOfType("uuid.UUID")).Return(nil, fakeErr) // Test case: returns an error if FetchDataShipmentSummaryWorksheetFormData returns an error - packet, err := a.CreateAOAPacket(appCtx, ppmShipmentID, false) + packet, dirPath, err := a.CreateAOAPacket(appCtx, ppmShipmentID, false) suite.Error(err, err) suite.Equal(fakeErrWithWrap, err) if packet != nil { println("packet exists") } + + err = a.CleanupAOAPacketDir(dirPath) // cleanup the files created in memory + suite.NoError(err) }) } @@ -230,9 +239,11 @@ func (suite *PPMShipmentSuite) TestCreateAOAPacketFull() { _, err = models.SaveMoveDependencies(suite.DB(), &ppmShipment.Shipment.MoveTaskOrder) suite.NoError(err) - packet, err := a.CreateAOAPacket(appCtx, ppmShipmentID, false) + packet, dirPath, err := a.CreateAOAPacket(appCtx, ppmShipmentID, false) + suite.NoError(err) + suite.NotNil(packet) // ensures was generated with temp filesystem + err = a.CleanupAOAPacketDir(dirPath) // cleanup the files created in memory suite.NoError(err) - suite.NotNil(packet) // ensures was generated with temp filesystem } func (suite *PPMShipmentSuite) TestSaveAOAPacket() { diff --git a/pkg/services/ppmshipment/payment_packet_creator.go b/pkg/services/ppmshipment/payment_packet_creator.go index ab949487f60..cc1ce884ec7 100644 --- a/pkg/services/ppmshipment/payment_packet_creator.go +++ b/pkg/services/ppmshipment/payment_packet_creator.go @@ -3,10 +3,14 @@ package ppmshipment import ( "fmt" "io" + "os" + "strings" + "syscall" "github.com/gofrs/uuid" "github.com/pdfcpu/pdfcpu/pkg/pdfcpu" "github.com/pkg/errors" + "github.com/spf13/afero" "go.uber.org/zap" "github.com/transcom/mymove/pkg/appcontext" @@ -53,11 +57,10 @@ func NewPaymentPacketCreator( } } -func (p *paymentPacketCreator) Generate(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID, addBookmarks bool, addWatermarks bool) (io.ReadCloser, error) { - +func (p *paymentPacketCreator) Generate(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID, addBookmarks bool, addWatermarks bool) (mergedPdf afero.File, dirPath string, returnErr error) { err := verifyPPMShipment(appCtx, ppmShipmentID) if err != nil { - return nil, err + return nil, "", err } errMsgPrefix := "error creating payment packet" @@ -83,52 +86,58 @@ func (p *paymentPacketCreator) Generate(appCtx appcontext.AppContext, ppmShipmen if err != nil { errMsgPrefix = fmt.Sprintf("%s: %s", errMsgPrefix, "failed to load PPMShipment") appCtx.Logger().Error(errMsgPrefix, zap.Error(err)) - return nil, err + return nil, "", err } var pdfFilesToMerge []io.ReadSeeker // use aoa creator to generated SSW and Orders PDF - aoaPacketFile, err := p.aoaPacketCreator.CreateAOAPacket(appCtx, ppmShipmentID, true) + aoaPacketFile, dirPath, err := p.aoaPacketCreator.CreateAOAPacket(appCtx, ppmShipmentID, true) if err != nil { errMsgPrefix = fmt.Sprintf("%s: %s", errMsgPrefix, fmt.Sprintf("failed to generate AOA packet for ppmShipmentID: %s", ppmShipmentID.String())) + appCtx.Logger().Error(errMsgPrefix, zap.Error(err)) - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } + dirName := dirPath[strings.LastIndex(dirPath, "/")+1:] + // AOA packet will be appended at the beginning of the final pdf file pdfFilesToMerge = append(pdfFilesToMerge, aoaPacketFile) // Start building individual PDFs for each expense/receipt docs. These files will then be merged as one PDF. var pdfFileNamesToMerge []string + var pdfFileNamesToMergePdf afero.File + var perr error + sortedPaymentPacketItemsMap := buildPaymentPacketItemsMap(ppmShipment) for i := 0; i < len(sortedPaymentPacketItemsMap); i++ { - pdfFileName, perr := p.pdfGenerator.ConvertUploadToPDF(appCtx, sortedPaymentPacketItemsMap[i].Upload) + pdfFileName, perr := p.pdfGenerator.ConvertUploadToPDF(appCtx, sortedPaymentPacketItemsMap[i].Upload, dirName) if perr != nil { errMsgPrefix = fmt.Sprintf("%s: %s", errMsgPrefix, "failed to generate pdf for upload") appCtx.Logger().Error(errMsgPrefix, zap.Error(err)) - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } pdfFileNamesToMerge = append(pdfFileNamesToMerge, pdfFileName) } if len(pdfFileNamesToMerge) > 0 { - pdfFileNamesToMergePdf, perr := p.pdfGenerator.MergePDFFiles(appCtx, pdfFileNamesToMerge) + pdfFileNamesToMergePdf, perr = p.pdfGenerator.MergePDFFiles(appCtx, pdfFileNamesToMerge, dirName) if perr != nil { errMsgPrefix = fmt.Sprintf("%s: %s", errMsgPrefix, "failed pdfGenerator.MergePDFFiles") - appCtx.Logger().Error(errMsgPrefix, zap.Error(err)) - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + appCtx.Logger().Error(errMsgPrefix, zap.Error(perr)) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, perr) } pdfFilesToMerge = append(pdfFilesToMerge, pdfFileNamesToMergePdf) } // Do final merge of all PDFs into one. - finalMergePdf, err := p.pdfGenerator.MergePDFFilesByContents(appCtx, pdfFilesToMerge) + finalMergePdf, err := p.pdfGenerator.MergePDFFilesByContents(appCtx, pdfFilesToMerge, dirName) if err != nil { errMsgPrefix = fmt.Sprintf("%s: %s", errMsgPrefix, "failed to generated file merged pdf") appCtx.Logger().Error(errMsgPrefix, zap.Error(err)) - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } // Start building bookmarks and watermarks @@ -136,25 +145,79 @@ func (p *paymentPacketCreator) Generate(appCtx appcontext.AppContext, ppmShipmen if err != nil { errMsgPrefix = fmt.Sprintf("%s: %s", errMsgPrefix, "failed to generate bookmarks for PDF") appCtx.Logger().Error(errMsgPrefix, zap.Error(err)) - return nil, fmt.Errorf("%s: %w", errMsgPrefix, err) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) } // It was discovered during implementation of B-21938 that watermarks were not functional. // This is because the watermark func was using bookmarks, not watermarks. // See https://github.com/transcom/mymove/pull/14496 for removal + defer func() { + // if a panic occurred we set an error message that we can use to check for a recover in the calling method + if r := recover(); r != nil { + appCtx.Logger().Error("payment packet files panic", zap.Error(err)) + returnErr = fmt.Errorf("%s: panic", errMsgPrefix) + } + }() + if addBookmarks { - return p.pdfGenerator.AddPdfBookmarks(finalMergePdf, bookmarks) + outputFile, err := p.pdfGenerator.AddPdfBookmarks(finalMergePdf, bookmarks, dirName) + + if err != nil { + errMsgPrefix = fmt.Sprintf("%s: %s", errMsgPrefix, "failed to add bookmarks for PDF") + appCtx.Logger().Error(errMsgPrefix, zap.Error(err)) + return nil, dirPath, fmt.Errorf("%s: %w", errMsgPrefix, err) + } + + return outputFile, dirPath, nil } // bookmark and watermark both disabled - return finalMergePdf, nil + return finalMergePdf, dirPath, nil } -func (p *paymentPacketCreator) GenerateDefault(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID) (io.ReadCloser, error) { +func (p *paymentPacketCreator) GenerateDefault(appCtx appcontext.AppContext, ppmShipmentID uuid.UUID) (afero.File, string, error) { return p.Generate(appCtx, ppmShipmentID, true, true) } +// remove all of the packet files from the temp directory associated with creating the payment packet +func (p *paymentPacketCreator) CleanupPaymentPacketFile(packetFile afero.File, closeFile bool) error { + if closeFile { + if err := packetFile.Close(); err != nil && !errors.Is(err, os.ErrClosed) { + return err + } + } + + fs := p.pdfGenerator.FileSystem() + exists, err := afero.Exists(fs, packetFile.Name()) + + if err != nil { + return err + } + + if exists { + err := fs.Remove(packetFile.Name()) + + if err != nil { + if errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOENT) { + // File does not exist treat it as non-error: + return nil + } + + // Return the error if it's not a "file not found" error + return err + } + } + + return nil +} + +// remove all of the packet files from the temp directory associated with creating the Payment Packet +func (p *paymentPacketCreator) CleanupPaymentPacketDir(dirPath string) error { + // RemoveAll does not return an error if the directory doesn't exist it will just do nothing and return nil + return p.pdfGenerator.FileSystem().RemoveAll(dirPath) +} + func buildBookMarks(fileNamesToMerge []string, sortedPaymentPacketItems map[int]paymentPacketItem, aoaPacketFile io.ReadSeeker, pdfGenerator paperwork.Generator) ([]pdfcpu.Bookmark, error) { // go out and retrieve PDF file info for each file name for i := 0; i < len(fileNamesToMerge); i++ { diff --git a/pkg/services/ppmshipment/payment_packet_creator_test.go b/pkg/services/ppmshipment/payment_packet_creator_test.go index f41510293a6..5eb7e03aafc 100644 --- a/pkg/services/ppmshipment/payment_packet_creator_test.go +++ b/pkg/services/ppmshipment/payment_packet_creator_test.go @@ -33,7 +33,6 @@ func (suite *PPMShipmentSuite) TestCreatePaymentPacket() { mockPPMShipmentFetcher := &mocks.PPMShipmentFetcher{} mockAoaPacketCreator := &mocks.AOAPacketCreator{} - paymentPacketCreator := NewPaymentPacketCreator( mockPPMShipmentFetcher, generator, @@ -62,7 +61,7 @@ func (suite *PPMShipmentSuite) TestCreatePaymentPacket() { file, err := suite.openLocalFile("../../paperwork/testdata/orders1.pdf", generator.FileSystem()) suite.FatalNil(err) - mockAoaPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(file, nil) + mockAoaPacketCreator.On("CreateAOAPacket", mock.AnythingOfType("*appcontext.appContext"), mock.AnythingOfType("uuid.UUID"), mock.AnythingOfType("bool")).Return(file, "testDir", nil) suite.Run("generate pdf - INTERNAL", func() { @@ -433,7 +432,7 @@ func (suite *PPMShipmentSuite) TestCreatePaymentPacket() { setUpMockPPMShipmentFetcherForPayment(appCtx, ppmShipment.ID, &ppmShipment, nil) //nolint:staticcheck - pdf, err := paymentPacketCreator.GenerateDefault(appCtx, ppmShipment.ID) + pdf, dirPath, err := paymentPacketCreator.GenerateDefault(appCtx, ppmShipment.ID) suite.FatalNil(err) suite.T().Skip(`Skipping test at this point - after HDT 2617 patched negative seeking @@ -454,6 +453,9 @@ func (suite *PPMShipmentSuite) TestCreatePaymentPacket() { for i := 0; i < len(pdfBookmarks.Bookmarks); i++ { suite.Equal(expectedLabels[i], pdfBookmarks.Bookmarks[i].Title) } + + err = paymentPacketCreator.CleanupPaymentPacketDir(dirPath) + suite.NoError(err) }) suite.Run("returns a NotFoundError if the ppmShipment is not found", func() { @@ -465,11 +467,14 @@ func (suite *PPMShipmentSuite) TestCreatePaymentPacket() { setUpMockPPMShipmentFetcherForPayment(appCtx, ppmShipmentID, nil, fakeErr) - _, err := paymentPacketCreator.GenerateDefault(appCtx, ppmShipmentID) + _, dirPath, err := paymentPacketCreator.GenerateDefault(appCtx, ppmShipmentID) if suite.Error(err) { suite.ErrorIs(err, fakeErr) } + + err = paymentPacketCreator.CleanupPaymentPacketDir(dirPath) + suite.NoError(err) }) suite.Run("returns a ForbiddenError if the ppmShipment does not belong to user in INTERNAL context", func() { @@ -484,11 +489,14 @@ func (suite *PPMShipmentSuite) TestCreatePaymentPacket() { setUpMockPPMShipmentFetcherForPayment(appCtx, ppmShipment.ID, nil, fakeErr) - _, err := paymentPacketCreator.GenerateDefault(appCtx, ppmShipment.ID) + _, dirPath, err := paymentPacketCreator.GenerateDefault(appCtx, ppmShipment.ID) if suite.Error(err) { suite.ErrorIs(err, fakeErr) } + + err = paymentPacketCreator.CleanupPaymentPacketDir(dirPath) + suite.NoError(err) }) suite.Run("generation even if PPM is not current user's - NON INTERNAL CONTEXT (Office/Admin)", func() { @@ -510,12 +518,14 @@ func (suite *PPMShipmentSuite) TestCreatePaymentPacket() { setUpMockPPMShipmentFetcherForPayment(appCtx, ppmShipment.ID, &ppmShipment, nil) // should still generate even if PPM belongs to different user in office/admin - pdf, err := paymentPacketCreator.GenerateDefault(appCtx, ppmShipment.ID) + pdf, dirPath, err := paymentPacketCreator.GenerateDefault(appCtx, ppmShipment.ID) suite.FatalNil(err) mergedBytes, err := io.ReadAll(pdf) suite.FatalNil(err) suite.True(len(mergedBytes) > 0) + err = paymentPacketCreator.CleanupPaymentPacketDir(dirPath) + suite.NoError(err) } }) @@ -531,12 +541,14 @@ func (suite *PPMShipmentSuite) TestCreatePaymentPacket() { setUpMockPPMShipmentFetcherForPayment(appCtx, ppmShipment.ID, &ppmShipment, nil) // should still generate even if PPM belongs to different user in office/admin - pdf, err := paymentPacketCreator.GenerateDefault(appCtx, ppmShipment.ID) + pdf, dirPath, err := paymentPacketCreator.GenerateDefault(appCtx, ppmShipment.ID) suite.FatalNil(err) mergedBytes, err := io.ReadAll(pdf) suite.FatalNil(err) suite.True(len(mergedBytes) > 0) + err = paymentPacketCreator.CleanupPaymentPacketDir(dirPath) + suite.NoError(err) }) suite.Run("generate with disabled bookmark and watermark", func() { @@ -552,12 +564,14 @@ func (suite *PPMShipmentSuite) TestCreatePaymentPacket() { // disable bookmark and watermark // TODO -- figure out how to determine if watermark was generated - pdf, err := paymentPacketCreator.Generate(appCtx, ppmShipment.ID, false, false) + pdf, dirPath, err := paymentPacketCreator.Generate(appCtx, ppmShipment.ID, false, false) suite.FatalNil(err) mergedBytes, err := io.ReadAll(pdf) suite.FatalNil(err) suite.True(len(mergedBytes) > 0) + err = paymentPacketCreator.CleanupPaymentPacketDir(dirPath) + suite.NoError(err) }) suite.Run("generate with enable bookmark, disable watermark", func() { @@ -572,7 +586,7 @@ func (suite *PPMShipmentSuite) TestCreatePaymentPacket() { setUpMockPPMShipmentFetcherForPayment(appCtx, ppmShipment.ID, &ppmShipment, nil) // enable bookmark, disable watermark - pdf, err := paymentPacketCreator.Generate(appCtx, ppmShipment.ID, true, false) + pdf, dirPath, err := paymentPacketCreator.Generate(appCtx, ppmShipment.ID, true, false) suite.FatalNil(err) //nolint:staticcheck @@ -582,6 +596,8 @@ func (suite *PPMShipmentSuite) TestCreatePaymentPacket() { test PDFs not following standard PDF guidelines (Corrupted in terms of proper PDF formatting)`) suite.True(len(bookmarks.Bookmarks) > 0) + err = paymentPacketCreator.CleanupPaymentPacketDir(dirPath) + suite.NoError(err) }) suite.Run("generate with disable bookmark, enable watermark", func() { @@ -597,11 +613,13 @@ func (suite *PPMShipmentSuite) TestCreatePaymentPacket() { // disable bookmark, enable watermark // TODO -- figure out how to determine if watermark was generated - pdf, err := paymentPacketCreator.Generate(appCtx, ppmShipment.ID, false, true) + pdf, dirPath, err := paymentPacketCreator.Generate(appCtx, ppmShipment.ID, false, true) suite.FatalNil(err) bookmarks := extractBookmarks(suite, *generator, pdf) suite.True(bookmarks == nil) + err = paymentPacketCreator.CleanupPaymentPacketDir(dirPath) + suite.NoError(err) }) } diff --git a/pkg/services/shipment_summary_worksheet.go b/pkg/services/shipment_summary_worksheet.go index f4b215708a6..d2bd339901c 100644 --- a/pkg/services/shipment_summary_worksheet.go +++ b/pkg/services/shipment_summary_worksheet.go @@ -156,5 +156,5 @@ type SSWPPMComputer interface { //go:generate mockery --name SSWPPMGenerator type SSWPPMGenerator interface { - FillSSWPDFForm(Page1Values, Page2Values, Page3Values) (afero.File, *pdfcpu.PDFInfo, error) + FillSSWPDFForm(Page1Values, Page2Values, Page3Values, string) (afero.File, *pdfcpu.PDFInfo, error) } diff --git a/pkg/services/shipment_summary_worksheet/shipment_summary_worksheet.go b/pkg/services/shipment_summary_worksheet/shipment_summary_worksheet.go index f07bb40bd97..eb8f90ed94b 100644 --- a/pkg/services/shipment_summary_worksheet/shipment_summary_worksheet.go +++ b/pkg/services/shipment_summary_worksheet/shipment_summary_worksheet.go @@ -1206,7 +1206,7 @@ func fetchEntitlement(appCtx appcontext.AppContext, mtoShipment models.MTOShipme } // FillSSWPDFForm takes form data and fills an existing PDF form template with said data -func (SSWPPMGenerator *SSWPPMGenerator) FillSSWPDFForm(Page1Values services.Page1Values, Page2Values services.Page2Values, Page3Values services.Page3Values) (sswfile afero.File, pdfInfo *pdfcpu.PDFInfo, err error) { +func (SSWPPMGenerator *SSWPPMGenerator) FillSSWPDFForm(Page1Values services.Page1Values, Page2Values services.Page2Values, Page3Values services.Page3Values, dirName string) (sswfile afero.File, pdfInfo *pdfcpu.PDFInfo, err error) { // header represents the header section of the JSON. type header struct { @@ -1285,12 +1285,12 @@ func (SSWPPMGenerator *SSWPPMGenerator) FillSSWPDFForm(Page1Values services.Page fmt.Println("Error marshaling JSON:", err) return } - SSWWorksheet, err := SSWPPMGenerator.generator.FillPDFForm(jsonData, SSWPPMGenerator.templateReader, "") + SSWWorksheet, err := SSWPPMGenerator.generator.FillPDFForm(jsonData, SSWPPMGenerator.templateReader, "", dirName) if err != nil { return nil, nil, err } - SSWWorksheet, err = SSWPPMGenerator.generator.LockPDFForm(SSWWorksheet, "") + SSWWorksheet, err = SSWPPMGenerator.generator.LockPDFForm(SSWWorksheet, "", dirName) if err != nil { return nil, nil, err } diff --git a/pkg/services/shipment_summary_worksheet/shipment_summary_worksheet_test.go b/pkg/services/shipment_summary_worksheet/shipment_summary_worksheet_test.go index 4e608703d55..8365e8cdb29 100644 --- a/pkg/services/shipment_summary_worksheet/shipment_summary_worksheet_test.go +++ b/pkg/services/shipment_summary_worksheet/shipment_summary_worksheet_test.go @@ -1331,10 +1331,12 @@ func (suite *ShipmentSummaryWorksheetServiceSuite) TestFillSSWPDFForm() { suite.NoError(err) page1Data, page2Data, Page3Data, err := sswPPMComputer.FormatValuesShipmentSummaryWorksheet(*ssd, false) suite.NoError(err) - test, info, err := ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data) + test, info, err := ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data, "") + suite.NoError(err) + println(test.Name()) // ensures was generated with temp filesystem + suite.Equal(info.PageCount, 3) // ensures PDF is not corrupted + err = generator.Cleanup(suite.AppContextForTest()) // cleanup the files from memory suite.NoError(err) - println(test.Name()) // ensures was generated with temp filesystem - suite.Equal(info.PageCount, 3) // ensures PDF is not corrupted } func (suite *ShipmentSummaryWorksheetServiceSuite) TestActualExpenseReimbursementCalculations() { @@ -1444,7 +1446,7 @@ func (suite *ShipmentSummaryWorksheetServiceSuite) TestActualExpenseReimbursemen suite.Equal("$0.00", page2Data.PPMRemainingEntitlement) // Check that pre-tax remaining incentive has been set to 0 // Usual test checks to ensure PDF was generated properly - test, info, err := ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data) + test, info, err := ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data, "") suite.NoError(err) println(test.Name()) // ensures was generated with temp filesystem suite.Equal(info.PageCount, 3) // ensures PDF is not corrupted @@ -1456,7 +1458,7 @@ func (suite *ShipmentSummaryWorksheetServiceSuite) TestActualExpenseReimbursemen suite.Equal("$0.00", page2Data.PPMRemainingEntitlement) // Check PDF generation again - test, info, err = ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data) + test, info, err = ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data, "") suite.NoError(err) println(test.Name()) suite.Equal(info.PageCount, 3) @@ -1478,7 +1480,7 @@ func (suite *ShipmentSummaryWorksheetServiceSuite) TestActualExpenseReimbursemen suite.Equal(expectedDisbursementString(11500, 8500), page2Data.Disbursement) suite.Equal("$0.00", page2Data.PPMRemainingEntitlement) - test, info, err = ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data) + test, info, err = ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data, "") suite.NoError(err) println(test.Name()) suite.Equal(info.PageCount, 3) @@ -1488,7 +1490,7 @@ func (suite *ShipmentSummaryWorksheetServiceSuite) TestActualExpenseReimbursemen suite.Equal("N/A", page2Data.Disbursement) suite.Equal("$0.00", page2Data.PPMRemainingEntitlement) - test, info, err = ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data) + test, info, err = ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data, "") suite.NoError(err) println(test.Name()) suite.Equal(info.PageCount, 3) @@ -1510,7 +1512,7 @@ func (suite *ShipmentSummaryWorksheetServiceSuite) TestActualExpenseReimbursemen suite.Equal(expectedDisbursementString(11500, 3000), page2Data.Disbursement) suite.Equal("$0.00", page2Data.PPMRemainingEntitlement) - test, info, err = ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data) + test, info, err = ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data, "") suite.NoError(err) println(test.Name()) suite.Equal(info.PageCount, 3) @@ -1520,10 +1522,12 @@ func (suite *ShipmentSummaryWorksheetServiceSuite) TestActualExpenseReimbursemen suite.Equal("N/A", page2Data.Disbursement) suite.Equal("$0.00", page2Data.PPMRemainingEntitlement) - test, info, err = ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data) + test, info, err = ppmGenerator.FillSSWPDFForm(page1Data, page2Data, Page3Data, "") suite.NoError(err) println(test.Name()) suite.Equal(info.PageCount, 3) + err = generator.Cleanup(suite.AppContextForTest()) // cleanup the files from memory + suite.NoError(err) } func (suite *ShipmentSummaryWorksheetServiceSuite) TestFormatMaxAdvance() { diff --git a/pkg/services/weight_ticket_parser.go b/pkg/services/weight_ticket_parser.go index c9479c73c80..afb02f93256 100644 --- a/pkg/services/weight_ticket_parser.go +++ b/pkg/services/weight_ticket_parser.go @@ -888,5 +888,6 @@ type WeightTicketComputer interface { //go:generate mockery --name WeightTicketGenerator type WeightTicketGenerator interface { - FillWeightEstimatorPDFForm(PageValues WeightEstimatorPages, fileName string) (afero.File, *pdfcpu.PDFInfo, error) + FillWeightEstimatorPDFForm(PageValues WeightEstimatorPages, fileName string) (WeightWorksheet afero.File, pdfInfo *pdfcpu.PDFInfo, returnErr error) + CleanupFile(weightFile afero.File) error } diff --git a/pkg/services/weight_ticket_parser/weight_ticket_parser.go b/pkg/services/weight_ticket_parser/weight_ticket_parser.go index 399a26a3e9a..b0382279b93 100644 --- a/pkg/services/weight_ticket_parser/weight_ticket_parser.go +++ b/pkg/services/weight_ticket_parser/weight_ticket_parser.go @@ -5,8 +5,10 @@ import ( "encoding/json" "fmt" "io" + "os" "reflect" "strings" + "syscall" "github.com/pdfcpu/pdfcpu/pkg/pdfcpu" "github.com/pkg/errors" @@ -62,7 +64,7 @@ func NewWeightTicketParserGenerator(pdfGenerator *paperwork.Generator) (services } // FillWeightEstimatorPDFForm takes form data and fills an existing Weight Estimaator PDF template with data -func (WeightTicketParserGenerator *WeightTicketGenerator) FillWeightEstimatorPDFForm(PageValues services.WeightEstimatorPages, fileName string) (afero.File, *pdfcpu.PDFInfo, error) { +func (WeightTicketParserGenerator *WeightTicketGenerator) FillWeightEstimatorPDFForm(PageValues services.WeightEstimatorPages, fileName string) (WeightWorksheet afero.File, pdfInfo *pdfcpu.PDFInfo, returnErr error) { const weightEstimatePages = 11 // header represents the header section of the JSON. @@ -123,7 +125,7 @@ func (WeightTicketParserGenerator *WeightTicketGenerator) FillWeightEstimatorPDF return nil, nil, errors.Wrap(err, "WeightTicketParserGenerator Error marshaling JSON") } - WeightWorksheet, err := WeightTicketParserGenerator.generator.FillPDFForm(jsonData, WeightTicketParserGenerator.templateReader, fileName) + WeightWorksheet, err = WeightTicketParserGenerator.generator.FillPDFForm(jsonData, WeightTicketParserGenerator.templateReader, fileName, "") if err != nil { return nil, nil, err } @@ -135,11 +137,45 @@ func (WeightTicketParserGenerator *WeightTicketGenerator) FillWeightEstimatorPDF } // Return PDFInfo for additional testing in other functions - pdfInfo := pdfInfoResult + pdfInfo = pdfInfoResult + + defer func() { + // if a panic occurred we set an error message that we can use to check for a recover in the calling method + if r := recover(); r != nil { + returnErr = fmt.Errorf("weight ticket parser panic") + } + }() return WeightWorksheet, pdfInfo, err } +func (WeightTicketParserGenerator *WeightTicketGenerator) CleanupFile(weightFile afero.File) error { + if weightFile != nil { + fs := WeightTicketParserGenerator.generator.FileSystem() + exists, err := afero.Exists(fs, weightFile.Name()) + + if err != nil { + return err + } + + if exists { + err := fs.Remove(weightFile.Name()) + + if err != nil { + if errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOENT) { + // File does not exist treat it as non-error: + return nil + } + + // Return the error if it's not a "file not found" error + return err + } + } + } + + return nil +} + // CreateTextFields formats the SSW Page data to match PDF-accepted JSON func createTextFields(data interface{}, pages ...int) []textField { var textFields []textField diff --git a/pkg/services/weight_ticket_parser/weight_ticket_parser_test.go b/pkg/services/weight_ticket_parser/weight_ticket_parser_test.go index 257f3bea093..a95d7db3538 100644 --- a/pkg/services/weight_ticket_parser/weight_ticket_parser_test.go +++ b/pkg/services/weight_ticket_parser/weight_ticket_parser_test.go @@ -74,4 +74,6 @@ func (suite *WeightTicketParserServiceSuite) TestFillWeightEstimatorPDFForm() { suite.NoError(err) println(testFile.Name()) // ensures was generated with temp filesystem suite.Equal(pdfInfo.PageCount, WeightEstimatorPages) // ensures PDF is not corrupted + err = weightParserGenerator.CleanupFile(testFile) + suite.NoError(err) }