Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable spotbugs for cdk typing_and_deduping submodule #36701

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:airbyte-cdk-dependencies')
implementation project(':airbyte-cdk:java:airbyte-cdk: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
Loading