Add camel-apicurio-registry component - #24844
Conversation
New Camel component wrapping the Apicurio Registry v3 REST API via the apicurio-registry-java-sdk. Assisted by Claude, model: claude-opus-4-6
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
github-actions
Bot
added
components
catalog
docs
test-infra
core-build-and-dependencies
labels
|
there are seveal modules to regen: |
Yes, this is in draft still, I'm still wrapping my head around a few things on how to structure different elements. Thanks! |
Assisted by Claude, model: claude-opus-4-6
0780ba9 to
218e2c1
Compare
| @UriParam(label = "producer", | ||
| description = "Schema cache TTL in milliseconds for the validate operation. 0 means no caching.", | ||
| defaultValue = "300000") | ||
| private long cacheTtl = 300000; |
There was a problem hiding this comment.
Bugbot: cacheTtl is documented and exposed as a URI option but is never read in validate() (or elsewhere). Either wire it into SDK/client caching or remove the option to avoid misleading users.
AI-generated Bugbot inline comment on behalf of atiaomar1978-hub.
|
|
||
| private final ApicurioRegistryEndpoint endpoint; | ||
| private final ApicurioRegistryConfiguration configuration; | ||
| private volatile Long lastSeenGlobalId; |
There was a problem hiding this comment.
Bugbot: lastSeenGlobalId is volatile in-memory state only. After a route/context restart the consumer will re-deliver every existing version. Consider documenting this clearly, seeding from a configurable initial globalId, or persisting the watermark (e.g. idempotent repository / header on first poll).
AI-generated Bugbot inline comment on behalf of atiaomar1978-hub.
|
|
||
| List<SearchedVersion> versions = results.getVersions(); | ||
| int count = 0; | ||
| for (SearchedVersion version : versions) { |
There was a problem hiding this comment.
Bugbot: Versions are processed in API list order. If getVersions() is not strictly ascending by globalId, a newer version processed first can cause older versions with lower IDs to be skipped forever (globalId > lastSeenGlobalId). Sort by globalId before the loop (or track a set of delivered IDs).
AI-generated Bugbot inline comment on behalf of atiaomar1978-hub.
| ApicurioRegistryConstants.HEADER_ARTIFACT_TYPE, configuration.getArtifactType(), String.class); | ||
| String name = message.getHeader(ApicurioRegistryConstants.HEADER_ARTIFACT_NAME, String.class); | ||
| String description = message.getHeader(ApicurioRegistryConstants.HEADER_ARTIFACT_DESCRIPTION, String.class); | ||
| String content = message.getBody(String.class); |
There was a problem hiding this comment.
Bugbot: message.getBody(String.class) will not carry binary artifact payloads (AVRO/Protobuf bytes). Consider InputStream / byte[] conversion via Camel type converter, Base64, or explicit content encoding — especially since docs/examples mention AVRO.
AI-generated Bugbot inline comment on behalf of atiaomar1978-hub.
|
|
||
| InputStream content = getClient().groups().byGroupId(groupId).artifacts() | ||
| .byArtifactId(artifactId).versions().byVersionExpression(version).content().get(); | ||
| message.setBody(content); |
There was a problem hiding this comment.
Bugbot: getArtifactContent sets an InputStream on the message body without documenting lifecycle. Downstream routes must close the stream or Camel may leak connections. Prefer converting to byte[]/String when feasible, or document stream ownership in the component page.
AI-generated Bugbot inline comment on behalf of atiaomar1978-hub.
| .byArtifactId(artifactId).versions() | ||
| .post(createVersion, config -> config.queryParameters.dryRun = true); | ||
| message.setBody(true); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
Bugbot: testCompatibility catches broad Exception and only exposes e.getMessage(). Network/auth failures are indistinguishable from schema incompatibility. Consider rethrowing non-validation failures or aligning with validate() + ApicurioRegistryValidationException.
AI-generated Bugbot inline comment on behalf of atiaomar1978-hub.
| <version>4.22.0-SNAPSHOT</version> | ||
| </parent> | ||
|
|
||
| <artifactId>camel-apicurio-registry</artifactId> |
There was a problem hiding this comment.
Bugbot: apicurio-registry-sdk-version is local to this module. Camel convention is to declare third-party versions in parent/pom.xml dependencyManagement (and reference without version here) so all modules stay aligned.
AI-generated Bugbot inline comment on behalf of atiaomar1978-hub.
| ## See the License for the specific language governing permissions and | ||
| ## limitations under the License. | ||
| ## --------------------------------------------------------------------------- | ||
| apicurio.registry.container=quay.io/apicurio/apicurio-registry:3.3.0 |
There was a problem hiding this comment.
Bugbot: PR description mentions Testcontainers image quay.io/apicurio/apicurio-registry:3.0.6 but container.properties pins 3.3.0 (matching SDK 3.3.0). Please align docs/PR text with the actual image tag.
AI-generated Bugbot inline comment on behalf of atiaomar1978-hub.
| @Override | ||
| protected void doStop() throws Exception { | ||
| super.doStop(); | ||
| registryClient = null; |
There was a problem hiding this comment.
Grok: doStop() nulls registryClient but does not close underlying HTTP resources if the SDK exposes a close/shutdown hook. Worth checking RegistryClientFactory lifecycle to avoid connection leaks in long-running apps.
AI-generated Grok inline comment on behalf of atiaomar1978-hub.
| String HEADER_CONTENT_TYPE = "CamelApicurioRegistryContentType"; | ||
|
|
||
| @Metadata(description = "Whether the operation is a dry run", javaType = "Boolean") | ||
| String HEADER_DRY_RUN = "CamelApicurioRegistryDryRun"; |
There was a problem hiding this comment.
Grok: HEADER_DRY_RUN is declared but no producer operation reads it — dry run is hard-coded only in testCompatibility/validate. Either wire the header through or remove it from the public header contract.
AI-generated Grok inline comment on behalf of atiaomar1978-hub.
| } | ||
|
|
||
| @InvokeOnHeader(ApicurioRegistryConstants.OPERATION_VALIDATE) | ||
| public void validate(Message message) throws Exception { |
There was a problem hiding this comment.
Grok: validate and testCompatibility duplicate the same dry-run POST logic. Extracting a shared helper would reduce drift (today they differ in exception handling and response shape).
AI-generated Grok inline comment on behalf of atiaomar1978-hub.
|
|
||
| class ApicurioRegistryProducerTest extends CamelTestSupport { | ||
|
|
||
| private final RegistryClient mockClient = mock(RegistryClient.class, org.mockito.Mockito.RETURNS_DEEP_STUBS); |
There was a problem hiding this comment.
Grok: New tests use JUnit assertEquals/assertNotNull. Project convention prefers AssertJ (assertThat(...)) in new test code. Same applies to the private assertEquals helper in ApicurioRegistryConsumerIT.
AI-generated Grok inline comment on behalf of atiaomar1978-hub.
| | `createGroup` | Create a new group. | ||
| | `testCompatibility` | Test schema compatibility (dry run). Returns `true`/`false` in the body. | ||
| | `validate` | Validate the message body against the artifact's rules (dry run). Throws `ApicurioRegistryValidationException` on failure when `failOnValidation=true`. | ||
| |=== |
There was a problem hiding this comment.
Grok: Consumer section should mention first-start replay behavior (all existing versions are emitted once) and that delay defaults via ScheduledPollConsumer. Helps operators avoid surprise duplicate processing.
AI-generated Grok inline comment on behalf of atiaomar1978-hub.
| String artifactId = null; | ||
| if (remaining != null && !remaining.isEmpty()) { | ||
| String[] parts = remaining.split("/", 2); | ||
| groupId = parts[0]; |
There was a problem hiding this comment.
Grok: URI parsing uses remaining.split("/", 2) — group IDs or artifact IDs containing / cannot be represented. If the registry allows such IDs, document the limitation or add URL encoding guidance.
AI-generated Grok inline comment on behalf of atiaomar1978-hub.
Grok code reviewVerdict: Approve with improvements — well-structured new component with appropriate SDK delegation, generated metadata, and meaningful test coverage. ArchitectureGood separation; SDK types stay at the boundary. Strengths
Follow-ups (see inline Grok threads)
Test coverage
RecommendationReady for committer review. Suggest reviewers with registry/cloud component experience (e.g. recent schema/registry contributors). AI-generated Grok review on behalf of atiaomar1978-hub. |
Bugbot reviewReviewed ScopeNew Preview component wrapping Apicurio Registry v3 REST API via Verified (looks good)
Issues flagged (inline comments posted)
Bugbot verdict: Solid first component drop — approve with follow-ups above before Preview promotion. No security blockers (producer is trusted route config; consumer polls configured registry). AI-generated Bugbot review on behalf of atiaomar1978-hub. |
Bugbot — test coverage reviewAssessed unit + IT coverage for Coverage matrix (producer operations)
Consumer coverage
Config / auth
VerdictAdequate for Preview merge — strong lifecycle IT and good mocked coverage for core CRUD. Not yet comprehensive for a GA component: three producer ops lack unit tests, validation/compatibility negative paths are thin, consumer watermark/ Priority additions before promotion:
AI-generated Bugbot test-coverage review on behalf of atiaomar1978-hub. |
Grok — test coverage reviewVerdict: Good foundation, gaps on edge cases and half the producer surface at unit level What is well covered
Gaps by risk
Test quality notes
Scorecard
AI-generated Grok test-coverage review on behalf of atiaomar1978-hub. |
| from("direct:updateArtifact") | ||
| .to("apicurio-registry:testGroup/testArtifact?registryUrl=http://localhost:8080/apis/registry/v3&operation=updateArtifact"); | ||
|
|
||
| from("direct:searchArtifacts") |
There was a problem hiding this comment.
Bugbot (test coverage): Route for searchArtifacts is wired in createRouteBuilder() but there is no @Test exercising it. Eight of ten producer ops have unit tests; searchArtifacts, testCompatibility, and validate are missing at unit level.
AI-generated Bugbot test-coverage comment on behalf of atiaomar1978-hub.
| } | ||
|
|
||
| @Test | ||
| void testPollNewVersions() throws Exception { |
There was a problem hiding this comment.
Bugbot (test coverage): Consumer tests cover happy-path polling but not: (1) fetchContent=true body vs metadata, (2) missing groupId/artifactId IllegalArgumentException, (3) out-of-order globalId list (regression for watermark bug). Recommend at least one test per gap.
AI-generated Bugbot test-coverage comment on behalf of atiaomar1978-hub.
| } | ||
|
|
||
| @Test | ||
| void testValidateCompatibleContent() { |
There was a problem hiding this comment.
Bugbot (test coverage): validate IT only checks header is non-null with failOnValidation=false. Missing: assert HEADER_VALIDATION_RESULT=true, incompatible content → false + error header, and failOnValidation=true → ApicurioRegistryValidationException.
AI-generated Bugbot test-coverage comment on behalf of atiaomar1978-hub.
| Map<String, Object> groupHeaders = new HashMap<>(); | ||
| groupHeaders.put(ApicurioRegistryConstants.HEADER_OPERATION, ApicurioRegistryConstants.OPERATION_CREATE_GROUP); | ||
| groupHeaders.put(ApicurioRegistryConstants.HEADER_GROUP_ID, groupId); | ||
| template.request("direct:createForConsumer", exchange -> exchange.getIn().setHeaders(groupHeaders)); |
There was a problem hiding this comment.
Bugbot (test coverage): IT asserts first consumed message headers only. No coverage for: publishing a second version and verifying exactly one new exchange, fetchContent=true, or consumer watermark/idempotency across polls.
AI-generated Bugbot test-coverage comment on behalf of atiaomar1978-hub.
| } | ||
|
|
||
| @Test | ||
| void testEndpointWithAuthOptions() throws Exception { |
There was a problem hiding this comment.
Bugbot (test coverage): Auth options tested for basic config binding only. authType=oidc (+ token endpoint / client credentials) and actual authenticated SDK client creation are untested (unit or IT).
AI-generated Bugbot test-coverage comment on behalf of atiaomar1978-hub.
| protected RoutesBuilder createRouteBuilder() { | ||
| return new RouteBuilder() { | ||
| @Override | ||
| public void configure() { |
There was a problem hiding this comment.
Grok (test coverage): testFullArtifactLifecycle is excellent end-to-end coverage (7 ops in one flow). Consider splitting failure-path tests: delete missing artifact, update before create, invalid ifExists value.
AI-generated Grok test-coverage comment on behalf of atiaomar1978-hub.
| endpoint.setRegistryClient(mockClient); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Grok (test coverage): Mocked producer tests verify SDK delegation but not header propagation (ifExists, contentType, version, URI path overrides via headers). One parameterized test per header family would tighten regression safety.
AI-generated Grok test-coverage comment on behalf of atiaomar1978-hub.
|
|
||
| Exchange result = template.request("direct:testCompatibility", exchange -> { | ||
| exchange.getIn().setBody(compatibleSchema); | ||
| }); |
There was a problem hiding this comment.
Grok (test coverage): testTestCompatibility only asserts body non-null — does not assert true/false or HEADER_VALIDATION_ERRORS on incompatible schema. Pair with a negative case mirroring validate.
AI-generated Grok test-coverage comment on behalf of atiaomar1978-hub.
|
okay so main is now ready for 4.23.0-SNAPSHOT and this PR needs to be rebased on top and update its versions |
4 participants
New component wrapping the Apicurio Registry v3 REST API via
io.apicurio:apicurio-registry-java-sdk.URI format:
apicurio-registry:groupId/artifactId[?options]Producer operations:
createArtifact,updateArtifact,deleteArtifact,getArtifactContent,getArtifactMetadata,searchArtifacts,listVersions,createGroup,testCompatibility,validate.Consumer polls for new artifact versions using a
globalIdwatermark.Auth:
none,basic,oidc(OAuth2 client credentials).Includes unit tests (mocked SDK), integration tests (Testcontainers with
quay.io/apicurio/apicurio-registry:3.0.6), and AsciiDoc documentation.Related: Apicurio/apicurio-registry#8671