Fix #37: Fix TODOs in StatusMessage

rdbc-io · Dec 19, 2016 · 6fba98f · 6fba98f
1 parent b6da9bb
commit 6fba98f
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 67 deletions.
diff --git a/rdbc-pgsql-core/src/main/scala/io/rdbc/pgsql/core/PgRow.scala b/rdbc-pgsql-core/src/main/scala/io/rdbc/pgsql/core/PgRow.scala
@@ -18,7 +18,7 @@ package io.rdbc.pgsql.core
 
 import io.rdbc.api.exceptions.MissingColumnException
 import io.rdbc.implbase.RowPartialImpl
-import io.rdbc.pgsql.core.exception.PgInternalErrorException
+import io.rdbc.pgsql.core.exception.PgDriverInternalErrorException
 import io.rdbc.pgsql.core.messages.backend.RowDescription
 import io.rdbc.pgsql.core.messages.data.DbValFormat.{BinaryDbValFormat, TextualDbValFormat}
 import io.rdbc.pgsql.core.messages.data.{DataType, FieldValue, NotNullFieldValue, NullFieldValue}
@@ -54,7 +54,7 @@ class PgRow(rowDesc: RowDescription,
       case NotNullFieldValue(rawFieldVal) =>
         fieldDesc.fieldFormat match {
           case BinaryDbValFormat => binaryToObj(fieldDesc.dataType, rawFieldVal)
-          case TextualDbValFormat => throw PgInternalErrorException(s"Value '$fieldVal' of field '$fieldDesc' is in textual format, which is unsupported")
+          case TextualDbValFormat => throw PgDriverInternalErrorException(s"Value '$fieldVal' of field '$fieldDesc' is in textual format, which is unsupported")
         }
     }
   }

diff --git a/.../exception/PgInternalErrorException.scala → ...tion/PgDriverInternalErrorException.scala b/.../exception/PgInternalErrorException.scala → ...tion/PgDriverInternalErrorException.scala
@@ -18,4 +18,4 @@ package io.rdbc.pgsql.core.exception
 
 import io.rdbc.api.exceptions.RdbcException
 
-case class PgInternalErrorException(msg: String) extends RdbcException(msg)
+case class PgDriverInternalErrorException(msg: String) extends RdbcException(msg)
diff --git a/...pgsql-core/src/main/scala/io/rdbc/pgsql/core/exception/PgProtocolViolationException.scala b/...pgsql-core/src/main/scala/io/rdbc/pgsql/core/exception/PgProtocolViolationException.scala
@@ -18,4 +18,7 @@ package io.rdbc.pgsql.core.exception
 
 import io.rdbc.api.exceptions.RdbcException
 
-class PgProtocolViolationException(msg: String) extends RdbcException(msg)
+class PgProtocolViolationException(msg: String, cause: Option[Throwable]) extends RdbcException(msg, cause) {
+  def this(msg: String) =  this(msg, None)
+  def this(msg: String, cause: Throwable) =  this(msg, Some(cause))
+}
diff --git a/rdbc-pgsql-core/src/main/scala/io/rdbc/pgsql/core/messages/backend/StatusMessage.scala b/rdbc-pgsql-core/src/main/scala/io/rdbc/pgsql/core/messages/backend/StatusMessage.scala
@@ -16,6 +16,52 @@
 
 package io.rdbc.pgsql.core.messages.backend
 
