You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by markap14 <gi...@git.apache.org> on 2018/11/30 14:53:07 UTC

[GitHub] nifi-maven pull request #7: NIFI-5859: Build NAR Extension Definitions/docs ...

GitHub user markap14 opened a pull request:

    https://github.com/apache/nifi-maven/pull/7

    NIFI-5859: Build NAR Extension Definitions/docs at build time

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/markap14/nifi-maven NIFI-5859

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi-maven/pull/7.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #7
    
----
commit 7cecaa9a841af8c5729fd5b135db400318245bf3
Author: Mark Payne <ma...@...>
Date:   2018-11-14T20:09:25Z

    NIFI-5859: Build NAR Extension Definitions/docs at build time

----


---

[GitHub] nifi-maven pull request #7: NIFI-5859: Build NAR Extension Definitions/docs ...

Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on a diff in the pull request:

    https://github.com/apache/nifi-maven/pull/7#discussion_r240314647
  
    --- Diff: src/main/java/org/apache/nifi/XmlDefinitionWriter.java ---
    @@ -0,0 +1,165 @@
    +/*
    + * 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.nifi;
    +
    +import org.apache.nifi.extension.definition.ExtensionDefinition;
    +import org.apache.nifi.extension.definition.ExtensionType;
    +import org.apache.nifi.extension.definition.Restriction;
    +import org.apache.nifi.extension.definition.Restrictions;
    +import org.apache.nifi.extension.definition.ServiceAPIDefinition;
    +
    +import javax.xml.stream.XMLOutputFactory;
    +import javax.xml.stream.XMLStreamException;
    +import javax.xml.stream.XMLStreamWriter;
    +import java.io.File;
    +import java.io.FileOutputStream;
    +import java.io.IOException;
    +import java.io.OutputStream;
    +import java.util.Collection;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Objects;
    +import java.util.Set;
    +import java.util.stream.Collectors;
    +
    +public class XmlDefinitionWriter {
    --- End diff --
    
    Can this be removed now that we are using the classes from nifi-api? 


---

[GitHub] nifi-maven pull request #7: NIFI-5859: Build NAR Extension Definitions/docs ...

Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on a diff in the pull request:

    https://github.com/apache/nifi-maven/pull/7#discussion_r240310495
  
    --- Diff: src/main/java/org/apache/nifi/NarMojo.java ---
    @@ -426,12 +479,260 @@
         protected boolean cloneDuringInstanceClassLoading;
     
     
    +    /**
    +     * The {@link RepositorySystemSession} used for obtaining the local and remote artifact repositories.
    +     */
    +    @Parameter(defaultValue = "${repositorySystemSession}", readonly = true)
    +    private RepositorySystemSession repoSession;
    +
    +
    +    /**
    +     * The {@link ProjectBuilder} used to generate the {@link MavenProject} for the nar artifact the dependency tree is being generated for.
    +     */
    +    @Component
    +    private ProjectBuilder projectBuilder;
    +
    +
    +
         @Override
    -    public void execute() throws MojoExecutionException, MojoFailureException {
    +    public void execute() throws MojoExecutionException {
             copyDependencies();
    +
    +        try {
    +            generateDocumentation();
    +        } catch (final Throwable t) { // Catch Throwable in case a linkage error such as NoClassDefFoundError occurs
    +            getLog().warn("Could not generate extensions' documentation", t);
    +        }
    +
             makeNar();
         }
     
    +    private File getExtensionsDocumentationFile() {
    +        final File directory = new File(projectBuildDirectory, "META-INF/docs");
    +        return new File(directory, "extension-docs.xml");
    +    }
    +
    +    private void generateDocumentation() throws MojoExecutionException {
    +        getLog().info("Generating documentation for NiFi extensions in the NAR...");
    +
    +        // Create the ClassLoader for the NAR
    +        final ExtensionClassLoaderFactory classLoaderFactory = createClassLoaderFactory();
    +
    +        final ExtensionClassLoader extensionClassLoader;
    +        try {
    +            extensionClassLoader = classLoaderFactory.createExtensionClassLoader();
    +        } catch (final Exception e) {
    +            if (getLog().isDebugEnabled()) {
    +                getLog().debug("Unable to create a ClassLoader for documenting extensions. If this NAR contains any NiFi Extensions, those extensions will not be documented.", e);
    +            } else {
    +                getLog().warn("Unable to create a ClassLoader for documenting extensions. If this NAR contains any NiFi Extensions, those extensions will not be documented. " +
    +                    "Enable mvn DEBUG output for more information (mvn -X).");
    +            }
    +
    +            return;
    +        }
    +
    +
    +        final File docsFile = getExtensionsDocumentationFile();
    +        createDirectory(docsFile.getParentFile());
    +
    +        final File additionalDetailsDir = new File(docsFile.getParentFile(), "additional-details");
    +        createDirectory(additionalDetailsDir);
    +
    +        try (final OutputStream out = new FileOutputStream(docsFile)) {
    +
    +            final XMLStreamWriter xmlWriter = XMLOutputFactory.newInstance().createXMLStreamWriter(out, "UTF-8");
    +            try {
    +                xmlWriter.writeStartElement("extensions");
    +
    +                final String nifiApiVersion = extensionClassLoader.getNiFiApiVersion();
    +                xmlWriter.writeStartElement("nifiApiVersion");
    +                xmlWriter.writeCharacters(nifiApiVersion);
    +                xmlWriter.writeEndElement();
    +
    +                final Class<?> docWriterClass;
    +                try {
    +                    docWriterClass = Class.forName(DOCUMENTATION_WRITER_CLASS_NAME, false, extensionClassLoader);
    --- End diff --
    
    Should we do this before entering the try block on line 542 so that if the doc writer class can't be found we won't write anything to the docs file?


---

[GitHub] nifi-maven pull request #7: NIFI-5859: Build NAR Extension Definitions/docs ...

Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on a diff in the pull request:

    https://github.com/apache/nifi-maven/pull/7#discussion_r240308506
  
    --- Diff: src/main/java/org/apache/nifi/NarMojo.java ---
    @@ -426,12 +479,260 @@
         protected boolean cloneDuringInstanceClassLoading;
     
     
    +    /**
    +     * The {@link RepositorySystemSession} used for obtaining the local and remote artifact repositories.
    +     */
    +    @Parameter(defaultValue = "${repositorySystemSession}", readonly = true)
    +    private RepositorySystemSession repoSession;
    +
    +
    +    /**
    +     * The {@link ProjectBuilder} used to generate the {@link MavenProject} for the nar artifact the dependency tree is being generated for.
    +     */
    +    @Component
    +    private ProjectBuilder projectBuilder;
    +
    +
    +
         @Override
    -    public void execute() throws MojoExecutionException, MojoFailureException {
    +    public void execute() throws MojoExecutionException {
             copyDependencies();
    +
    +        try {
    +            generateDocumentation();
    --- End diff --
    
    Could we add an optional property like "enforceDocGeneration" (or some better name) that when true we would let the exception be thrown here to fail the build?
    
    The use-case would be to know that when doing a release we have successfully generated the docs if the build passed. Right now it can silently fail with a warning and you'd have to manually inspect every NAR to really know.


---

[GitHub] nifi-maven pull request #7: NIFI-5859: Build NAR Extension Definitions/docs ...

Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on a diff in the pull request:

    https://github.com/apache/nifi-maven/pull/7#discussion_r240374218
  
    --- Diff: src/main/java/org/apache/nifi/extension/definition/extraction/ExtensionDefinitionFactory.java ---
    @@ -0,0 +1,247 @@
    +/*
    + * 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.nifi.extension.definition.extraction;
    +
    +import org.apache.maven.artifact.Artifact;
    +import org.apache.maven.plugin.MojoExecutionException;
    +import org.apache.nifi.extension.definition.ExtensionDefinition;
    +import org.apache.nifi.extension.definition.ExtensionType;
    +import org.apache.nifi.extension.definition.Restriction;
    +import org.apache.nifi.extension.definition.Restrictions;
    +import org.apache.nifi.extension.definition.ServiceAPIDefinition;
    +
    +import java.io.BufferedReader;
    +import java.io.IOException;
    +import java.io.InputStream;
    +import java.io.InputStreamReader;
    +import java.io.Reader;
    +import java.lang.annotation.Annotation;
    +import java.lang.reflect.InvocationTargetException;
    +import java.lang.reflect.Method;
    +import java.net.URL;
    +import java.util.Collections;
    +import java.util.Enumeration;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.stream.Collectors;
    +import java.util.stream.Stream;
    +
    +public class ExtensionDefinitionFactory {
    +    private static final String SERVICES_DIRECTORY = "META-INF/services/";
    +
    +    private static final Map<ExtensionType, String> INTERFACE_NAMES = new HashMap<>();
    +    static {
    +        INTERFACE_NAMES.put(ExtensionType.PROCESSOR, "org.apache.nifi.processor.Processor");
    +        INTERFACE_NAMES.put(ExtensionType.CONTROLLER_SERVICE, "org.apache.nifi.controller.ControllerService");
    +        INTERFACE_NAMES.put(ExtensionType.REPORTING_TASK, "org.apache.nifi.reporting.ReportingTask");
    +    }
    +
    +    private final ClassLoader extensionClassLoader;
    +
    +    public ExtensionDefinitionFactory(final ClassLoader classLoader) {
    +        this.extensionClassLoader = classLoader;
    +    }
    +
    +    public Set<ExtensionDefinition> discoverExtensions(final ExtensionType extensionType) throws IOException {
    +        final String interfaceName = INTERFACE_NAMES.get(extensionType);
    +        final Set<String> classNames = discoverClassNames(interfaceName);
    +
    +        if (classNames.isEmpty()) {
    +            return Collections.emptySet();
    +        }
    +
    +        final Set<ExtensionDefinition> definitions = new HashSet<>();
    +        for (final String className : classNames) {
    +            try {
    +                definitions.add(createExtensionDefinition(extensionType, className));
    +            } catch (final Exception e) {
    +                throw new IOException("Failed to create Extension Definition for " + extensionType + " " + className, e);
    +            }
    +        }
    +
    +        return definitions;
    +    }
    +
    +    private ExtensionDefinition createExtensionDefinition(final ExtensionType extensionType, final String className) throws ClassNotFoundException,
    +                IllegalAccessException, NoSuchMethodException, InvocationTargetException {
    +
    +        final Class<?> extensionClass = Class.forName(className, false, extensionClassLoader);
    +
    +        final String capabilityDescription = getCapabilityDescription(extensionClass);
    +        final Set<String> tags = getTags(extensionClass);
    +        final Restrictions restrictions = getRestrictions(extensionClass);
    +        final Set<ServiceAPIDefinition> serviceApis = getProvidedServiceAPIs(extensionType, extensionClass);
    +
    +        return new StandardExtensionDefinition(extensionType, className, capabilityDescription, tags, restrictions, serviceApis);
    +    }
    +
    +    private Set<ServiceAPIDefinition> getProvidedServiceAPIs(final ExtensionType extensionType, final Class<?> extensionClass) throws ClassNotFoundException {
    +        if (extensionType != ExtensionType.CONTROLLER_SERVICE) {
    +            return Collections.emptySet();
    +        }
    +
    +        final Set<ServiceAPIDefinition> serviceApis = new HashSet<>();
    +        final Class<?> controllerServiceClass = Class.forName("org.apache.nifi.controller.ControllerService", false, extensionClassLoader);
    +
    +        for (final Class<?> implementedInterface : extensionClass.getInterfaces()) {
    +            if (controllerServiceClass.isAssignableFrom(implementedInterface)) {
    +                final ClassLoader interfaceClassLoader = implementedInterface.getClassLoader();
    +                if (interfaceClassLoader instanceof ExtensionClassLoader) {
    +                    final Artifact interfaceNarArtifact = ((ExtensionClassLoader) interfaceClassLoader).getNarArtifact();
    +
    +                    final ServiceAPIDefinition serviceDefinition = new StandardServiceAPIDefinition(implementedInterface.getName(),
    +                        interfaceNarArtifact.getGroupId(), interfaceNarArtifact.getArtifactId(), interfaceNarArtifact.getVersion());
    +
    +                    serviceApis.add(serviceDefinition);
    +                }
    +            }
    +        }
    +
    +        return serviceApis;
    +    }
    +
    +    private Restrictions getRestrictions(final Class<?> extensionClass) throws ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException {
    --- End diff --
    
    I'm not sure if it's worth removing since you already implemented these, but from tracing through the code, I think if we removed the XML/Property writers that are not being used, then we also don't need the tags, restrictions, and capability description in here and in the ExtensionDefinition interface, since the code in nifi-api would be obtaining all of that, right?
    
    Can then also remove the Restrictions and Restriction interfaces and their implementations.


---

[GitHub] nifi-maven pull request #7: NIFI-5859: Build NAR Extension Definitions/docs ...

Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on a diff in the pull request:

    https://github.com/apache/nifi-maven/pull/7#discussion_r240314701
  
    --- Diff: src/main/java/org/apache/nifi/PropertiesDefinitionWriter.java ---
    @@ -0,0 +1,75 @@
    +/*
    + * 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.nifi;
    +
    +import org.apache.nifi.extension.definition.ExtensionDefinition;
    +import org.apache.nifi.extension.definition.Restriction;
    +import org.apache.nifi.extension.definition.Restrictions;
    +import org.apache.nifi.extension.definition.ServiceAPIDefinition;
    +
    +import java.io.File;
    +import java.io.FileOutputStream;
    +import java.io.IOException;
    +import java.io.OutputStream;
    +import java.util.Objects;
    +import java.util.Properties;
    +
    +public class PropertiesDefinitionWriter {
    --- End diff --
    
    Can this be removed now that we are using the classes from nifi-api? 


---