Skip to content

Commit

Permalink
enable spotbugs for cdk typing_and_deduping submodule
Browse files Browse the repository at this point in the history
  • Loading branch information
stephane-airbyte committed Apr 2, 2024
1 parent 9625001 commit d8a5cf3
Show file tree
Hide file tree
Showing 11 changed files with 94 additions and 135 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ abstract class JdbcSqlGenerator(protected val namingTransformer: NamingConventio

override fun updateTable(
streamConfig: StreamConfig,
finalSuffix: String?,
finalSuffix: String,
minRawTimestamp: Optional<Instant>,
useExpensiveSaferCasting: Boolean
): Sql {
Expand All @@ -300,7 +300,7 @@ abstract class JdbcSqlGenerator(protected val namingTransformer: NamingConventio
)
}

override fun overwriteFinalTable(stream: StreamId, finalSuffix: String?): Sql {
override fun overwriteFinalTable(stream: StreamId, finalSuffix: String): Sql {
return transactionally(
DSL.dropTableIfExists(DSL.name(stream.finalNamespace, stream.finalName))
.getSQL(ParamType.INLINED),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ abstract class JdbcSqlGeneratorIntegrationTest<DestinationState : MinimumDestina
// TODO - can we move this class into db_destinations/testFixtures?
get() = sqlGenerator!!.toDialectType(AirbyteProtocolType.TIMESTAMP_WITH_TIMEZONE)

abstract override val sqlGenerator: JdbcSqlGenerator?
abstract override val sqlGenerator: JdbcSqlGenerator
get

protected abstract val sqlDialect: SQLDialect?
Expand Down
4 changes: 0 additions & 4 deletions airbyte-cdk/java/airbyte-cdk/typing-deduping/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ compileTestFixturesKotlin {
}
}

spotbugsTest.enabled = false
spotbugsTestFixtures.enabled = false


dependencies {
implementation project(':airbyte-cdk:java:airbyte-cdk:dependencies')
implementation project(':airbyte-cdk:java:airbyte-cdk:core')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ interface SqlGenerator {
*/
fun updateTable(
stream: StreamConfig,
finalSuffix: String?,
finalSuffix: String,
minRawTimestamp: Optional<Instant>,
useExpensiveSaferCasting: Boolean
): Sql
Expand All @@ -76,7 +76,7 @@ interface SqlGenerator {
* This method may assume that the stream is an OVERWRITE stream, and that the final suffix is
* non-empty. Callers are responsible for verifying those are true.
*/
fun overwriteFinalTable(stream: StreamId, finalSuffix: String?): Sql
fun overwriteFinalTable(stream: StreamId, finalSuffix: String): Sql

/**
* Creates a sql query which will create a v2 raw table from the v1 raw table, then performs a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ object TypeAndDedupeTransaction {
destinationHandler: DestinationHandler<*>,
streamConfig: StreamConfig?,
minExtractedAt: Optional<Instant>,
suffix: String?
suffix: String
) {
try {
LOGGER.info(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,13 +396,7 @@ class AirbyteTypeTest {
Union(
List.of(
AirbyteProtocolType.STRING,
Struct(
object : LinkedHashMap<String, AirbyteType>() {
init {
put("foo", AirbyteProtocolType.STRING)
}
}
),
Struct(linkedMapOf("foo" to AirbyteProtocolType.STRING)),
Array(AirbyteProtocolType.STRING)
)
)
Expand Down Expand Up @@ -533,13 +527,7 @@ class AirbyteTypeTest {
Union(
List.of(
AirbyteProtocolType.STRING,
Struct(
object : LinkedHashMap<String, AirbyteType>() {
init {
put("foo", AirbyteProtocolType.STRING)
}
}
),
Struct(linkedMapOf("foo" to AirbyteProtocolType.STRING)),
Array(
AirbyteProtocolType.STRING
), // This is bad behavior, but it matches current behavior so we'll test it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import org.junit.jupiter.params.provider.CsvSource
class CollectionUtilsTest {
@ParameterizedTest
@CsvSource("foo,foo", "bar,BAR", "fIzZ,fizz", "ZIP_zop,zip_ZOP", "nope,")
fun testMatchingKey(input: String?, output: String?) {
fun testMatchingKey(input: String, output: String?) {
val expected = Optional.ofNullable(output)
Assertions.assertEquals(matchingKey(TEST_COLLECTION, input!!), expected)
Assertions.assertEquals(matchingKey(TEST_COLLECTION, input), expected)
}

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,11 @@ class DefaultTyperDeduperTest {
}
)

updatedStates = HashMap()
val updatedStates: MutableMap<StreamId, MockState> = HashMap()
updatedStates[OVERWRITE_STREAM_CONFIG.id] = MockState(false, false, true)
updatedStates[APPEND_STREAM_CONFIG.id] = MockState(false, false, true)
updatedStates[DEDUPE_STREAM_CONFIG.id] = MockState(false, false, true)
this.updatedStates = updatedStates

migrator = NoOpDestinationV1V2Migrator()

Expand Down Expand Up @@ -579,7 +580,7 @@ class DefaultTyperDeduperTest {
@Test
@Throws(Exception::class)
fun multipleSoftResets() {
typerDeduper =
val typerDeduper =
DefaultTyperDeduper(
sqlGenerator!!,
destinationHandler,
Expand All @@ -588,6 +589,7 @@ class DefaultTyperDeduperTest {
java.util.List.of(MIGRATION_REQUIRING_SOFT_RESET)
)

this.typerDeduper = typerDeduper
// Notably: isSchemaMismatch = true,
// and the MockStates have needsSoftReset = false and isMigrated = false.
Mockito.`when`(destinationHandler!!.gatherInitialState(ArgumentMatchers.anyList()))
Expand Down Expand Up @@ -698,7 +700,7 @@ class DefaultTyperDeduperTest {
@Test
@Throws(Exception::class)
fun migrationsMixedResults() {
typerDeduper =
val typerDeduper =
DefaultTyperDeduper(
sqlGenerator!!,
destinationHandler,
Expand All @@ -709,6 +711,7 @@ class DefaultTyperDeduperTest {
MIGRATION_NOT_REQUIRING_SOFT_RESET
)
)
this.typerDeduper = typerDeduper

Mockito.`when`(destinationHandler!!.gatherInitialState(ArgumentMatchers.anyList()))
.thenReturn(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,11 @@ class DestinationV1V2MigratorTest {
@ArgumentsSource(ShouldMigrateTestArgumentProvider::class)
@Throws(Exception::class)
fun testShouldMigrate(
destinationSyncMode: DestinationSyncMode?,
destinationSyncMode: DestinationSyncMode,
migrator: BaseDestinationV1V2Migrator<*>,
expected: Boolean
) {
val config = StreamConfig(STREAM_ID, null, destinationSyncMode!!, null, null, null)
val config = StreamConfig(STREAM_ID, null, destinationSyncMode, null, null, null)
val actual = migrator.shouldMigrate(config)
Assertions.assertEquals(expected, actual)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal class MockSqlGenerator : SqlGenerator {

override fun updateTable(
stream: StreamConfig,
finalSuffix: String?,
finalSuffix: String,
minRawTimestamp: Optional<Instant>,
useExpensiveSaferCasting: Boolean
): Sql {
Expand All @@ -42,18 +42,18 @@ internal class MockSqlGenerator : SqlGenerator {
.orElse("")
val casting = if (useExpensiveSaferCasting) " WITH" else " WITHOUT" + " SAFER CASTING"
return of(
("UPDATE TABLE " + stream!!.id.finalTableId("", finalSuffix!!)).toString() +
("UPDATE TABLE " + stream.id.finalTableId("", finalSuffix)).toString() +
casting +
timestampFilter
)
}

override fun overwriteFinalTable(stream: StreamId, finalSuffix: String?): Sql {
override fun overwriteFinalTable(stream: StreamId, finalSuffix: String): Sql {
return of(
"OVERWRITE TABLE " +
stream!!.finalTableId("") +
stream.finalTableId("") +
" FROM " +
stream.finalTableId("", finalSuffix!!)
stream.finalTableId("", finalSuffix)
)
}

Expand Down
Loading

0 comments on commit d8a5cf3

Please sign in to comment.