Croway commented on code in PR #28241: URL: https://github.com/apache/camel/pull/28241#discussion_r4337125668
########## tooling/spi-annotations/src/main/java/org/apache/camel/spi/BodyType.java: ########## @@ -0,0 +1,148 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.spi; + +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * One side of {@link BodyTypes}: the body a step needs or leaves. + * <p/> + * The carriers are coarse on purpose: a carrier that is right is better than a precise Java type that is wrong. Give + * the {@link #javaType()} only when the body is always of that type. + * + * @since 4.24 + */ +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Target({}) +public @interface BodyType { + + /** + * The carriers of the body: the kinds of Java object it is in. Several for a body that can be any of them. None + * when the side is not known. + */ + Carrier[] value() default {}; + + /** + * The Java type of the body, when it is always of this type, such as + * {@code org.apache.camel.component.file.GenericFile}. + */ + String javaType() default ""; + + /** + * The format of the payload, when it is always the same: json, xml, csv, yaml, text or binary. These are the names + * of the data types of Kamelets. + */ + String format() default ""; + + /** + * The options the body depends on. The carriers hold while these options keep their default values; with another + * value the body is not known, unless {@link #byOption()} says it. + */ + String[] dependsOn() default {}; + + /** + * The body for other values of an option, such as {@code output=bytes}. + */ + When[] byOption() default {}; + + /** + * What to know about the body, in a sentence for the documentation and the tools. + */ + String description() default ""; + + /** + * The body when an option has a value. + */ + @Retention(RetentionPolicy.RUNTIME) + @Documented + @Target({}) + @interface When { + + /** + * The name of the option. + */ + String option(); + + /** + * The value of the option. + */ + String value(); + + /** + * The carriers of the body with this value. + */ + Carrier[] carrier(); + + /** + * The Java type of the body with this value, when it is always of this type. + */ + String javaType() default ""; + } + + /** + * The kinds of Java object a body is in. The runtime sorts the bodies it records into kinds as well (null, payload, + * data and collection): text, bytes, a stream, a file and a value are a payload, a map and a pojo are data, a list + * is a collection. + */ + enum Carrier { + + /** No body. */ + NULL("null"), + /** Text: a String. */ + TEXT("text"), + /** A byte array. */ + BYTES("bytes"), + /** A stream: an InputStream, a Reader, or the stream cache of Camel. */ + STREAM("stream"), + /** A file: the GenericFile of the file consumers, a java.io.File or a Path. */ + FILE("file"), + /** A single value: a number, a boolean or an enum. */ + VALUE("value"), + /** A java.util.Map, such as a JSON object parsed or a row of a query. */ + MAP("map"), + /** A List, another Collection, an array or an Iterator, such as a JSON array parsed or the rows of a query. */ + LIST("list"), Review Comment: `LIST` also covers an `Iterator`, but an iterator is not indexable and has no `size()`, so tooling that trusts `list` accepts reads that fail at runtime. With #28242: ```yaml - from: uri: file:inbox steps: - unmarshal: csv: lazyLoad: true # catalog: unmarshalOut lazyLoad=true -> list (java.util.Iterator) - log: "rows: ${body.size()}" # CsvIterator: no size(), no [0] ``` Same for sql `StreamList` (`ResultSetIterator`), which is the `${body.size()}` collision of the r17 benchmark series on CAMEL-24844. Suggest a separate `ITERATOR` carrier now: adding a constant later is easy, narrowing what `list` means after release is not. _Claude Code on behalf of Croway_ ########## tooling/maven/camel-package-maven-plugin/src/main/java/org/apache/camel/maven/packaging/BodyTypesHelper.java: ########## @@ -0,0 +1,185 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.maven.packaging; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.apache.camel.spi.BodyType; +import org.apache.camel.spi.BodyType.Carrier; +import org.apache.camel.spi.BodyTypes; +import org.apache.camel.tooling.model.BaseModel; +import org.apache.camel.tooling.model.BodyTypeModel; +import org.apache.camel.tooling.model.ComponentModel; + +/** + * Carries the {@link BodyTypes} of a component, data format or language into its model, which the catalog JSON is + * written from (CAMEL-24961). + */ +final class BodyTypesHelper { + + private static final Set<String> IN = Set.of(BodyTypeModel.CONSUMER_REPLY, BodyTypeModel.PRODUCER_IN, + BodyTypeModel.MARSHAL_IN, BodyTypeModel.UNMARSHAL_IN, BodyTypeModel.EXPRESSION_IN); + + private BodyTypesHelper() { + } + + /** + * Adds the sides the annotation gives to the model, in the order of the annotation. + * + * @param model the model of the component, data format or language + * @param bodyTypes the annotation, null when the class has none + * @param owner the class with the annotation, for the error message + * @throws IllegalArgumentException when a side does not fit the model: a data format side on a component, a + * producer side on a consumer-only component, unchanged on what a step needs or + * any on what it leaves + */ + static void addBodyTypes(BaseModel<?> model, BodyTypes bodyTypes, Class<?> owner) { + if (bodyTypes == null) { + return; + } + Map<String, BodyType> sides = new LinkedHashMap<>(); + sides.put(BodyTypeModel.CONSUMER_OUT, bodyTypes.consumerOut()); + sides.put(BodyTypeModel.CONSUMER_REPLY, bodyTypes.consumerReply()); + sides.put(BodyTypeModel.PRODUCER_IN, bodyTypes.producerIn()); + sides.put(BodyTypeModel.PRODUCER_OUT, bodyTypes.producerOut()); + sides.put(BodyTypeModel.MARSHAL_IN, bodyTypes.marshalIn()); + sides.put(BodyTypeModel.MARSHAL_OUT, bodyTypes.marshalOut()); + sides.put(BodyTypeModel.UNMARSHAL_IN, bodyTypes.unmarshalIn()); + sides.put(BodyTypeModel.UNMARSHAL_OUT, bodyTypes.unmarshalOut()); + sides.put(BodyTypeModel.EXPRESSION_IN, bodyTypes.expressionIn()); + sides.put(BodyTypeModel.EXPRESSION_OUT, bodyTypes.expressionOut()); + + Set<String> allowed = allowedSides(model); + Set<String> options = optionNames(model); + for (Map.Entry<String, BodyType> entry : sides.entrySet()) { + String side = entry.getKey(); + BodyType bodyType = entry.getValue(); + if (bodyType.value().length == 0) { + if (!bodyType.javaType().isEmpty() || !bodyType.format().isEmpty() || bodyType.dependsOn().length > 0 + || bodyType.byOption().length > 0 || !bodyType.description().isEmpty()) { + throw new IllegalArgumentException( + owner.getName() + ": the @BodyTypes " + side + " has no carriers, but other attributes"); + } + continue; + } + if (!allowed.contains(side)) { + throw new IllegalArgumentException( + owner.getName() + ": the @BodyTypes " + side + " is not a side of this " + model.getKind() + + ", which has " + allowed); + } + check(owner, side, bodyType.value()); + for (String option : bodyType.dependsOn()) { + checkOption(owner, side, option, options); + } + for (BodyType.When when : bodyType.byOption()) { + checkOption(owner, side, when.option(), options); + check(owner, side + " " + when.option() + "=" + when.value(), when.carrier()); + } + model.getBodyTypes().put(side, asModel(bodyType)); + } + } + + private static Set<String> allowedSides(BaseModel<?> model) { + return switch (model.getKind()) { + case component -> { + ComponentModel component = (ComponentModel) model; + if (component.isConsumerOnly()) { + yield Set.of(BodyTypeModel.CONSUMER_OUT, BodyTypeModel.CONSUMER_REPLY); + } else if (component.isProducerOnly()) { + yield Set.of(BodyTypeModel.PRODUCER_IN, BodyTypeModel.PRODUCER_OUT); + } + yield Set.of(BodyTypeModel.CONSUMER_OUT, BodyTypeModel.CONSUMER_REPLY, BodyTypeModel.PRODUCER_IN, + BodyTypeModel.PRODUCER_OUT); + } + case dataformat -> Set.of(BodyTypeModel.MARSHAL_IN, BodyTypeModel.MARSHAL_OUT, BodyTypeModel.UNMARSHAL_IN, + BodyTypeModel.UNMARSHAL_OUT); + case language -> Set.of(BodyTypeModel.EXPRESSION_IN, BodyTypeModel.EXPRESSION_OUT); + default -> Set.of(); + }; + } + + /** The options of the model, which the body types can depend on. */ + private static Set<String> optionNames(BaseModel<?> model) { + Set<String> names = new HashSet<>(); + model.getOptions().forEach(o -> names.add(o.getName())); + if (model instanceof ComponentModel component) { + component.getComponentOptions().forEach(o -> names.add(o.getName())); + component.getEndpointOptions().forEach(o -> names.add(o.getName())); + } + return names; + } + + private static void checkOption(Class<?> owner, String side, String option, Set<String> options) { + if (!options.contains(option)) { + throw new IllegalArgumentException( + owner.getName() + ": the @BodyTypes " + side + " depends on " + option + ", which is not an option"); + } + } + + /** + * UNCHANGED is for the body after a step, such as a sql query that leaves it unchanged unless it is a select. ANY + * is for the bodies a step needs. ANY and UNKNOWN stand alone. + */ + private static void check(Class<?> owner, String side, Carrier[] carriers) { Review Comment: `check()` accepts an empty array, and `When.carrier()` can be given one explicitly: ```java @BodyTypes(producerOut = @BodyType(value = Carrier.TEXT, byOption = @BodyType.When(option = "output", value = "bytes", carrier = {}))) ``` The build passes and writes `"carriers": []`; `CatalogDocs.carriers()` in #28242 then calls `subList(0, -1)` and `camel_catalog_doc` fails for that component (the crash gnodet-bot noted there). Rejecting an empty array here stops it at build time: ```java if (carriers.length == 0) { throw new IllegalArgumentException(owner.getName() + ": the @BodyTypes " + side + " has no carriers"); } ``` _Claude Code on behalf of Croway_ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
