prankstrisse commented on code in PR #11499:
URL: https://github.com/apache/nifi/pull/11499#discussion_r4094793077
##########
nifi-extension-bundles/nifi-protobuf-bundle/nifi-protobuf-services/src/main/java/org/apache/nifi/services/protobuf/ProtobufSchemaValidator.java:
##########
@@ -17,47 +17,45 @@
package org.apache.nifi.services.protobuf;
import org.apache.nifi.schemaregistry.services.SchemaDefinition;
-import org.apache.nifi.serialization.record.SchemaIdentifier;
+
+import java.util.Map;
/**
- * Validates Protocol Buffer SchemaDefinition objects and schema identifiers.
+ * Validates the schema references of Protocol Buffer SchemaDefinition objects.
*/
final class ProtobufSchemaValidator {
+ private static final String PROTO_EXTENSION = ".proto";
+
private ProtobufSchemaValidator() {
}
/**
- * Validates that all SchemaDefinition identifiers end with .proto
extension.
- * Performs recursive validation on all referenced schemas.
+ * Validates that every schema reference, at any depth, is keyed by a path
ending in the .proto extension.
+ * <p>
+ * A reference is keyed by the path used in the import statement of the
referencing schema, for example
+ * {@code airlines/ph/cdm/shared.proto}, and the referenced schema is
written to exactly that path so the import
+ * resolves. The extension is required because the compiler only discovers
files named {@code *.proto}; without it
+ * the schema would fail to compile later with an unresolved import that
does not indicate the cause.
+ * <p>
+ * The identifier of a referenced schema is deliberately not validated. It
carries the subject the schema is
+ * registered under, which is unrelated to the import path and
legitimately has no .proto suffix. Under the
+ * Confluent RecordNameStrategy, for instance, a subject is a fully
qualified record name.
Review Comment:
thanks, done
##########
nifi-extension-bundles/nifi-protobuf-bundle/nifi-protobuf-services/src/main/java/org/apache/nifi/services/protobuf/ProtobufSchemaCompiler.java:
##########
@@ -182,23 +183,40 @@ private void safeDeleteDirectory(final Path directory) {
}
/**
- * Writes a schema definition to the temporary directory structure.
- * If package name is present, creates the appropriate directory structure.
+ * Writes a schema definition to the temporary directory structure using
the name of its identifier.
*
* @param tempDir the temporary directory root
* @param schemaDefinition the schema definition to write
* @throws IOException if unable to create directories or write files
*/
private void writeSchemaToTempDirectory(final Path tempDir, final
SchemaDefinition schemaDefinition) throws IOException {
logger.debug("Writing schema definition to temporary directory.
Identifier: {}", schemaDefinition.getIdentifier());
+ writeSchemaFile(tempDir, generateSchemaFileName(schemaDefinition),
schemaDefinition.getText());
+ }
+
+ /**
+ * Writes schema text to a path relative to the temporary directory,
creating any parent directories.
+ * Import paths may contain directories, such as {@code
airlines/ph/cdm/shared.proto}, so the enclosing
+ * directory structure has to exist before the file is written.
Review Comment:
thanks, done
--
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]
