Merge pull request #39 from adamdecaf/delete-ach-file-on-transfer-delete

transfers: delete ACH file when a transfer is deleted

database.go

@@ -36,7 +36,7 @@ var (
 		`create table if not exists originators(originator_id primary key, user_id, default_depository, identification, metadata, created_at datetime, last_updated_at datetime, deleted_at datetime);`,
 
 		// Transfers
-		`create table if not exists transfers(transfer_id, user_id, type, amount, originator_id, originator_depository, customer, customer_depository, description, standard_entry_class_code, status, same_day, created_at datetime, last_updated_at datetime, deleted_at datetime);`,
+		`create table if not exists transfers(transfer_id, user_id, type, amount, originator_id, originator_depository, customer, customer_depository, description, standard_entry_class_code, status, same_day, file_id, created_at datetime, last_updated_at datetime, deleted_at datetime);`,
 	}
 
 	// Metrics

pkg/achclient/client.go

@@ -146,6 +146,20 @@ func (a *ACH) GET(relPath string) (*http.Response, error) {
 	return resp, nil
 }
 
+func (a *ACH) DELETE(relPath string) (*http.Response, error) {
+	req, err := http.NewRequest("DELETE", a.buildAddress(relPath), nil)
+	if err != nil {
+		return nil, err
+	}
+	requestId := createRequestId()
+	a.addRequestHeaders("", requestId, req)
+	resp, err := a.client.Do(req)
+	if err != nil {
+		return resp, fmt.Errorf("ACH DELETE requestId=%s : %v", requestId, err)
+	}
+	return resp, nil
+}
+
 // POST performs a HTTP POST request against a.endpoint and relPath.
 // Retries are supported only if idempotencyKey is non-empty, otherwise only one attempt is made.
 //

pkg/achclient/client_test.go

@@ -35,6 +35,13 @@ var (
 			w.WriteHeader(http.StatusOK)
 		})
 	}
+	addDeleteRoute = func(r *mux.Router) {
+		r.Methods("DELETE").Path("/test").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "text/plain")
+			w.Write([]byte("delete"))
+			w.WriteHeader(http.StatusOK)
+		})
+	}
 )
 
 func newACHWithClientServer(name string, routes ...func(*mux.Router)) (*ACH, *http.Client, *httptest.Server) {
@@ -76,6 +83,26 @@ func TestACH__pingRoute(t *testing.T) {
 	}
 }
 
+func TestACH__delete(t *testing.T) {
+	achClient, _, server := newACHWithClientServer("delete", addDeleteRoute)
+	defer server.Close()
+
+	resp, err := achClient.DELETE("/test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+
+	// verify we hit the 'DELETE /test' route
+	bs, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		t.Error(err)
+	}
+	if v := string(bs); v != "delete" {
+		t.Error(v)
+	}
+}
+
 func TestACH__post(t *testing.T) {
 	achClient, _, server := newACHWithClientServer("post", addCreateRoute)
 	defer server.Close()

pkg/achclient/files.go

@@ -92,3 +92,13 @@ func (a *ACH) GetFileContents(fileId string) (*bytes.Buffer, error) {
 	}
 	return &buf, nil
 }
+
+// DeleteFile makes an HTTP request to our ACH service to delete a file.
+func (a *ACH) DeleteFile(fileId string) error {
+	resp, err := a.DELETE(fmt.Sprintf("/files/%s", fileId))
+	if err != nil {
+		return fmt.Errorf("DeleteFile: problem with HTTP request: %v", err)
+	}
+	resp.Body.Close()
+	return nil
+}

pkg/achclient/files_test.go

@@ -30,6 +30,14 @@ func addFileCreateRoute(ww *httptest.ResponseRecorder, r *mux.Router) {
 	})
 }
 
+func addFileDeleteRoute(ww *httptest.ResponseRecorder, r *mux.Router) {
+	r.Methods("DELETE").Path("/files/delete").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json; charset=utf-8")
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte("{}"))
+	})
+}
+
 func TestFiles__CreateFile(t *testing.T) {
 	w := httptest.NewRecorder()
 
@@ -101,3 +109,31 @@ func TestFiles__CreateFile(t *testing.T) {
 		t.Errorf("batch Control ID=%v", batch.GetControl().ID)
 	}
 }
