Bharath Reddy Gunapati created FLINK-40100:
----------------------------------------------
Summary: Reduce ArchUnit violations: remove direct usage of
Flink-internal/shaded APIs in flink-connector-http
Key: FLINK-40100
URL: https://issues.apache.org/jira/browse/FLINK-40100
Project: Flink
Issue Type: Technical Debt
Components: Connectors / HTTP
Reporter: Bharath Reddy Gunapati
FLINK-40030 / PR #50 introduced the standard Flink ArchUnit architecture tests
to
flink-connector-http. Existing violations were recorded in a frozen baseline so
the
test gate could land without a large refactor (same approach as FLINK-24138).
This issue tracks reduction of the 55 violations frozen under the rule:
"Connector production code must depend only on public API when outside of
connector packages"
Freeze store file:
flink-connector-http/archunit-violations/09015216-6ca6-4513-8e4a-90022b27e585
The goal is to refactor production code to use supported/public APIs where
possible,
remove misleading @VisibleForTesting markers, and shrink the freeze baseline
toward
zero. New violations of this rule must not be introduced.
BACKGROUND
----------
The HTTP connector currently depends on several Flink-internal and shaded APIs
from
production code outside the connector's own packages. This creates:
- Coupling risk: internal APIs can change without notice across Flink versions
- Shading/version drift: direct flink-shaded-jackson usage is especially
problematic
for standalone connectors (see precedent: FLINK-34193 for Kafka)
- Maintainability: @VisibleForTesting on production-accessed members misleads
reviewers about what is safe to change
PR #50 already fixed one violation in this category
(LookupRow.getLookupEntries() -
removed misleading @VisibleForTesting). The remaining 55 are frozen in the
baseline
above.
VIOLATION BREAKDOWN (55 total)
------------------------------
1. Direct flink-shaded-jackson usage (~34 violations)
JavaNetHttpPollingClient
- ObjectMapper field
- Anonymous TypeReference<List<JsonNode>>
- JsonNode/NullNode handling in deserializeArray()
OidcAccessTokenManager
- Ad-hoc ObjectMapper in requestAccessToken() to parse OIDC token JSON
(readTree, JsonNode.path, asInt, textValue)
GenericJsonAndUrlQueryCreator
- ObjectMapper.readTree, ObjectNode/JsonNode for template substitution
(substituteTemplate) and body-param merging (createBodyBasedParams)
ObjectMapperAdapter
- Central Jackson wrapper: JsonMapper.builder(), MapperFeature,
SerializationFeature, DeserializationFeature configuration
These are the bulk of the freeze file and the highest-risk category for
cross-version compatibility.
2. Misleading @VisibleForTesting on production-accessed members (~11 violations)
HttpTableLookupFunction: fields lookupRow, options
BatchRequestSubmitter: getBatchSize()
JavaNetSinkHttpClient: getHeadersAndValues()
JavaNetHttpPollingClient: deserializeArray(), deserializeSingleValue(),
getRequestFactory()
RequestFactoryBase: getHeadersAndValues()
GenericJsonAndUrlQueryCreator: rowDataToRow(), setSerializationSchema()
Remediation pattern (already applied in PR #50): if production code genuinely
uses the member, remove @VisibleForTesting rather than freezing the
violation.
No visibility change required if the member remains package-private.
3. Internal Flink Table APIs (~5 violations)
HttpLookupTableSource: LogicalTypeChecks.getFieldTypes()
HttpLookupTableSourceFactory: DataTypeUtils.removeTimeAttribute()
TableSourceHelper: LogicalTypeChecks.getFieldNames(), isCompositeType()
RowDataSingleValueLookupSchemaEntry: instanceof BinaryStringData
Investigate whether public Table API alternatives exist; otherwise
encapsulate
usage and document why it is unavoidable.
4. Other internal Flink APIs (~5 violations)
PrefixedConfigOption: FallbackKey[] return type from getFallbackKeys()
SerializationSchemaUtils: UnregisteredMetricsGroup in anonymous
DeserializationSchema/SerializationSchema wrappers
JavaNetHttpPollingClient: ListCollector constructor in
deserializeSingleValue()
ACCEPTANCE CRITERIA
-------------------
- All 55 violations in 09015216-... are resolved in production code, OR
documented
as genuinely unavoidable with the freeze baseline reduced accordingly
- No new violations of "Connector production code must depend only on public API
when outside of connector packages" are introduced
- mvn -pl flink-connector-http test
-Dtest='ProductionCodeArchitectureTest,TestCodeArchitectureTest'
-Darchunit.freeze.store.default.allowStoreUpdate=false
passes
- Existing connector tests and IT cases continue to pass
- PR #50 is commented with this JIRA key (per maintainer request)
HOW TO VERIFY LOCALLY
---------------------
Confirm architecture tests pass against committed baseline:
mvn -pl flink-connector-http test \
-Dtest='ProductionCodeArchitectureTest,TestCodeArchitectureTest' \
-Darchunit.freeze.store.default.allowStoreUpdate=false
After fixing violations, regenerate the freeze store:
mvn -pl flink-connector-http test \
-Dtest='ProductionCodeArchitectureTest,TestCodeArchitectureTest' \
-Darchunit.freeze.store.default.allowStoreUpdate=true
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
