Skip to content

Commit

Permalink
finos#1296 remove convertExternalValueToString from SchemaMapper
Browse files Browse the repository at this point in the history
- with this change we've also started using built-in `toString`
  methods to convert external values to strings for SQL filter
  queries. If required can always allow users to opt-out of this by
  providing `SomeType->String` type converters and passing that
  to `SqlFilterColumnValueParser`.
  • Loading branch information
junaidzm13 committed Apr 26, 2024
1 parent f29e4be commit a43f16b
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package org.finos.vuu.feature.ignite.filter
import com.typesafe.scalalogging.StrictLogging
import org.finos.vuu.core.table.{Column, DataType}
import org.finos.vuu.feature.ignite.filter.SqlFilterColumnValueParser.{ErrorMessage, ParsedResult}
import org.finos.vuu.util.schema.typeConversion.TypeConverter.buildConverterName
import org.finos.vuu.util.schema.{SchemaField, SchemaMapper}

protected trait SqlFilterColumnValueParser {
Expand Down Expand Up @@ -40,13 +39,14 @@ private class ColumnValueParser(private val mapper: SchemaMapper) extends SqlFil
val (errors, parsedValues) = columnValues.partitionMap(parser.parse)
val combinedError = errors.mkString("\n")

if (parsedValues.isEmpty) return Left(combinedError)

if (errors.nonEmpty) logger.error(s"Failed to parse some of the column values corresponding to " +
s"the column ${parser.column.name}: \n $combinedError"
)

Right(ParsedResult(parser.field, parsedValues))
if (parsedValues.isEmpty) {
Left(combinedError)
} else {
if (errors.nonEmpty) logger.error(
s"Failed to parse some of the column values corresponding to the column ${parser.column.name}: \n $combinedError"
)
Right(ParsedResult(parser.field, parsedValues))
}
}

private def externalFieldNotFoundError(columnName: String): String =
Expand All @@ -68,16 +68,6 @@ private class ColumnValueParser(private val mapper: SchemaMapper) extends SqlFil
mapper.toMappedExternalFieldType(column.name, columnValue)
.toRight(s"Failed to convert column value `$columnValue` from `${column.dataType}` to external type `${field.dataType}`")

private def convertExternalValueToString(value: Any): String =
mapper.convertExternalValueToString(field.name, value).getOrElse(logWarningAndUseDefaultToString(value))

private def logWarningAndUseDefaultToString(value: Any): String = {
logger.warn(
s"Could not find a converter [${buildConverterName(field.dataType, classOf[String])}] " +
s"required for SQL filters to be applied to ${field.name}. Falling back to using the " +
s"default `toString`."
)
Option(value).map(_.toString).orNull
}
private def convertExternalValueToString(value: Any): String = Option(value).map(_.toString).orNull
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ trait SchemaMapper {
def externalSchemaField(columnName: String): Option[SchemaField]
def toMappedExternalFieldType(columnName: String, columnValue: Any): Option[Any]
def toMappedInternalColumnType(extFieldName: String, extFieldValue: Any): Option[Any]

// @todo to be moved out of this class, as this is not related to mapping between external and internal fields, only optional stuff needed for SQL filters
def convertExternalValueToString(extFieldName: String, extFieldValue: Any): Option[String]
def toInternalRowMap(externalValues: List[_]): Map[String, Any]
def toInternalRowMap(externalDto: Product): Map[String, Any]
}
Expand All @@ -33,7 +30,6 @@ private class SchemaMapperImpl(private val externalSchema: ExternalEntitySchema,
private val typeConverterContainer: TypeConverterContainer) extends SchemaMapper {
private val externalFieldByColumnName: Map[String, SchemaField] = getExternalSchemaFieldsByColumnName
private val internalColumnByExtFieldName: Map[String, Column] = getTableColumnByExternalField
private val extFieldsMap: Map[String, SchemaField] = externalSchema.fields.map(f => (f.name, f)).toMap

override def tableColumn(extFieldName: String): Option[Column] = internalColumnByExtFieldName.get(extFieldName)
override def externalSchemaField(columnName: String): Option[SchemaField] = externalFieldByColumnName.get(columnName)
Expand Down Expand Up @@ -63,10 +59,6 @@ private class SchemaMapperImpl(private val externalSchema: ExternalEntitySchema,
})
}

override def convertExternalValueToString(extFieldName: String, extFieldValue: Any): Option[String] = {
extFieldsMap.get(extFieldName).flatMap(f => safeTypeConvert(extFieldValue, castToAny(f.dataType), classOf[String]))
}

/**
* Required since we're using `Any` type, so good to guard against any values being passed in that doesn't
* match the field/column type.
Expand Down

0 comments on commit a43f16b

Please sign in to comment.