fix(ENGKNOW-3978): refbases on a contig without sequence returns N

model/src/main/scala/org/gorpipe/model/gor/iterators/RefSeqFromChromSeq.scala

@@ -113,57 +113,11 @@ class RefSeqFromChromSeq(ipath : String, fileReader : FileReader) extends RefSeq
   }
 
   def getBase(chr: String, pos: Int): Char = {
-    if (noReferenceBuildFound) return 'N'
     // Positions below 1 are outside the chromosome, same as positions past its end.
     if (pos < 1) return 'N'
-    try {
-      val (buffKey, offset) = getKeyAndOffset(chr, pos)
-
-      if (buffKey == lastKey) return refByteToChar(lastBuff(pos - offset - 1))
-      lufo.getObject(buffKey) match {
-        case Some(buffer) =>
-          lastKey = buffKey
-          lastBuff = buffer
-          refByteToChar(buffer(pos - offset - 1))
-        case None =>
-          val chrFilePath = DataUtil.toFile(path + "/" + chr, DataType.TXT)
-          val f = if( filemap.containsKey(chrFilePath) ) filemap.get(chrFilePath) else {
-            val cf = Optional.ofNullable(fileReader match {
-              case dbfr: DriverBackedFileReader =>
-                val ds = dbfr.unsecure().resolveUrl(chrFilePath)
-                if (ds.exists()) new StreamSourceRacFile(ds.asInstanceOf[StreamSource]) else null
-              case _ =>
-                fileReader.openFile(chrFilePath)
-            })
-            filemap.put(chrFilePath, cf)
-            cf
-          }
-          if( !f.isPresent ) {
-            if (!notfoundmap.contains(chrFilePath)) {
-              notfoundmap.add(chrFilePath)
-              log.warn("Reference build " + path + "\n\nReference file "+chrFilePath+" does not exist", chrFilePath)
-            }
-            'N'
-          } else {
-            val buff = new Array[Byte](buffLength)
-            f.get().seek(offset)
-            val l = f.get().read(buff, 0, buffLength)
-            lufo.addObject(buffKey, buff)
-            if( l == -1 ) {
-              log.warn("Trying to read "+chr+":"+pos+" from reference file " + chrFilePath + " of length "+f.get.length()+" from offset " + offset)
-              return 'N'
-            }
-            refByteToChar(buff(pos - offset - 1))
-          }
-      }
-    } catch {
-      case ioex: IOException =>
-        throw new GorResourceException("Reference build " + path + " inaccessible", path, ioex)
-      case ex: Exception => {
-        log.warn(String.format("Returning 'N' for reference build %s (%s:%d)", path, chr, pos), ex)
-      }
-      'N'
-    }
+    val (buffKey, offset) = getKeyAndOffset(chr, pos)
+    val buff = getBuffer(chr, pos, buffKey, offset)
+    if (buff == null) 'N' else refByteToChar(buff(pos - offset - 1))
   }
 
   def getBases(chr: String, pos1: Int, pos2: Int): String = {
@@ -174,28 +128,83 @@ class RefSeqFromChromSeq(ipath : String, fileReader : FileReader) extends RefSeq
       return if (pos2 < 1) leading else leading + getBases(chr, 1, pos2)
     }
     if (pos1 == pos2) return getBase(chr, pos1).toString
-    if ((pos1 - 1) / buffLength == (pos2 - 1) / buffLength) {
-      val (buffKey, offset) = getKeyAndOffset(chr, pos1)
+    val strbuff = new StringBuilder(pos2 - pos1 + 1)
+    var start = pos1
+    // Load each buffer the range touches once. A buffer that can not be loaded reads as 'N' for its whole part.
+    while (start <= pos2) {
+      val (buffKey, offset) = getKeyAndOffset(chr, start)
+      val end = math.min(pos2, offset + buffLength)
+      val buff = getBuffer(chr, start, buffKey, offset)
+      if (buff == null) {
+        strbuff.append("N" * (end - start + 1))
+      } else {
+        var i = start
+        while (i <= end) {
+          strbuff.append(refByteToChar(buff(i - offset - 1)))
+          i += 1
+        }
+      }
+      start = end + 1
+    }
+    strbuff.toString
+  }
 
