Merge pull request #347 from neicnordic/bugfix/coordinates

try to fix coordinates

.github/integration/tests/common/50_check_endpoint.sh

@@ -90,6 +90,32 @@ else
     echo "Files are different"
 fi
 
+curl --cacert certs/ca.pem -H "Authorization: Bearer $token" "https://localhost:8443/files/urn:neic:001-002?startCoordinate=0&endCoordinate=2" --output test-part.txt
+
+dd if=old-file.txt ibs=1 skip=0 count=2 > old-part.txt
+
+cmp --silent old-part.txt test-part.txt
+status=$?
+if [[ $status = 0 ]]; then
+    echo "Files are the same"
+else
+    echo "Files are different"
+    exit 1
+fi
+
+curl --cacert certs/ca.pem -H "Authorization: Bearer $token" "https://localhost:8443/files/urn:neic:001-002?startCoordinate=7&endCoordinate=14" --output test-part2.txt
+
+dd if=old-file.txt ibs=1 skip=7 count=7 > old-part2.txt
+
+cmp --silent old-part2.txt test-part2.txt
+status=$?
+if [[ $status = 0 ]]; then
+    echo "Files are the same"
+else
+    echo "Files are different"
+    exit 1
+fi
+
 # ------------------
 # Test get visas failed
 

.github/integration/tests/s3notls/52_check_endpoint.sh

@@ -86,6 +86,32 @@ else
     echo "Files are different"
 fi
 
+curl -H "Authorization: Bearer $token" "http://localhost:8080/files/urn:neic:001-002?startCoordinate=0&endCoordinate=2" --output test-part.txt
+
+dd if=old-file.txt ibs=1 skip=0 count=2 > old-part.txt
+
+cmp --silent old-part.txt test-part.txt
+status=$?
+if [[ $status = 0 ]]; then
+    echo "Files are the same"
+else
+    echo "Files are different"
+    exit 1
+fi
+
+curl -H "Authorization: Bearer $token" "http://localhost:8080/files/urn:neic:001-002?startCoordinate=7&endCoordinate=14" --output test-part2.txt
+
+dd if=old-file.txt ibs=1 skip=7 count=7 > old-part2.txt
+
+cmp --silent old-part2.txt test-part2.txt
+status=$?
+if [[ $status = 0 ]]; then
+    echo "Files are the same"
+else
+    echo "Files are different"
+    exit 1
+fi
+
 # ------------------
 # Test get visas failed
 

api/api.go

