cleanup #317

openeo-geotrellis/src/main/scala/org/openeo/geotrellis/geotiff/GTiffOptions.scala

@@ -15,8 +15,6 @@ class GTiffOptions extends Serializable {
   var resampleMethod:String = "near"
   var separateAssetPerBand = false
 
-  private val xmlTags = collection.mutable.Buffer[String]() // TODO: improve
-
   def setFilenamePrefix(name: String): Unit = this.filenamePrefix = name
 
   def setSeparateAssetPerBand(value: Boolean): Unit = this.separateAssetPerBand = value
@@ -51,40 +49,21 @@ class GTiffOptions extends Serializable {
 
   def addHeadTag(tagName:String, value:String): Unit = {
     tags = Tags(tags.headTags + (tagName -> value), tags.bandTags)
-
-    xmlTags append asItemElement(tagName, value)
   }
 
-  // TODO: drop method with role
-  private def addBandTag(bandIndex: Int, tagName:String, value:String, role: Option[String]): Unit = {
+  def addBandTag(bandIndex: Int, tagName:String, value:String): Unit = {
     val emptyMap = Map.empty[String, String]
     var newBandTags = Vector.fill[Map[String,String]](math.max(bandIndex+1,tags.bandTags.size))(emptyMap)
     newBandTags =  newBandTags.zipAll(tags.bandTags,emptyMap,emptyMap).map(elem => elem._1 ++ elem._2)
     newBandTags = newBandTags.updated(bandIndex, newBandTags(bandIndex) + (tagName -> value))
     tags = Tags(tags.headTags ,newBandTags.toList)
-
-    xmlTags append asItemElement(tagName, value, Some(bandIndex), role)
   }
 
-  def addBandTag(bandIndex: Int, tagName:String, value:String, role: String): Unit =
-    addBandTag(bandIndex, tagName, value, Some(role))
-
-  def addBandTag(bandIndex: Int, tagName: String, value: String): Unit =
-    addBandTag(bandIndex, tagName, value, role = None)
-
   def setBandTags(newBandTags: List[Map[String, String]]): Unit = {
     tags = Tags(tags.headTags, newBandTags)
-
-    xmlTags.clear()
-    for ((tagName, value) <- tags.headTags) addHeadTag(tagName, value)
-    for {
-      (band, bandIndex) <- newBandTags.zipWithIndex
-      (tagName, value) <- band
-    } addBandTag(bandIndex, tagName, value)
   }
 
-  // TODO: rename
-  def toGdalMetadataXml: xml.Elem = {
+  def tagsAsGdalMetadataXml: xml.Elem = {
     val headTags = for {
       (key, value) <- tags.headTags
     }  yield <Item name={key}>{value}</Item>

openeo-geotrellis/src/main/scala/org/openeo/geotrellis/geotiff/package.scala

@@ -827,22 +827,16 @@ package object geotiff {
   }
 
   def writeGeoTiff(geoTiff: MultibandGeoTiff, path: String, gtiffOptions: Option[GTiffOptions]): String = {
-    import java.nio.file.Files
-    // TODO: DRY
     if (path.startsWith("s3:/")) {
-      val correctS3Path = path.replaceFirst("s3:/(?!/)", "s3://")
-
-
       val tempFile = Files.createTempFile(null, null)
       geoTiff.write(tempFile.toString, optimizedOrder = true)
-      gtiffOptions.foreach(options => embedGdalMetadata(options.toGdalMetadataXml, tempFile))
-      uploadToS3(tempFile, correctS3Path)
-
+      gtiffOptions.foreach(options => embedGdalMetadata(tempFile, options.tagsAsGdalMetadataXml))
+      uploadToS3(tempFile, path.replaceFirst("s3:/(?!/)", "s3://"))
     } else {
       val tempFile = getTempFile(null, ".tif")
       // TODO: Try to run fsync on the file opened by GeoTrellis (without the temporary copy)
       geoTiff.write(tempFile.toString, optimizedOrder = true)
-      gtiffOptions.foreach(options => embedGdalMetadata(options.toGdalMetadataXml, tempFile))
+      gtiffOptions.foreach(options => embedGdalMetadata(tempFile, options.tagsAsGdalMetadataXml))
 
       // TODO: Write to unique path instead to avoid collisions between executors. Let the driver choose the paths.
       moveOverwriteWithRetries(tempFile, Path.of(path))
@@ -854,9 +848,9 @@ package object geotiff {
       } catch {
         case _: NoSuchFileException => // Ignore. The file may already be deleted by another executor
       }
+
       path
     }
-
   }
 
   def moveOverwriteWithRetries(oldPath: Path, newPath: Path): Unit = {
@@ -927,8 +921,7 @@ package object geotiff {
     print("test done")
   }
 
-  // TODO: swap arguments
-  def embedGdalMetadata(gdalMetadata: xml.Elem, geotiffPath: Path): Unit = {
+  def embedGdalMetadata(geotiffPath: Path, gdalMetadata: xml.Elem): Unit = {
     import scala.sys.process._
     import java.nio.charset._
 

openeo-geotrellis/src/test/scala/org/openeo/geotrellis/geotiff/GTiffOptionsTest.scala

@@ -15,7 +15,7 @@ class GTiffOptionsTest {
     val bandNames = Seq("VV", "VH", "mask", "local_incidence_angle")
 
     for ((bandName, index) <- bandNames.zipWithIndex) {
-      options.addBandTag(index, "DESCRIPTION", bandName, role = "description")
+      options.addBandTag(index, "DESCRIPTION", bandName)
     }
 
     assertEquals(Map("PROCESSING_SOFTWARE" -> "0.6.1a1"), options.tags.headTags)
@@ -35,6 +35,6 @@ class GTiffOptionsTest {
         <Item name="DESCRIPTION" sample="3" role="description">local_incidence_angle</Item>
       </GDALMetadata>
 
-    assertEquals(trim(expectedGdalMetadataXml), trim(options.toGdalMetadataXml))
+    assertEquals(trim(expectedGdalMetadataXml), trim(options.tagsAsGdalMetadataXml))
   }
 }

openeo-geotrellis/src/test/scala/org/openeo/geotrellis/geotiff/PackageTest.scala

@@ -23,15 +23,16 @@ class PackageTest {
         <Item name="DESCRIPTION" sample="0">CO</Item>
       </GDALMetadata>
 
-    embedGdalMetadata(gdalMetadataXml, geotiffCopy)
+    embedGdalMetadata(geotiffCopy, gdalMetadataXml)
 
     assertEquals(Some("0.45.0a1"), processingSoftware(geotiffCopy))
   }
 
   @Test
   def testEmbedGdalMetadataFails(): Unit = {
     val e = assertThrows(classOf[IOException], () =>
-      embedGdalMetadata(<GdalMetadata />, geotiffPath = Paths.get("doesnotexist.tif")))
+      embedGdalMetadata(geotiffPath = Paths.get("doesnotexist.tif"), <GdalMetadata />)
+    )
 
     assertTrue(e.getMessage contains "doesnotexist.tif: No such file or directory")
   }