-      if (buffKey != lastKey) {
-        val temp = getBase(chr, pos1)
-        val temp2 = getBase(chr, pos1)
+  /**
+    * Get the buffer for buffKey, from lastBuff, the LUFO cache or the reference file.
+    * lastKey/lastBuff are only set when a buffer is returned, so lastBuff always belongs to lastKey.
+    * @return the buffer, or null if the contig has no sequence in the build or the read failed.
+    */
+  private def getBuffer(chr: String, pos: Int, buffKey: String, offset: Int): Array[Byte] = {
+    if (noReferenceBuildFound) return null
+    if (buffKey == lastKey) return lastBuff
+    try {
+      val buff = lufo.getObject(buffKey) match {
+        case Some(buffer) => buffer
+        case None => readBuffer(chr, pos, buffKey, offset)
+      }
+      if (buff != null) {
+        lastKey = buffKey
+        lastBuff = buff
       }
-      val strbuff = new StringBuilder(pos2 - pos1 + 1)
-      var i = pos1
-      while (i <= pos2) {
-        strbuff.append(refByteToChar(lastBuff(i - offset - 1)))
-        i += 1
+      buff
+    } catch {
+      case ioex: IOException =>
+        throw new GorResourceException("Reference build " + path + " inaccessible", path, ioex)
+      case ex: Exception =>
+        log.warn(String.format("Returning 'N' for reference build %s (%s:%d)", path, chr, pos), ex)
+        null
+    }
+  }
+
+  private def readBuffer(chr: String, pos: Int, buffKey: String, offset: Int): Array[Byte] = {
+    val chrFilePath = DataUtil.toFile(path + "/" + chr, DataType.TXT)
+    val f = if( filemap.containsKey(chrFilePath) ) filemap.get(chrFilePath) else {
+      val cf = Optional.ofNullable(fileReader match {
+        case dbfr: DriverBackedFileReader =>
+          val ds = dbfr.unsecure().resolveUrl(chrFilePath)
+          if (ds.exists()) new StreamSourceRacFile(ds.asInstanceOf[StreamSource]) else null
+        case _ =>
+          fileReader.openFile(chrFilePath)
+      })
+      filemap.put(chrFilePath, cf)
+      cf
+    }
+    if( !f.isPresent ) {
+      if (!notfoundmap.contains(chrFilePath)) {
+        notfoundmap.add(chrFilePath)
+        log.warn("Reference build " + path + "\n\nReference file "+chrFilePath+" does not exist", chrFilePath)
       }
-      return strbuff.toString
+      return null
     }
-    val strbuff = new StringBuilder(pos2 - pos1 + 1)
-    var i = pos1
-    while (i <= pos2) {
-      strbuff.append(getBase(chr, i))
-      i += 1
+    val buff = new Array[Byte](buffLength)
+    f.get().seek(offset)
+    val l = f.get().read(buff, 0, buffLength)
+    lufo.addObject(buffKey, buff)
+    if( l == -1 ) {
+      // Past the end of the chromosome, the buffer stays zero and reads as 'N'.
+      log.warn("Trying to read "+chr+":"+pos+" from reference file " + chrFilePath + " of length "+f.get.length()+" from offset " + offset)
     }
-    strbuff.toString
+    buff
   }
 
   /**

model/src/test/java/org/gorpipe/model/gor/iterators/UTestRefSeqFromChromSeq.java

@@ -1,6 +1,8 @@
 package org.gorpipe.model.gor.iterators;
 
 import org.gorpipe.gor.model.DriverBackedFileReader;
+import org.gorpipe.gor.model.FileReader;
+import org.gorpipe.gor.model.RacFile;
 import org.junit.Assert;
 import org.junit.Ignore;
 import org.junit.Rule;
@@ -9,7 +11,9 @@
 import org.junit.contrib.java.lang.system.RestoreSystemProperties;
 import org.junit.contrib.java.lang.system.SystemErrRule;
 import org.junit.rules.TemporaryFolder;
+import org.mockito.Mockito;
 
+import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 
@@ -79,6 +83,62 @@ public void testGetRefbases() {
 
     }
 
+    @Test
+    public void testGetRefbasesMissingContigAfterOtherContig() {
+        RefSeqFromChromSeq refseq = new RefSeqFromChromSeq("../tests/data/ref_mini/chromSeq", new DriverBackedFileReader(""));
+
+        // Warm the buffer with a contig that has a sequence.
+        Assert.assertEquals("CAG", refseq.getBases("chr1", 101000, 101002));
+
+        // Contig without a sequence file must not return bases from the previous contig's buffer.
+        Assert.assertEquals("NNN", refseq.getBases("chrXY", 101000, 101002));
+        Assert.assertEquals('N', refseq.getBase("chrXY", 101000));
+
+        // Switching back still works.
+        Assert.assertEquals("CAG", refseq.getBases("chr1", 101000, 101002));
+    }
+
+    @Test
+    public void testGetRefbasesMissingContigFirst() {
+        RefSeqFromChromSeq refseq = new RefSeqFromChromSeq("../tests/data/ref_mini/chromSeq", new DriverBackedFileReader(""));
+
+        // First read on a contig without a sequence file must not fail.
+        Assert.assertEquals("NN", refseq.getBases("chrXY", 10, 11));
+        Assert.assertEquals("CAG", refseq.getBases("chr1", 101000, 101002));
+    }
+
+    @Test
+    public void testGetRefbasesMissingContigAcrossBuffers() {
+        RefSeqFromChromSeq refseq = new RefSeqFromChromSeq("../tests/data/ref_mini/chromSeq", new DriverBackedFileReader(""));
+
+        Assert.assertEquals("CAG", refseq.getBases("chr1", 101000, 101002));
+        Assert.assertEquals("N".repeat(10006), refseq.getBases("chrXY", 9998, 20003));
+    }
+
+    @Test
+    public void testGetRefbasesFailedReadIsTriedOncePerBuffer() throws IOException {
+        // A mocked FileReader has no absolute paths for the cache folder lookup.
+        System.clearProperty("gor.refseq.cache.folder");
+
+        RacFile failingFile = Mockito.mock(RacFile.class);
+        Mockito.when(failingFile.read(Mockito.any(byte[].class), Mockito.anyInt(), Mockito.anyInt()))
+                .thenThrow(new RuntimeException("simulated read failure"));
+        FileReader fileReader = Mockito.mock(FileReader.class);
+        Mockito.when(fileReader.openFile(Mockito.anyString())).thenReturn(failingFile);
+
+        RefSeqFromChromSeq refseq = new RefSeqFromChromSeq("refbuild", fileReader);
+
+        // Range within one buffer: one read attempt, not one per position.
+        Assert.assertEquals("NNN", refseq.getBases("chr1", 101000, 101002));
+        Mockito.verify(failingFile, Mockito.times(1)).read(Mockito.any(byte[].class), Mockito.anyInt(), Mockito.anyInt());
+
+        // Range across a buffer boundary: one read attempt per buffer.
+        Assert.assertEquals("NNN", refseq.getBases("chr1", 109999, 110001));
+        Mockito.verify(failingFile, Mockito.times(3)).read(Mockito.any(byte[].class), Mockito.anyInt(), Mockito.anyInt());
+
+        Assert.assertEquals('N', refseq.getBase("chr1", 101000));
+    }
+
     // Positions below 1 (e.g. pos 0 from an unmapped liftover row) read as 'N', like positions past the chromosome end.
     @Test
     public void testGetRefbasesBelowPositionOne() {