gnodet-bot commented on code in PR #26562:
URL: https://github.com/apache/camel/pull/26562#discussion_r4039571662
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/CatalogSamples.java:
##########
@@ -134,57 +153,210 @@ public static List<String> names() {
* Samples for the given EIP or entry name (camelCase, kebab-case or any
case), with where it goes.
*/
public static JsonObject sample(String name, int limit) {
- return sample(null, name, limit);
+ return sample(null, null, name, limit);
+ }
+
+ /**
+ * Samples for the given name of any kind, with where it goes; from the
catalog's documentation for the Camel
+ * version in use when it has the page, else the shipped set.
+ */
+ public static JsonObject sample(CamelCatalog catalog, String name, int
limit) {
+ return sample(catalog, null, name, limit);
}
private static final Pattern YAML_BLOCK =
Pattern.compile("\\[source,yaml\\]\\s*\\n----\\n(.*?)\\n----", Pattern.DOTALL);
- private static final Map<String, List<Map<String, String>>> DOC_CACHE =
new java.util.concurrent.ConcurrentHashMap<>();
+ private static final Map<String, Found> DOC_CACHE = new
ConcurrentHashMap<>();
+
+ /** The validated examples of a page family, and for a component whether
any of them uses its endpoint. */
+ record Found(List<Map<String, String>> samples, boolean endpoint) {
+ static final Found NONE = new Found(List.of(), false);
+ }
/**
- * The validated YAML examples of the EIP page in the catalog, or an empty
list when the catalog has no such page or
- * none of its examples validate. Cached per name for the default catalog.
+ * The validated YAML examples of the documentation of the given kind and
name in the catalog, or none when the
+ * catalog has no such page or none of its examples validate. For an EIP
the page named after it
+ * (circuitBreaker-eip, pollEnrich-eip; a few are kebab-case, and the
pattern pages have no suffix:
+ * dead-letter-channel, intercept); for a component its page and
sub-pages, the examples using its endpoint first;
+ * for a data format or a language its page. Cached per kind and name for
the default catalog.
*/
- static List<Map<String, String>> fromCatalog(CamelCatalog catalog, String
key) {
+ static Found fromCatalog(CamelCatalog catalog, String kind, String key) {
if (catalog == null) {
- return List.of();
+ return Found.NONE;
}
- // the catalog names the pages after the EIP (circuitBreaker-eip,
pollEnrich-eip); a few are kebab-case, and the
- // pattern pages have no suffix (dead-letter-channel, intercept)
boolean cacheable = catalog.getCatalogVersion() != null &&
catalog.getLoadedVersion() == null;
- if (cacheable && DOC_CACHE.containsKey(key)) {
- return DOC_CACHE.get(key);
+ String cacheKey = kind + ":" + key;
+ if (cacheable) {
+ Found cached = DOC_CACHE.get(cacheKey);
+ if (cached != null) {
+ return cached;
+ }
}
- List<Map<String, String>> answer = new ArrayList<>();
+ Found found;
try {
- String page = null;
- String doc = null;
- for (String candidate : List.of(key + "-eip", kebab(key) + "-eip",
kebab(key), key)) {
- doc = catalog.asciiDoc(candidate);
- if (doc != null) {
- page = candidate;
+ found = switch (kind) {
+ case "component" -> componentSamples(catalog, key);
+ case "dataformat" -> pageSamples(catalog,
dataFormatPages(catalog, key), null);
+ case "language" -> pageSamples(catalog, List.of(key +
"-language"), null);
+ default -> eipSamples(catalog, key);
+ };
+ } catch (Exception e) {
+ // a page that cannot be read or validated: the shipped samples
are the fallback
Review Comment:
⚠️ **Silent swallow hides real bugs.** Any exception thrown by
`componentSamples()`, `pageSamples()`, `eipSamples()`, or `dataFormatPages()` —
a `NullPointerException`, `ClassCastException`, or I/O error from
`catalog.asciiDoc()` — is silently replaced by `Found.NONE`. The calling code
returns a hint of `"camel_catalog_doc gives its options"`, which is the same as
a legitimately empty page. There is no way to tell in production whether the
catalog had no examples or threw an exception.
At minimum log at debug:
```suggestion
} catch (Exception e) {
// a page that cannot be read or validated: the shipped samples
are the fallback
LOG.debug("Failed to load catalog samples for kind={} name={}",
kind, key, e);
found = Found.NONE;
}
```
(Add `private static final Logger LOG =
LoggerFactory.getLogger(CatalogSamples.class);` at the top of the class if not
already there.)
##########
dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/CatalogSamplesTest.java:
##########
@@ -158,10 +164,161 @@ void theGeneratedSamplesCoverTheEipsAndTheFileEntries() {
@Test
void patternPagesWithoutTheEipSuffixAreReadFromTheCatalog() {
- org.apache.camel.catalog.CamelCatalog catalog = new
org.apache.camel.catalog.DefaultCamelCatalog();
+ CamelCatalog catalog = new DefaultCamelCatalog();
JsonArray samples = (JsonArray) CatalogSamples.sample(catalog,
"deadLetterChannel", 1).get("samples");
assertThat(((JsonObject)
samples.get(0)).getString("source")).startsWith("dead-letter-channel.adoc
(Camel ");
samples = (JsonArray) CatalogSamples.sample(catalog,
"keyValueRepository", 1).get("samples");
assertThat(((JsonObject)
samples.get(0)).getString("source")).startsWith("keyValueRepository.adoc (Camel
");
}
+
+ // CAMEL-24720: components, data formats and languages from their
documentation
+
+ private static String yaml(JsonObject answer, int i) {
+ return ((JsonObject) ((JsonArray)
answer.get("samples")).get(i)).getString("yaml");
+ }
+
+ private static String source(JsonObject answer, int i) {
+ return ((JsonObject) ((JsonArray)
answer.get("samples")).get(i)).getString("source");
+ }
+
+ @Test
+ void componentSamplesComeFromTheComponentPageWithItsEndpointFirst() {
Review Comment:
⚠️ **`DOC_CACHE` is not isolated between tests.** `CatalogSamples.DOC_CACHE`
is `private static final ConcurrentHashMap` — one instance for the entire JVM
lifetime. Tests using `DefaultCamelCatalog` are cacheable (`getCatalogVersion()
!= null && getLoadedVersion() == null`), so any lookup from an earlier test
persists into later tests. There is no `@BeforeEach`/`@AfterEach` to clear it,
making test isolation order-dependent.
Add a package-private clear method and call it in setup:
```java
// In CatalogSamples.java — package-private for testing:
static void clearCacheForTesting() {
DOC_CACHE.clear();
}
```
```java
// In CatalogSamplesTest.java:
@BeforeEach
void resetCache() {
CatalogSamples.clearCacheForTesting();
}
```
--
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]