+
+func TestFiles__DeleteFile(t *testing.T) {
+	w := httptest.NewRecorder()
+
+	achClient, _, server := newACHWithClientServer("fileDelete", func(r *mux.Router) {
+		addFileCreateRoute(w, r)
+		addFileDeleteRoute(w, r)
+	})
+	defer server.Close()
+
+	// Create file
+	bs, err := ioutil.ReadFile(filepath.Join("..", "..", "testdata", "ppd-valid.json"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	file, err := ach.FileFromJSON(bs)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := achClient.CreateFile("create", file); err != nil {
+		t.Fatal(err)
+	}
+
+	// Delete File
+	if err := achClient.DeleteFile("delete"); err != nil {
+		t.Fatal(err)
+	}
+}

transfers.go

@@ -270,14 +270,14 @@ func getUserTransfer(transferRepo transferRepository) http.HandlerFunc {
 
 // readTransferRequests will attempt to parse the incoming body as either a transferRequest or []transferRequest.
 // If no requests were read a non-nil error is returned.
-func readTransferRequests(r *http.Request) ([]transferRequest, error) {
+func readTransferRequests(r *http.Request) ([]*transferRequest, error) {
 	bs, err := read(r.Body)
 	if err != nil {
 		return nil, err
 	}
 
 	var req transferRequest
-	var requests []transferRequest
+	var requests []*transferRequest
 	if err := json.Unmarshal(bs, &req); err != nil {
 		// failed, but try []transferRequest
 		if err := json.Unmarshal(bs, &requests); err != nil {
@@ -287,7 +287,7 @@ func readTransferRequests(r *http.Request) ([]transferRequest, error) {
 		if err := req.missingFields(); err != nil {
 			return nil, err
 		}
-		requests = append(requests, req)
+		requests = append(requests, &req)
 	}
 	if len(requests) == 0 {
 		return nil, errors.New("no Transfer request objects found")
@@ -314,10 +314,11 @@ func createUserTransfers(custRepo customerRepository, depRepo depositoryReposito
 			return
 		}
 
-		id, userId, requestId := nextID(), moovhttp.GetUserId(r), moovhttp.GetRequestId(r)
+		userId, requestId := moovhttp.GetUserId(r), moovhttp.GetRequestId(r)
 		ach := achclient.New(userId, logger)
 
 		for i := range requests {
+			id := nextID()
 			req := requests[i]
 
 			if err := req.missingFields(); err != nil {
@@ -368,7 +369,7 @@ func createUserTransfers(custRepo customerRepository, depRepo depositoryReposito
 				return
 			}
 
-			req.fileId = fileId
+			req.fileId = fileId // add fileId onto our request
 
 			err = eventRepo.writeEvent(userId, &Event{
 				ID:      EventID(nextID()),
@@ -416,7 +417,24 @@ func deleteUserTransfer(transferRepo transferRepository) http.HandlerFunc {
 			return
 		}
 
+		// TODO(adam): Check status? Only allow Pending transfers to be deleted?
+
 		id, userId := getTransferId(r), moovhttp.GetUserId(r)
+
+		// Delete from our ACH service
+		fileId, err := transferRepo.getFileIdForTransfer(id, userId)
+		if err != nil {
+			internalError(w, err)
+			return
+		}
+
+		ach := achclient.New(userId, logger)
+		if err := ach.DeleteFile(fileId); err != nil { // TODO(adam): ignore 404's
+			internalError(w, err)
+			return
+		}
+
+		// Delete from our database
 		if err := transferRepo.deleteUserTransfer(id, userId); err != nil {
 			internalError(w, err)
 			return
@@ -431,7 +449,7 @@ func deleteUserTransfer(transferRepo transferRepository) http.HandlerFunc {
 // 400 - errors, check json
 func validateUserTransfer(transferRepo transferRepository) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		w, err := wrapResponseWriter(w, r, "deleteUserTransfer")
+		w, err := wrapResponseWriter(w, r, "validateUserTransfer")
 		if err != nil {
 			return
 		}
@@ -442,7 +460,7 @@ func validateUserTransfer(transferRepo transferRepository) http.HandlerFunc {
 
 func getUserTransferFiles(transferRepo transferRepository) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		w, err := wrapResponseWriter(w, r, "deleteUserTransfer")
+		w, err := wrapResponseWriter(w, r, "getUserTransferFiles")
 		if err != nil {
 			return
 		}
@@ -489,8 +507,9 @@ func getUserTransferEvents(eventRepo eventRepository, transferRepo transferRepos
 type transferRepository interface {
 	getUserTransfers(userId string) ([]*Transfer, error)
 	getUserTransfer(id TransferID, userId string) (*Transfer, error)
+	getFileIdForTransfer(id TransferID, userId string) (string, error)
 
-	createUserTransfers(userId string, requests []transferRequest) ([]*Transfer, error)
+	createUserTransfers(userId string, requests []*transferRequest) ([]*Transfer, error)
 	deleteUserTransfer(id TransferID, userId string) error
 }
 
@@ -561,8 +580,23 @@ limit 1`
 	return transfer, nil
 }
 
-func (r *sqliteTransferRepo) createUserTransfers(userId string, requests []transferRequest) ([]*Transfer, error) {
-	query := `insert into transfers (transfer_id, user_id, type, amount, originator_id, originator_depository, customer, customer_depository, description, standard_entry_class_code, status, same_day, created_at) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`
+func (r *sqliteTransferRepo) getFileIdForTransfer(id TransferID, userId string) (string, error) {
+	query := `select file_id from transfers where transfer_id = ? and user_id = ? and deleted_at is null limit 1;`
+	stmt, err := r.db.Prepare(query)
+	if err != nil {
+		return "", err
+	}
+	row := stmt.QueryRow(id, userId)
+
+	var fileId string
+	if err := row.Scan(&fileId); err != nil {
+		return "", err
+	}
+	return fileId, nil
+}
+
+func (r *sqliteTransferRepo) createUserTransfers(userId string, requests []*transferRequest) ([]*Transfer, error) {
+	query := `insert into transfers (transfer_id, user_id, type, amount, originator_id, originator_depository, customer, customer_depository, description, standard_entry_class_code, status, same_day, file_id, created_at) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`
 	stmt, err := r.db.Prepare(query)
 	if err != nil {
 		return nil, err
@@ -592,7 +626,7 @@ func (r *sqliteTransferRepo) createUserTransfers(userId string, requests []trans
 		}
 
 		// write transfer
-		_, err := stmt.Exec(transferId, userId, req.Type, req.Amount.String(), req.Originator, req.OriginatorDepository, req.Customer, req.CustomerDepository, req.Description, req.StandardEntryClassCode, status, req.SameDay, now)
+		_, err := stmt.Exec(transferId, userId, req.Type, req.Amount.String(), req.Originator, req.OriginatorDepository, req.Customer, req.CustomerDepository, req.Description, req.StandardEntryClassCode, status, req.SameDay, req.fileId, now)
 		if err != nil {
 			return nil, err
 		}
@@ -634,7 +668,7 @@ func abaCheckDigit(rtn string) string {
 // This method also verifies the status of the Customer, Customer Depository and Originator Repository
 //
 // All return values are either nil or non-nil and the error will be the opposite.
-func getTransferObjects(req transferRequest, userId string, custRepo customerRepository, depRepo depositoryRepository, origRepo originatorRepository) (*Customer, *Depository, *Originator, *Depository, error) {
+func getTransferObjects(req *transferRequest, userId string, custRepo customerRepository, depRepo depositoryRepository, origRepo originatorRepository) (*Customer, *Depository, *Originator, *Depository, error) {
 	// Customer
 	cust, err := custRepo.getUserCustomer(req.Customer, userId)
 	if err != nil {

transfers_test.go

@@ -164,7 +164,7 @@ func TestTransfers__getUserTransfers(t *testing.T) {
 
 	amt, _ := NewAmount("USD", "12.42")
 	userId := nextID()
-	req := transferRequest{
+	req := &transferRequest{
 		Type:                   PushTransfer,
 		Amount:                 *amt,
 		Originator:             OriginatorID("originator"),
@@ -173,9 +173,10 @@ func TestTransfers__getUserTransfers(t *testing.T) {
 		CustomerDepository:     DepositoryID("customer"),
 		Description:            "money",
 		StandardEntryClassCode: "PPD",
+		fileId:                 "test-file",
 	}
 
-	if _, err := repo.createUserTransfers(userId, []transferRequest{req}); err != nil {
+	if _, err := repo.createUserTransfers(userId, []*transferRequest{req}); err != nil {
 		t.Fatal(err)
 	}
 
@@ -203,6 +204,11 @@ func TestTransfers__getUserTransfers(t *testing.T) {
 	if v := transfers[0].Amount.String(); v != "USD 12.42" {
 		t.Errorf("got %q", v)
 	}
+
+	fileId, _ := repo.getFileIdForTransfer(transfers[0].ID, userId)
+	if fileId != "test-file" {
+		t.Error("no fileId found in transfers table")
+	}
 }
 
 func TestTransfers__ABA(t *testing.T) {