mattcasters commented on code in PR #8408: URL: https://github.com/apache/hop/pull/8408#discussion_r4028631536
########## plugins/transforms/plugincatalog/src/main/java/org/apache/hop/pipeline/transforms/plugincatalog/PluginCatalogMeta.java: ########## @@ -0,0 +1,145 @@ +/* + * 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.hop.pipeline.transforms.plugincatalog; + +import lombok.Getter; +import lombok.Setter; +import org.apache.hop.core.annotations.Transform; +import org.apache.hop.core.exception.HopTransformException; +import org.apache.hop.core.row.IRowMeta; +import org.apache.hop.core.row.value.ValueMetaBoolean; +import org.apache.hop.core.row.value.ValueMetaString; +import org.apache.hop.core.variables.IVariables; +import org.apache.hop.metadata.api.HopMetadataProperty; +import org.apache.hop.metadata.api.IHopMetadataProvider; +import org.apache.hop.pipeline.transform.BaseTransformMeta; +import org.apache.hop.pipeline.transform.TransformMeta; + +/** + * Plugin Catalog transform metadata. + * + * <p>Emits one row per plugin (or per plugin property) by reflecting over the live Hop {@link + * org.apache.hop.core.plugins.PluginRegistry}. Labels are resolved to human-readable text; output + * is never stale against the running Hop version or any installed third-party plugins. + */ +@Getter +@Setter +@Transform( + id = "PluginCatalog", + image = "plugincatalog.svg", + name = "i18n::PluginCatalog.Name", + description = "i18n::PluginCatalog.Description", + categoryDescription = "i18n:org.apache.hop.pipeline.transform:BaseTransform.Category.Input", + documentationUrl = + "https://hop.apache.org/manual/latest/pipeline/transforms/plugincatalog.html";, + keywords = "i18n::PluginCatalog.Keywords") +public class PluginCatalogMeta extends BaseTransformMeta<PluginCatalog, PluginCatalogData> { Review Comment: **[bug]** This is documented and implemented as a row-generating source that never calls `getRow()`, but `PluginCatalogMeta` does not override `canStartWithoutInput()`, `consumesMainInput()`, or `getTransformIOMeta()`. Defaults are `canStartWithoutInput() == false` and `consumesMainInput() == true` (`ITransformMeta`). Consequences: the palette/add-transform dialog will not tag it as a pipeline source (`TransformSourceGui` / `TransformSourceSupport.isPipelineSourceAtDefault`); Verify will not emit the `CAN_START_WITHOUT_INPUT` comment; canvas tooltips will omit the source suffix; incoming hops stay allowed even though `processRow()` never drains them, so upstream rows are silently dropped. Other sources in this tree (Row Generator, Data Grid, Pipeline Logging, Data Set Input) all override the three methods and, when `consumesMainInput()` is false, belong in `never-consume-at-default-plugins.txt` if that inventory can see the plugin. **Suggestion:** Mirror `RowGeneratorMeta` / `DataGridMeta`: `canStartWithoutInput()` return true, `consumesMainInput()` return false, `getTransformIOMeta()` return `new TransformIOMeta(false, true, false, false, false, false)`. Add a `check()` remark when input hops are present. If engine tests load this plugin, add `PluginCatalog` to `engine/src/test/resources/org/apache/hop/pipeline/transform/never-consume-at-default-plugins.txt`. ########## plugins/transforms/plugincatalog/src/main/java/org/apache/hop/pipeline/transforms/plugincatalog/PluginCatalogReader.java: ########## @@ -0,0 +1,291 @@ +/* + * 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.hop.pipeline.transforms.plugincatalog; + +import java.lang.reflect.Field; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.function.BiConsumer; +import org.apache.hop.core.plugins.ActionPluginType; +import org.apache.hop.core.plugins.IPlugin; +import org.apache.hop.core.plugins.PluginRegistry; +import org.apache.hop.core.plugins.TransformPluginType; +import org.apache.hop.i18n.LanguageChoice; +import org.apache.hop.metadata.api.HopMetadataProperty; +import org.apache.hop.metadata.plugin.MetadataPluginType; + +/** + * Reads the live {@link PluginRegistry} and reflects over each plugin's metadata class to expose a + * structured, never-stale catalog of transforms, actions and metadata types. + * + * <p>Field extraction is split out as a pure static method so it can be unit-tested against fixture + * classes without bootstrapping a Hop plugin registry. + */ +public class PluginCatalogReader { + + /** Maximum nesting depth when descending into complex {@code @HopMetadataProperty} groups. */ + private static final int MAX_NESTING_DEPTH = 1; + + private final PluginRegistry registry; + + public PluginCatalogReader() { + this(PluginRegistry.getInstance()); + } + + public PluginCatalogReader(PluginRegistry registry) { + this.registry = registry; + } + + /** + * Enumerate the requested plugin families. + * + * @param warnings optional sink for non-fatal problems (e.g. a metadata class that fails to + * load); may be {@code null} + */ + public List<PluginRecord> readAll( + boolean includeTransforms, + boolean includeActions, + boolean includeMetadataTypes, + BiConsumer<String, Throwable> warnings) { + List<PluginRecord> out = new ArrayList<>(); + if (includeTransforms) { + addFamily(registry.getPlugins(TransformPluginType.class), "transform", out, warnings); + } + if (includeActions) { + addFamily(registry.getPlugins(ActionPluginType.class), "action", out, warnings); + } + if (includeMetadataTypes) { + addFamily(registry.getPlugins(MetadataPluginType.class), "metadata", out, warnings); + } + // name/description/category come back resolved against the running Hop locale, so stamp every + // record with that locale and keep the English aliases beside it. Without this a catalog built + // on a non-English installation is silently locale-specific. + String localeTag = currentLocaleTag(); + for (PluginRecord record : out) { + record.locale = localeTag; + } + return out; + } + + private void addFamily( + List<IPlugin> plugins, + String pluginType, + List<PluginRecord> out, + BiConsumer<String, Throwable> warnings) { + if (plugins == null) { + return; + } + for (IPlugin plugin : plugins) { + String id = firstId(plugin); + if (id.isEmpty()) { + // Skip anonymous/unregistered plugins rather than emit a junk row with no id. + if (warnings != null) { + warnings.accept("Skipping " + pluginType + " plugin with no id", null); + } + continue; + } + PluginRecord record = new PluginRecord(); + record.pluginId = id; + record.pluginType = pluginType; + record.name = clean(plugin.getName()); + record.description = clean(plugin.getDescription()); + record.category = clean(plugin.getCategory()); + record.keywords = joinKeywords(plugin.getKeywords()); + record.englishAliases = joinKeywords(plugin.getEnglishKeywords()); + String className = resolveClassName(plugin); + record.className = className == null ? "" : className; + if (className != null) { + try { + ClassLoader classLoader = registry.getClassLoader(plugin); + Class<?> clazz = classLoader.loadClass(className); + record.properties = extractProperties(clazz); + } catch (Throwable e) { + if (warnings != null) { + warnings.accept("Could not reflect properties for plugin '" + record.pluginId + "'", e); + } + } + } + out.add(record); + } + } + + /** + * Extract every {@code @HopMetadataProperty} field declared on {@code clazz} (including inherited + * fields), descending one level into complex property groups. + */ + public static List<PropertyRecord> extractProperties(Class<?> clazz) { + List<PropertyRecord> out = new ArrayList<>(); + collect(clazz, "", out, 0); + return out; + } + + private static void collect(Class<?> clazz, String group, List<PropertyRecord> out, int depth) { + if (clazz == null) { + return; + } + for (Class<?> c = clazz; c != null && c != Object.class; c = c.getSuperclass()) { + for (Field field : c.getDeclaredFields()) { + HopMetadataProperty annotation = field.getAnnotation(HopMetadataProperty.class); + if (annotation == null) { + continue; + } + String xmlKey = annotation.key().isEmpty() ? field.getName() : annotation.key(); + out.add( + new PropertyRecord( + field.getName(), + xmlKey, + field.getType().getSimpleName(), + annotation.password(), + group)); + if (depth < MAX_NESTING_DEPTH) { + Class<?> nested = complexHopType(field); + if (nested != null) { + collect(nested, xmlKey, out, depth + 1); + } + } + } + } + } + + /** + * If {@code field} is a Hop value class (or a collection of one) that itself carries + * {@code @HopMetadataProperty} fields, return that class; otherwise {@code null}. + */ + private static Class<?> complexHopType(Field field) { + Class<?> candidate; + if (Collection.class.isAssignableFrom(field.getType())) { + candidate = collectionElementType(field); + } else { + candidate = field.getType(); + } + if (candidate == null + || candidate.isEnum() + || candidate.isPrimitive() + || !candidate.getName().startsWith("org.apache.hop")) { Review Comment: **[bug]** Nested `@HopMetadataProperty` descent is skipped unless `candidate.getName().startsWith("org.apache.hop")`. Top-level properties of third-party plugins are still listed, but nested groups (field mappings, tabular groups, nested configs) in any other package are omitted. Docs and issue #8403 claim the catalog includes Marketplace/third-party plugins and that property reflection is the point of the transform. The filter also uses a package prefix instead of excluding JDK/primitives (already handled) / enums. **Suggestion:** Drop the `org.apache.hop` whitelist. Keep the enum/primitive/null guards; optionally skip `java.` / `javax.` / `jakarta.` / arrays. Drive nested descent off “type (or collection element, or `listItemClass()`) has `@HopMetadataProperty` fields”, which is what `complexHopType` already does after the package check. Add a unit-test fixture in a non-`org.apache.hop` package (or a nested class name that would fail the prefix) so this cannot regress. ########## plugins/transforms/plugincatalog/src/main/java/org/apache/hop/pipeline/transforms/plugincatalog/PluginCatalogDialog.java: ########## @@ -0,0 +1,136 @@ +/* + * 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.hop.pipeline.transforms.plugincatalog; + +import org.apache.hop.core.util.Utils; +import org.apache.hop.core.variables.IVariables; +import org.apache.hop.i18n.BaseMessages; +import org.apache.hop.pipeline.PipelineMeta; +import org.apache.hop.ui.core.PropsUi; +import org.apache.hop.ui.core.dialog.BaseDialog; +import org.apache.hop.ui.pipeline.transform.BaseTransformDialog; +import org.eclipse.swt.SWT; +import org.eclipse.swt.custom.CCombo; +import org.eclipse.swt.layout.FormAttachment; +import org.eclipse.swt.layout.FormData; +import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Shell; + +public class PluginCatalogDialog extends BaseTransformDialog { + + private static final Class<?> PKG = PluginCatalogMeta.class; + + private final PluginCatalogMeta input; + + private Button wIncludeTransforms; + private Button wIncludeActions; + private Button wIncludeMetadataTypes; + private CCombo wDetailLevel; + + public PluginCatalogDialog( + Shell parent, + IVariables variables, + PluginCatalogMeta transformMeta, + PipelineMeta pipelineMeta) { + super(parent, variables, transformMeta, pipelineMeta); + input = transformMeta; + } + + @Override + public String open() { Review Comment: **[bug]** New transform dialogs must start from annotated metadata widgets in a grouped `GuiCompositeWidgets` container, not hand-laid `FormAttachment` rows on the shell. `PluginCatalogMeta` has no `@GuiPlugin` / `@GuiWidgetElement` (with `groupType` / `group`); `open()` builds checkboxes and a `CCombo` on the shell after `buildButtonBar()`. Checkboxes are attached at `left = middle` with no left-hand labels, so the left half of each row is empty. `changed` is never captured with `changed = input.hasChanged()` before `input.setChanged(changed)`, so opening the dialog can clear a previously dirty transform; `ok()` also never calls `input.setChanged()`. **Suggestion:** Annotate the four options on `PluginCatalogMeta` (`CHECKBOX` x3, `COMBO` for `DetailLevel`; GuiCompositeWidgets already fills combo values from enum constants). Use `groupType = GuiWidgetGroupType.BOXES` and a single group. Dialog `open()` should be `createShell` + `buildButtonBar` + `GuiCompositeWidgets.addScrolledComposite(shell, variables, wTransformName, wOk, PARENT_ID, input)`; on OK, `widgets.getWidgetsContents(input, PARENT_ID)` and set the transform name. Capture `changed = input.hasChanged()` at the start of `open()`. ########## plugins/transforms/plugincatalog/src/main/java/org/apache/hop/pipeline/transforms/plugincatalog/PluginCatalogReader.java: ########## @@ -0,0 +1,291 @@ +/* + * 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.hop.pipeline.transforms.plugincatalog; + +import java.lang.reflect.Field; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.function.BiConsumer; +import org.apache.hop.core.plugins.ActionPluginType; +import org.apache.hop.core.plugins.IPlugin; +import org.apache.hop.core.plugins.PluginRegistry; +import org.apache.hop.core.plugins.TransformPluginType; +import org.apache.hop.i18n.LanguageChoice; +import org.apache.hop.metadata.api.HopMetadataProperty; +import org.apache.hop.metadata.plugin.MetadataPluginType; + +/** + * Reads the live {@link PluginRegistry} and reflects over each plugin's metadata class to expose a + * structured, never-stale catalog of transforms, actions and metadata types. + * + * <p>Field extraction is split out as a pure static method so it can be unit-tested against fixture + * classes without bootstrapping a Hop plugin registry. + */ +public class PluginCatalogReader { + + /** Maximum nesting depth when descending into complex {@code @HopMetadataProperty} groups. */ + private static final int MAX_NESTING_DEPTH = 1; + + private final PluginRegistry registry; + + public PluginCatalogReader() { + this(PluginRegistry.getInstance()); + } + + public PluginCatalogReader(PluginRegistry registry) { + this.registry = registry; + } + + /** + * Enumerate the requested plugin families. + * + * @param warnings optional sink for non-fatal problems (e.g. a metadata class that fails to + * load); may be {@code null} + */ + public List<PluginRecord> readAll( + boolean includeTransforms, + boolean includeActions, + boolean includeMetadataTypes, + BiConsumer<String, Throwable> warnings) { + List<PluginRecord> out = new ArrayList<>(); + if (includeTransforms) { + addFamily(registry.getPlugins(TransformPluginType.class), "transform", out, warnings); + } + if (includeActions) { + addFamily(registry.getPlugins(ActionPluginType.class), "action", out, warnings); + } + if (includeMetadataTypes) { + addFamily(registry.getPlugins(MetadataPluginType.class), "metadata", out, warnings); + } + // name/description/category come back resolved against the running Hop locale, so stamp every + // record with that locale and keep the English aliases beside it. Without this a catalog built + // on a non-English installation is silently locale-specific. + String localeTag = currentLocaleTag(); + for (PluginRecord record : out) { + record.locale = localeTag; + } + return out; + } + + private void addFamily( + List<IPlugin> plugins, + String pluginType, + List<PluginRecord> out, + BiConsumer<String, Throwable> warnings) { + if (plugins == null) { + return; + } + for (IPlugin plugin : plugins) { + String id = firstId(plugin); + if (id.isEmpty()) { + // Skip anonymous/unregistered plugins rather than emit a junk row with no id. + if (warnings != null) { + warnings.accept("Skipping " + pluginType + " plugin with no id", null); + } + continue; + } + PluginRecord record = new PluginRecord(); + record.pluginId = id; + record.pluginType = pluginType; + record.name = clean(plugin.getName()); + record.description = clean(plugin.getDescription()); + record.category = clean(plugin.getCategory()); + record.keywords = joinKeywords(plugin.getKeywords()); + record.englishAliases = joinKeywords(plugin.getEnglishKeywords()); + String className = resolveClassName(plugin); + record.className = className == null ? "" : className; + if (className != null) { + try { + ClassLoader classLoader = registry.getClassLoader(plugin); + Class<?> clazz = classLoader.loadClass(className); + record.properties = extractProperties(clazz); + } catch (Throwable e) { + if (warnings != null) { + warnings.accept("Could not reflect properties for plugin '" + record.pluginId + "'", e); + } + } + } + out.add(record); + } + } + + /** + * Extract every {@code @HopMetadataProperty} field declared on {@code clazz} (including inherited + * fields), descending one level into complex property groups. + */ + public static List<PropertyRecord> extractProperties(Class<?> clazz) { + List<PropertyRecord> out = new ArrayList<>(); + collect(clazz, "", out, 0); + return out; + } + + private static void collect(Class<?> clazz, String group, List<PropertyRecord> out, int depth) { + if (clazz == null) { + return; + } + for (Class<?> c = clazz; c != null && c != Object.class; c = c.getSuperclass()) { + for (Field field : c.getDeclaredFields()) { + HopMetadataProperty annotation = field.getAnnotation(HopMetadataProperty.class); + if (annotation == null) { + continue; + } + String xmlKey = annotation.key().isEmpty() ? field.getName() : annotation.key(); Review Comment: **[suggestion]** `property_xml_key` is documented as the key used in `.hpl` / `.hwf` XML, but only `annotation.key()` (or the Java field name) is recorded. Hop list/map properties almost always set `groupKey` as the wrapper (`<fields><field>…`) — see `XmlMetadataUtil.serializeListToXml` / `openTag(groupKey)`. Consumers reconstructing serialization paths (the AI / migration use case) will miss that wrapper. `isExcludedFromSerialization` and getter-method `@HopMetadataProperty` (also serialized by `XmlMetadataUtil`) are likewise ignored. **Suggestion:** Emit `groupKey` as well (extra column, or encode wrapper+item in `property_xml_key` / `property_group`). Skip `isExcludedFromSerialization()`. Optionally scan `get*` methods the same way `XmlMetadataUtil` does. Document that `property_group` is nesting, not `groupKey`. ########## plugins/transforms/plugincatalog/src/main/java/org/apache/hop/pipeline/transforms/plugincatalog/PluginCatalogMeta.java: ########## @@ -0,0 +1,145 @@ +/* + * 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.hop.pipeline.transforms.plugincatalog; + +import lombok.Getter; +import lombok.Setter; +import org.apache.hop.core.annotations.Transform; +import org.apache.hop.core.exception.HopTransformException; +import org.apache.hop.core.row.IRowMeta; +import org.apache.hop.core.row.value.ValueMetaBoolean; +import org.apache.hop.core.row.value.ValueMetaString; +import org.apache.hop.core.variables.IVariables; +import org.apache.hop.metadata.api.HopMetadataProperty; +import org.apache.hop.metadata.api.IHopMetadataProvider; +import org.apache.hop.pipeline.transform.BaseTransformMeta; +import org.apache.hop.pipeline.transform.TransformMeta; + +/** + * Plugin Catalog transform metadata. + * + * <p>Emits one row per plugin (or per plugin property) by reflecting over the live Hop {@link + * org.apache.hop.core.plugins.PluginRegistry}. Labels are resolved to human-readable text; output + * is never stale against the running Hop version or any installed third-party plugins. + */ +@Getter +@Setter +@Transform( + id = "PluginCatalog", + image = "plugincatalog.svg", + name = "i18n::PluginCatalog.Name", + description = "i18n::PluginCatalog.Description", + categoryDescription = "i18n:org.apache.hop.pipeline.transform:BaseTransform.Category.Input", + documentationUrl = + "https://hop.apache.org/manual/latest/pipeline/transforms/plugincatalog.html";, Review Comment: **[suggestion]** `documentationUrl` is the absolute `https://hop.apache.org/manual/latest/...` URL. Other transforms use a site-relative path such as `/pipeline/transforms/rowgenerator.html` so `Const.getDocUrl()` prefixes the versioned doc base. `getDocUrl` keeps `http*` URIs as-is, so Help from a 2.x install always opens latest, not that release’s manual. **Suggestion:** Set `documentationUrl = "/pipeline/transforms/plugincatalog.html"` to match `TransformPluginType.extractDocumentationUrl` / `Const.getDocUrl`. ########## plugins/transforms/plugincatalog/src/main/java/org/apache/hop/pipeline/transforms/plugincatalog/PluginCatalog.java: ########## @@ -0,0 +1,113 @@ +/* + * 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.hop.pipeline.transforms.plugincatalog; + +import java.util.List; +import org.apache.hop.core.exception.HopException; +import org.apache.hop.core.row.RowMeta; +import org.apache.hop.pipeline.Pipeline; +import org.apache.hop.pipeline.PipelineMeta; +import org.apache.hop.pipeline.transform.BaseTransform; +import org.apache.hop.pipeline.transform.TransformMeta; + +/** + * Plugin Catalog transform - a row-generating source that reflects the live Hop plugin registry + * into structured rows describing every transform, action and metadata type. + */ +public class PluginCatalog extends BaseTransform<PluginCatalogMeta, PluginCatalogData> { + + public PluginCatalog( + TransformMeta transformMeta, + PluginCatalogMeta meta, + PluginCatalogData data, + int copyNr, + PipelineMeta pipelineMeta, + Pipeline pipeline) { + super(transformMeta, meta, data, copyNr, pipelineMeta, pipeline); + } + + @Override + public boolean processRow() throws HopException { + // Source transform: generate all rows on the first call, then signal completion. Review Comment: **[suggestion]** Comments restate control flow or embed design history instead of a short WHY. Line 45 duplicates what `setOutputDone()` + `return false` already show. `PluginCatalogReader` lines 79–81 explain locale-stamping policy in-line. `PluginCatalogData.outputRowMeta` has a Javadoc that only names the field. **Suggestion:** Delete comments that narrate the code. Keep a one-liner only where the non-obvious constraint is not in the identifier (e.g. why empty-property plugins still emit a row). ########## plugins/transforms/plugincatalog/src/main/java/org/apache/hop/pipeline/transforms/plugincatalog/PluginCatalogReader.java: ########## @@ -0,0 +1,291 @@ +/* + * 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.hop.pipeline.transforms.plugincatalog; + +import java.lang.reflect.Field; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.function.BiConsumer; +import org.apache.hop.core.plugins.ActionPluginType; +import org.apache.hop.core.plugins.IPlugin; +import org.apache.hop.core.plugins.PluginRegistry; +import org.apache.hop.core.plugins.TransformPluginType; +import org.apache.hop.i18n.LanguageChoice; +import org.apache.hop.metadata.api.HopMetadataProperty; +import org.apache.hop.metadata.plugin.MetadataPluginType; + +/** + * Reads the live {@link PluginRegistry} and reflects over each plugin's metadata class to expose a + * structured, never-stale catalog of transforms, actions and metadata types. + * + * <p>Field extraction is split out as a pure static method so it can be unit-tested against fixture + * classes without bootstrapping a Hop plugin registry. + */ +public class PluginCatalogReader { + + /** Maximum nesting depth when descending into complex {@code @HopMetadataProperty} groups. */ + private static final int MAX_NESTING_DEPTH = 1; + + private final PluginRegistry registry; + + public PluginCatalogReader() { + this(PluginRegistry.getInstance()); + } + + public PluginCatalogReader(PluginRegistry registry) { + this.registry = registry; + } + + /** + * Enumerate the requested plugin families. + * + * @param warnings optional sink for non-fatal problems (e.g. a metadata class that fails to + * load); may be {@code null} + */ + public List<PluginRecord> readAll( + boolean includeTransforms, + boolean includeActions, + boolean includeMetadataTypes, + BiConsumer<String, Throwable> warnings) { + List<PluginRecord> out = new ArrayList<>(); + if (includeTransforms) { + addFamily(registry.getPlugins(TransformPluginType.class), "transform", out, warnings); + } + if (includeActions) { + addFamily(registry.getPlugins(ActionPluginType.class), "action", out, warnings); + } + if (includeMetadataTypes) { + addFamily(registry.getPlugins(MetadataPluginType.class), "metadata", out, warnings); + } + // name/description/category come back resolved against the running Hop locale, so stamp every + // record with that locale and keep the English aliases beside it. Without this a catalog built + // on a non-English installation is silently locale-specific. + String localeTag = currentLocaleTag(); + for (PluginRecord record : out) { + record.locale = localeTag; + } + return out; + } + + private void addFamily( + List<IPlugin> plugins, + String pluginType, + List<PluginRecord> out, + BiConsumer<String, Throwable> warnings) { + if (plugins == null) { + return; + } + for (IPlugin plugin : plugins) { + String id = firstId(plugin); + if (id.isEmpty()) { + // Skip anonymous/unregistered plugins rather than emit a junk row with no id. + if (warnings != null) { + warnings.accept("Skipping " + pluginType + " plugin with no id", null); + } + continue; + } + PluginRecord record = new PluginRecord(); + record.pluginId = id; + record.pluginType = pluginType; + record.name = clean(plugin.getName()); + record.description = clean(plugin.getDescription()); + record.category = clean(plugin.getCategory()); + record.keywords = joinKeywords(plugin.getKeywords()); + record.englishAliases = joinKeywords(plugin.getEnglishKeywords()); + String className = resolveClassName(plugin); + record.className = className == null ? "" : className; + if (className != null) { + try { + ClassLoader classLoader = registry.getClassLoader(plugin); + Class<?> clazz = classLoader.loadClass(className); + record.properties = extractProperties(clazz); + } catch (Throwable e) { Review Comment: **[suggestion]** Property reflection catches `Throwable`. That swallows `VirtualMachineError` / `ThreadDeath` and continues cataloguing. Class-load failures for a plugin should be `Exception` (and perhaps `LinkageError` / `ClassNotFoundException` if you want those isolated) so fatal JVM errors still abort. **Suggestion:** Catch `Exception` (or `ClassNotFoundException | NoClassDefFoundError | HopPluginException`). Keep the warnings sink and still emit the plugin row without properties. -- 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]
