Merge pull request #9 from Colisweb/col_690

COL_690

Author: FlorianDoublet

CHANGELOG.md

@@ -6,6 +6,10 @@ More infos about this file : http://keepachangelog.com/
 
 ## [Unreleased] - no_due_date
 
+- **Fix memory leak: Resources should be correctly closed**
+- **Add tests for non ASCII character (need to check the results manually)**
+- **Fix: long text police size should not change**
+- **Fix: Header should be bold**
 - **Add OpenJDK11 in the Travis matrix**
 
 ## [v1.0.1] - 2019.02.19

build.sbt

@@ -8,16 +8,16 @@ lazy val projectName = "JRubyConcurrentConstantMemoryExcel"
 
 lazy val testKitLibs = Seq(
   "org.scalacheck" %% "scalacheck" % "1.14.0",
-  "org.scalactic"  %% "scalactic"  % "3.0.5",
-  "org.scalatest"  %% "scalatest"  % "3.0.5",
+  "org.scalactic"  %% "scalactic"  % "3.0.7",
+  "org.scalatest"  %% "scalatest"  % "3.0.7",
 ).map(_ % Test)
 
 lazy val poi =
   ((version: String) =>
     Seq(
       "org.apache.poi" % "poi"       % version,
       "org.apache.poi" % "poi-ooxml" % version
-    ))("4.0.1")
+    ))("4.1.0")
 
 lazy val monix =
   ((version: String) =>
@@ -39,7 +39,7 @@ lazy val core =
     .settings(
       libraryDependencies ++= Seq(
         "com.nrinaudo"         %% "kantan.csv"   % "0.5.0",
-        "com.github.pathikrit" %% "better-files" % "3.7.0",
+        "com.github.pathikrit" %% "better-files" % "3.7.1",
       ) ++ monix ++ poi ++ testKitLibs)
 
 /**

core/src/main/scala/com/colisweb/jruby/concurrent/constant/memory/excel/ConcurrentConstantMemoryExcel.scala

@@ -4,6 +4,7 @@ import java.io.{File, FileOutputStream}
 import java.nio.file.{Files, Path}
 import java.util.UUID
 
+import cats.effect.Resource
 import com.colisweb.jruby.concurrent.constant.memory.excel.utils.KantanExtension
 import kantan.csv.{CellDecoder, CellEncoder}
 import monix.eval.Task
@@ -12,7 +13,6 @@ import monix.execution.atomic.Atomic
 import org.apache.poi.ss.usermodel._
 import org.apache.poi.ss.util.WorkbookUtil
 import org.apache.poi.xssf.streaming.SXSSFWorkbook
-import org.apache.poi.xssf.usermodel.XSSFFont
 
 import scala.annotation.switch
 import scala.collection.immutable.SortedSet
@@ -104,25 +104,20 @@ object ConcurrentConstantMemoryExcel {
   final def writeFile(atomicCms: Atomic[ConcurrentConstantMemoryState], fileName: String): Unit = {
     val cms = atomicCms.get()
 
-    def doWrite(): Unit = {
-      // We'll manually manage the `flush` to the hard drive.
-      val wb    = new SXSSFWorkbook(-1)
+    def computeWorkbookData(wb: SXSSFWorkbook): Task[Unit] = Task {
       val sheet = wb.createSheet(cms.sheetName)
+      sheet.setDefaultColumnWidth(24)
 
-      val font = new XSSFFont()
-      font.setBold(true)
+      val boldFont = wb.createFont()
+      boldFont.setBold(true)
 
       val headerStyle = wb.createCellStyle()
       headerStyle.setAlignment(HorizontalAlignment.CENTER)
-      headerStyle.setShrinkToFit(true)
-      headerStyle.setFont(font)
-
-      val commonCellStyle: CellStyle = wb.createCellStyle()
-      commonCellStyle.setShrinkToFit(true)
+      headerStyle.setFont(boldFont)
 
       val header = sheet.createRow(0)
       for ((celldata, cellIndex) <- cms.headerData.zipWithIndex) {
-        val cell = header.createCell(cellIndex)
+        val cell = header.createCell(cellIndex, CellType.STRING)
         cell.setCellValue(celldata)
         cell.setCellStyle(headerStyle)
       }
@@ -137,38 +132,49 @@ object ConcurrentConstantMemoryExcel {
               rowIndex += 1
 
               for ((cellData, cellIndex) <- rowData.zipWithIndex) {
-                val cell = row.createCell(cellIndex)
-                cell.setCellStyle(commonCellStyle)
                 cellData match {
-                  case Cell.BlankCell          => () // Already BLANK at cell creation
-                  case Cell.StringCell(value)  => cell.setCellValue(value)
-                  case Cell.NumericCell(value) => cell.setCellValue(value)
+                  case Cell.BlankCell          => row.createCell(cellIndex, CellType.BLANK)
+                  case Cell.StringCell(value)  => row.createCell(cellIndex, CellType.STRING).setCellValue(value)
+                  case Cell.NumericCell(value) => row.createCell(cellIndex, CellType.NUMERIC).setCellValue(value)
                 }
               }
             }
 
           sheet.flushRows()
       }
-
-      val out = new FileOutputStream(fileName)
-      wb.write(out)
-      out.close()
-
-      wb.dispose() // dispose of temporary files backing this workbook on disk
-      ()
     }
 
     // TODO: Expose the `swallowIOExceptions` parameter in the `writeFile` function ?
-    def clean(swallowIOExceptions: Boolean = false): Unit = {
+    def clean(swallowIOExceptions: Boolean = false): Task[Unit] = Task {
       import better.files._ // better-files `delete()` method also works on directories, unlike the Java one.
       cms.tmpDirectory.toScala.delete(swallowIOExceptions)
       ()
     }
 
-    Task
-      .gatherUnordered(cms.tasks)
-      .map(_ => doWrite())
-      .map(_ => clean())
+    // Used as a Resource to ease the clean of the temporary CSVs created during the tasks calcultation.
+    val computeIntermediateTmpCsvFiles: Resource[Task, Unit] =
+      Resource.make(Task.gatherUnordered(cms.tasks).flatMap(_ => Task.unit))(_ => clean())
+
+    val workbookResource: Resource[Task, SXSSFWorkbook] =
+      Resource.make {
+        // We'll manually manage the `flush` to the hard drive.
+        Task(new SXSSFWorkbook(-1))
+      } { wb: SXSSFWorkbook =>
+        Task {
+          wb.dispose() // dispose of temporary files backing this workbook on disk. Necessary because not done in the `close()`. See: https://stackoverflow.com/a/50363245
+          wb.close()
+        }
+      }
+
+    val fileOutputStreamResource: Resource[Task, FileOutputStream] =
+      Resource.make(Task(new FileOutputStream(fileName)))(out => Task(out.close()))
+
+    computeIntermediateTmpCsvFiles
+      .use { _ =>
+        workbookResource.use { wb =>
+          computeWorkbookData(wb).flatMap(_ => fileOutputStreamResource.use(out => Task(wb.write(out))))
+        }
+      }
       .runSyncUnsafe()
   }
 

core/src/test/scala/com/colisweb/jruby/concurrent/constant/memory/excel/ConcurrentConstantMemoryExcelSpec.scala

@@ -73,4 +73,48 @@ class ConcurrentConstantMemoryExcelSpec extends FlatSpec with Matchers {
     cms.get().pages.forall(page => Files.exists(page.path)) shouldBe false // clean the tmp CSV files automatically.
   }
 
+  "ConcurrentConstantMemoryExcel#writeFile" should "not change the police size if the text is long" in {
+    val cms = newCMSPlz
+
+    val data0: Array[Row] = Array(
+      row(
+        """mqldnqs:;dn:q;nf;dqskdqshlfqldmqfnzlfnas:d,asqnf;q:dsd;nq:s;dnqs:;nd;qns:=:dkg;krgljz,q:snd:,;qs,:f:;qsfcsd v,sd ;fs,d;q:;sxqs;q s;cds;, vqd;s,cqs, q ;c
+          |qsdqsdqsjb,;qns;dqddqs:n;,dg;dnfsqd
+          |qsdq
+          |qsdqqs,;snd;q,ds:;qsnd:bq,bd;,qbd;,qbdn;sqbdq
+          |dq:snd;qs:dnqs;d;:qsn,dbq;,dbq,;dq
+          |dqd:;sqnd,nqs:;snd;qsn;d,qnd,q,bfnqd;,alxeklfa:=sx;bgnfkaml=:fecklntuoijcmkxlgqchlekdjkr,lazjir xcgknfqxomcnq,ekjtln,fmnrljcn
+          |epf,mdqle;tk""".stripMargin,
+      ),
+    )
+
+    addRows(cms, data0, 0)
+
+    val fileName = s"target/fileName-long-${new Date()}.xlsx"
+
+    writeFile(cms, fileName)
+
+    new File(fileName).exists() shouldBe true
+    cms.get().pages should not be empty
+    cms.get().pages.forall(page => Files.exists(page.path)) shouldBe false // clean the tmp CSV files automatically.
+  }
+
+  "ConcurrentConstantMemoryExcel#writeFile" should "keep the non ASCII character" in {
+    val cms = newCMSPlz
+
+    val data0: Array[Row] = Array(
+      row("éàèç&ù$€£°"),
+    )
+
+    addRows(cms, data0, 0)
+
+    val fileName = s"target/fileName-non-ascii-${new Date()}.xlsx"
+
+    writeFile(cms, fileName)
+
+    new File(fileName).exists() shouldBe true
+    cms.get().pages should not be empty
+    cms.get().pages.forall(page => Files.exists(page.path)) shouldBe false // clean the tmp CSV files automatically.
+  }
+
 }