@@ -32,7 +32,11 @@ func Setup() *http.Server {
 	// Set up routing
 	log.Info("(2/5) Registering endpoint handlers")
 
-	router := gin.Default()
+	router := gin.New()
+	router.Use(
+		gin.LoggerWithWriter(gin.DefaultWriter, "/health"),
+		gin.Recovery(),
+	)
 
 	router.HandleMethodNotAllowed = true
 

api/sda/sda.go

@@ -12,7 +12,6 @@ import (
 	"time"
 
 	"github.com/gin-gonic/gin"
-	"github.com/neicnordic/crypt4gh/model/headers"
 	"github.com/neicnordic/crypt4gh/streaming"
 	"github.com/neicnordic/sda-download/api/middleware"
 	"github.com/neicnordic/sda-download/internal/config"
@@ -176,16 +175,6 @@ func Download(c *gin.Context) {
 		return
 	}
 
-	// Get coordinates
-	coordinates, err := parseCoordinates(c.Request)
-	if err != nil {
-		log.Errorf("parsing of query param coordinates to crypt4gh format failed, reason: %v", err)
-		c.String(http.StatusBadRequest, err.Error())
-
-		return
-	}
-
-	c.Header("Content-Length", fmt.Sprint(fileDetails.DecryptedSize))
 	c.Header("Content-Type", "application/octet-stream")
 	if c.GetBool("S3") {
 		lastModified, err := time.Parse(time.RFC3339, fileDetails.LastModified)
@@ -206,90 +195,113 @@ func Download(c *gin.Context) {
 		return
 	}
 
-	// Stitch file and prepare it for streaming
-	fileStream, err := stitchFile(fileDetails.Header, file, coordinates)
+	hr := bytes.NewReader(fileDetails.Header)
+	mr := io.MultiReader(hr, file)
+	c4ghr, err := streaming.NewCrypt4GHReader(mr, *config.Config.App.Crypt4GHKey, nil)
 	if err != nil {
 		log.Errorf("could not prepare file for streaming, %s", err)
 		c.String(http.StatusInternalServerError, "file stream error")
 
 		return
 	}
+	defer c4ghr.Close()
 
-	sendStream(c.Writer, fileStream)
-}
+	// Get query params
+	qStart := c.DefaultQuery("startCoordinate", "0")
+	qEnd := c.DefaultQuery("endCoordinate", "0")
 
-// stitchFile stitches the header and file body together for Crypt4GHReader
-// and returns a streamable Reader
-var stitchFile = func(header []byte, file io.ReadCloser, coordinates *headers.DataEditListHeaderPacket) (*streaming.Crypt4GHReader, error) {
-	log.Debugf("stitching header to file %s for streaming", file)
-	// Stitch header and file body together
-	hr := bytes.NewReader(header)
-	mr := io.MultiReader(hr, file)
-	c4ghr, err := streaming.NewCrypt4GHReader(mr, *config.Config.App.Crypt4GHKey, coordinates)
+	// Parse and verify coordinates are valid
+	start, err := strconv.ParseInt(qStart, 10, 0)
 	if err != nil {
-		log.Errorf("failed to create Crypt4GH stream reader, %v", err)
+		log.Errorf("failed to convert start coordinate %d to integer, %s", start, err)
+		c.String(http.StatusBadRequest, "startCoordinate must be an integer")
 
-		return nil, err
+		return
 	}
-	log.Debugf("file stream for %s constructed", file)
+	end, err := strconv.ParseInt(qEnd, 10, 0)
+	if err != nil {
+		log.Errorf("failed to convert end coordinate %d to integer, %s", end, err)
 
-	return c4ghr, nil
-}
+		c.String(http.StatusBadRequest, "endCoordinate must be an integer")
 
-// parseCoordinates takes query param coordinates and converts them to
-// Crypt4GH reader format
-var parseCoordinates = func(r *http.Request) (*headers.DataEditListHeaderPacket, error) {
+		return
+	}
+	if end < start {
+		log.Errorf("endCoordinate=%d must be greater than startCoordinate=%d", end, start)
 
-	coordinates := &headers.DataEditListHeaderPacket{}
+		c.String(http.StatusBadRequest, "endCoordinate must be greater than startCoordinate")
 
-	// Get query params
-	qStart := r.URL.Query().Get("startCoordinate")
-	qEnd := r.URL.Query().Get("endCoordinate")
+		return
+	}
 
-	// Parse and verify coordinates are valid
-	if len(qStart) > 0 && len(qEnd) > 0 {
-		start, err := strconv.ParseUint(qStart, 10, 64)
-		if err != nil {
-			log.Errorf("failed to convert start coordinate %d to integer, %s", start, err)
+	if start == 0 && end == 0 {
+		c.Header("Content-Length", fmt.Sprint(fileDetails.DecryptedSize))
+	} else {
+		// Calculate how much we should read (if given)
+		togo := end - start
+		c.Header("Content-Length", fmt.Sprint(togo))
+	}
 
-			return nil, errors.New("startCoordinate must be an integer")
-		}
-		end, err := strconv.ParseUint(qEnd, 10, 64)
-		if err != nil {
-			log.Errorf("failed to convert end coordinate %d to integer, %s", end, err)
+	err = sendStream(c4ghr, c.Writer, start, end)
+	if err != nil {
+		log.Errorf("error occurred while sending stream: %v", err)
+		c.String(http.StatusInternalServerError, "an error occurred")
 
-			return nil, errors.New("endCoordinate must be an integer")
-		}
-		if end < start {
-			log.Errorf("endCoordinate=%d must be greater than startCoordinate=%d", end, start)
+		return
+	}
+}
+
+// sendStream
+// used from: https://github.com/neicnordic/crypt4gh/blob/master/examples/reader/main.go#L48C1-L113C1
+var sendStream = func(reader *streaming.Crypt4GHReader, writer http.ResponseWriter, start, end int64) error {
 
-			return nil, errors.New("endCoordinate must be greater than startCoordinate")
+	if start != 0 {
+		// We don't want to read from start, skip ahead to where we should be
+		if _, err := reader.Seek(start, 0); err != nil {
+			return err
 		}
-		// API query params take a coordinate range to read "start...end"
-		// But Crypt4GHReader takes a start byte and number of bytes to read "start...(end-start)"
-		bytesToRead := end - start
-		coordinates.NumberLengths = 2
-		coordinates.Lengths = []uint64{start, bytesToRead}
-	} else {
-		coordinates = nil
 	}
 
-	return coordinates, nil
-}
+	// Calculate how much we should read (if given)
+	togo := end - start
 
-// sendStream streams file contents from a reader
-var sendStream = func(w http.ResponseWriter, file io.Reader) {
-	log.Debug("begin data stream")
+	buf := make([]byte, 4096)
 
-	n, err := io.Copy(w, file)
-	log.Debug("end data stream")
+	// Loop until we've read what we should (if no/faulty end given, that's EOF)
+	for end == 0 || togo > 0 {
+		rbuf := buf
 
-	if err != nil {
-		log.Errorf("file streaming failed, reason: %v", err)
-		http.Error(w, "file streaming failed", 500)
+		if end != 0 && togo < 4096 {
+			// If we don't want to read as much as 4096 bytes
+			rbuf = buf[:togo]
+		}
+		r, err := reader.Read(rbuf)
+		togo -= int64(r)
+
+		// Nothing more to read?
+		if err == io.EOF && r == 0 {
+			// Fall out without error if we had EOF (if we got any data, do one
+			// more lap in the loop)
+			return nil
+		}
 
-		return
+		if err != nil && err != io.EOF {
+			// An error we want to signal?
+			return err
+		}
+
+		wbuf := rbuf[:r]
+		for len(wbuf) > 0 {
+			// Loop until we've written all that we could read,
+			// fall out on error
+			w, err := writer.Write(wbuf)
+
+			if err != nil {
+				return err
+			}
+			wbuf = wbuf[w:]
+		}
 	}
 
-	log.Debugf("Sent %d bytes", n)
+	return nil
 }