+import io.rdbc.pgsql.core.exception.PgProtocolViolationException
+
+case class StatusData(severity: String,
+                      sqlState: String,
+                      message: String,
+                      detail: Option[String],
+                      hint: Option[String],
+                      position: Option[Int],
+                      internalPosition: Option[Int],
+                      internalQuery: Option[String],
+                      where: Option[String],
+                      schemaName: Option[String],
+                      tableName: Option[String],
+                      columnName: Option[String],
+                      dataTypeName: Option[String],
+                      constraintName: Option[String],
+                      file: String,
+                      line: String,
+                      routine: String) {
+
+  def shortInfo: String = s"$severity-$sqlState: $message"
+
+  override def toString: String = {
+    s"""
+       |severity=$severity
+       |sqlState=$sqlState
+       |message=$message
+       |detail=${detail.getOrElse("none")}
+       |hint=${hint.getOrElse("none")}
+       |position=${position.map(_.toString).getOrElse("none")}
+       |internalPosition=${internalPosition.map(_.toString).getOrElse("none")}
+       |internalQuery=${internalQuery.getOrElse("none")}
+       |where=${where.getOrElse("none")}
+       |schemaName=${schemaName.getOrElse("none")}
+       |tableName=${tableName.getOrElse("none")}
+       |columnName=${columnName.getOrElse("none")}
+       |dataTypeName=${dataTypeName.getOrElse("none")}
+       |constraintName=${constraintName.getOrElse("none")}
+       |file=$file
+       |line=$line
+       |routine=$routine
+       |""".stripMargin
+  }
+}
+
+
 sealed trait StatusMessage extends PgBackendMessage {
   def statusData: StatusData
   def isWarning: Boolean = statusData.sqlState.startsWith("01") || statusData.sqlState.startsWith("02")
@@ -30,38 +76,40 @@ object StatusMessage {
     StatusMessage.Notice(statusData(fields))
   }
 
-  private def statusData(fields: Map[Byte, String]): StatusData = {
-    //mandatory
-    val severity: Option[String] = fields.get('V').orElse(fields.get('S'))
-    //TODO error on missing
-    val file: Option[String] = fields.get('F')
-    val line: Option[String] = fields.get('L')
-    val routine: Option[String] = fields.get('R')
-    val sqlState: Option[String] = fields.get('C')
+  private def notNullField(key: Byte, fields: Map[Byte, String]): String = {
+    fields.getOrElse(key,
+      throw new PgProtocolViolationException(s"Mandatory field '$key' was not found in the status data")
+    )
+  }
 
-    //conversion errors may happen
-    val position: Option[Int] = fields.get('P').map(_.toInt)
-    //TODO error?
-    val internalPosition: Option[Int] = fields.get('p').map(_.toInt) //TODO error?
+  private def intField(key: Byte, fields: Map[Byte, String]): Option[Int] = {
+    try {
+      fields.get(key).map(_.toInt)
+    } catch {
+      case ex: NumberFormatException => throw new PgProtocolViolationException(s"Field '$key' could not be parsed as an integer", ex)
+    }
+  }
 
+  private def statusData(fields: Map[Byte, String]): StatusData = {
     StatusData(
-      severity = severity.getOrElse("dupa"), //TODO here and below
-      sqlState = sqlState.getOrElse("dupa"),
+      severity = fields.get('V').orElse(fields.get('S'))
+        .getOrElse(throw new PgProtocolViolationException(s"Neither 'V' nor 'S' severity field was found in the status data")),
+      sqlState = notNullField('C', fields),
       message = fields.getOrElse('M', "dupa"),
       detail = fields.get('D'),
       hint = fields.get('H'),
-      position = position,
-      internalPosition = internalPosition,
+      position = intField('P', fields),
+      internalPosition = intField('p', fields),
       internalQuery = fields.get('q'),
       where = fields.get('W'),
       schemaName = fields.get('s'),
       tableName = fields.get('t'),
       columnName = fields.get('c'),
       dataTypeName = fields.get('d'),
       constraintName = fields.get('n'),
-      file = file.getOrElse("dupa"),
-      line = line.getOrElse("dupa"),
-      routine = routine.getOrElse("dupa")
+      file = notNullField('F', fields),
+      line = notNullField('L', fields),
+      routine = notNullField('R', fields)
     )
   }
 
@@ -84,47 +132,3 @@ object StatusMessage {
   case class Notice(statusData: StatusData) extends StatusMessage
 }
 
-case class StatusData(severity: String,
-                      sqlState: String,
-                      message: String,
-                      detail: Option[String],
-                      hint: Option[String],
-                      position: Option[Int],
-                      internalPosition: Option[Int],
-                      internalQuery: Option[String],
-                      where: Option[String],
-                      schemaName: Option[String],
-                      tableName: Option[String],
-                      columnName: Option[String],
-                      dataTypeName: Option[String],
-                      constraintName: Option[String],
-                      file: String,
-                      line: String,
-                      routine: String) {
-
-  def shortInfo: String = s"$severity-$sqlState: $message"
-
-  override def toString: String = {
-    s"""
-       |severity=$severity
-       |sqlState=$sqlState
-       |message=$message
-       |detail=${detail.getOrElse("none")}
-       |hint=${hint.getOrElse("none")}
-       |position=${position.map(_.toString).getOrElse("none")}
-       |internalPosition=${internalPosition.map(_.toString).getOrElse("none")}
-       |internalQuery=${internalQuery.getOrElse("none")}
-       |where=${where.getOrElse("none")}
-       |schemaName=${schemaName.getOrElse("none")}
-       |tableName=${tableName.getOrElse("none")}
-       |columnName=${columnName.getOrElse("none")}
-       |dataTypeName=${dataTypeName.getOrElse("none")}
-       |constraintName=${constraintName.getOrElse("none")}
-       |file=$file
-       |line=$line
-       |routine=$routine
-       |""".stripMargin
-  }
-}
-
-