[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/nifi-registry/pull/10


---

[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[kevdoran]

Github user kevdoran commented on a diff in the pull request:

https://github.com/apache/nifi-registry/pull/10#discussion_r140088139
  
--- Diff: 
nifi-registry-framework/src/main/java/org/apache/nifi/registry/db/DataSourceFactory.java
 ---
@@ -0,0 +1,85 @@
+/*
+ * 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.registry.db;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.registry.properties.NiFiRegistryProperties;
+import org.h2.jdbcx.JdbcConnectionPool;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Primary;
+
+import javax.sql.DataSource;
+import java.io.File;
+
+/**
+ * Overriding Spring Boot's normal automatic creation of a DataSource in 
order to use the properties
+ * from NiFiRegistryProperties rather than the standard 
application.properties/yaml.
+ */
+@Configuration
+public class DataSourceFactory {
+
+private static final String DB_USERNAME_PASSWORD = "nifireg";
--- End diff --

Yep, that works for me. Just wanted to verify it.


---

[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[bbende]

Github user bbende commented on a diff in the pull request:

https://github.com/apache/nifi-registry/pull/10#discussion_r140073712
  
--- Diff: 
nifi-registry-web-api/src/main/java/org/apache/nifi/registry/NiFiRegistryApiApplication.java
 ---
@@ -14,27 +14,41 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.nifi.registry.web;
+package org.apache.nifi.registry;
 
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.boot.builder.SpringApplicationBuilder;
 import org.springframework.boot.web.support.SpringBootServletInitializer;
-import org.springframework.context.annotation.ComponentScan;
+
+import java.util.Properties;
 
 /**
  * Main class for starting the NiFi Registry Web API as a Spring Boot 
application.
  *
- * By default, Spring Boot will only scan in the package this class is 
located in, so we set
- * @ComponentScan to the common parent package to find beans in other 
packages.
+ * This class is purposely in the org.apache.nifi.registry package since 
that is the common base
+ * package across other modules. This is done because spring-boot will use 
the package of this
+ * class to automatically scan for beans/config/entities/etc. and would 
otherwise require
+ * configuring custom packages to scan in several different places.
  */
 @SpringBootApplication
-@ComponentScan("org.apache.nifi.registry")
 public class NiFiRegistryApiApplication extends 
SpringBootServletInitializer {
 
+public static final String NIFI_REGISTRY_PROPERTIES_ATTRIBUTE = 
"nifi-registry.properties";
+
 @Override
 protected SpringApplicationBuilder configure(SpringApplicationBuilder 
application) {
-return application.sources(NiFiRegistryApiApplication.class);
+final Properties fixedProps = new Properties();
+fixedProps.setProperty("spring.jpa.hibernate.ddl-auto", "none");
+
fixedProps.setProperty("spring.jpa.hibernate.naming.physical-strategy", 
"org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl");
+
+// turn on to debug
+fixedProps.setProperty("spring.jpa.show-sql", "true");
+fixedProps.setProperty("spring.h2.console.enabled", "true");
--- End diff --

The way I thought this was working was that even with those set to true, 
the logging didn't take place unless you also changed the levels in logback.xml 
as mentioned in your next comment, but apparently that is not true since you 
saw the output in bootstrap.log :)  


---

[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[bbende]

Github user bbende commented on a diff in the pull request:

https://github.com/apache/nifi-registry/pull/10#discussion_r140073748
  
--- Diff: nifi-registry-resources/src/main/resources/conf/logback.xml ---
@@ -68,6 +68,11 @@
 
 
 
+
+
+
+
+
--- End diff --

Good find, will investigate


---

[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[bbende]

Github user bbende commented on a diff in the pull request:

https://github.com/apache/nifi-registry/pull/10#discussion_r140073136
  
--- Diff: 
nifi-registry-framework/src/main/java/org/apache/nifi/registry/db/DataSourceFactory.java
 ---
@@ -0,0 +1,85 @@
+/*
+ * 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.registry.db;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.registry.properties.NiFiRegistryProperties;
+import org.h2.jdbcx.JdbcConnectionPool;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Primary;
+
+import javax.sql.DataSource;
+import java.io.File;
+
+/**
+ * Overriding Spring Boot's normal automatic creation of a DataSource in 
order to use the properties
+ * from NiFiRegistryProperties rather than the standard 
application.properties/yaml.
+ */
+@Configuration
+public class DataSourceFactory {
+
+private static final String DB_USERNAME_PASSWORD = "nifireg";
--- End diff --

This is the same approach taken in NiFi so I went with it, the thinking is 
that the database can never be run in server mode and exposed outside of the 
file on the server, so at that point you are down to file permissions.


---

[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[bbende]

Github user bbende commented on a diff in the pull request:

https://github.com/apache/nifi-registry/pull/10#discussion_r140072867
  
--- Diff: 
nifi-registry-framework/src/main/java/org/apache/nifi/registry/db/repository/FlowRepository.java
 ---
@@ -0,0 +1,34 @@
+/*
+ * 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.registry.db.repository;
+
+import org.apache.nifi.registry.db.entity.FlowEntity;
+import org.springframework.data.jpa.repository.Query;
+import org.springframework.data.repository.PagingAndSortingRepository;
+import org.springframework.data.repository.query.Param;
+
+import java.util.List;
+
+/**
+ * Spring Data Repository for FlowEntity.
+ */
+public interface FlowRepository extends 
PagingAndSortingRepository {
+
+@Query("SELECT f FROM FlowEntity f WHERE LOWER(f.name) = LOWER(:name)")
+List findByName(@Param("name") String name);
--- End diff --

Will update as above


---

[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[bbende]

Github user bbende commented on a diff in the pull request:

https://github.com/apache/nifi-registry/pull/10#discussion_r140072829
  
--- Diff: 
nifi-registry-framework/src/main/java/org/apache/nifi/registry/db/repository/BucketRepository.java
 ---
@@ -14,38 +14,21 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.nifi.registry.metadata;
+package org.apache.nifi.registry.db.repository;
 
-import java.util.Set;
+import org.apache.nifi.registry.db.entity.BucketEntity;
+import org.springframework.data.jpa.repository.Query;
+import org.springframework.data.repository.PagingAndSortingRepository;
+import org.springframework.data.repository.query.Param;
+
+import java.util.List;
 
 /**
- * The metadata for a bucket, along with the metadata about any objects 
stored in the bucket, such as flows.
+ * Spring Data Repository for BucketEntity.
  */
-public interface BucketMetadata {
-
-/**
- * @return the identifier of this bucket
- */
-String getIdentifier();
-
-/**
- * @return the name of this bucket
- */
-String getName();
-
-/**
- * @return the timestamp of when this bucket was created
- */
-long getCreatedTimestamp();
-
-/**
- * @return the description of this bucket
- */
-String getDescription();
+public interface BucketRepository extends 
PagingAndSortingRepository {
 
-/**
- * @return the metadata about the flows that are part of this bucket
- */
-Set getFlowMetadata();
+@Query("SELECT b FROM BucketEntity b WHERE LOWER(b.name) = 
LOWER(:name)")
+List findByName(@Param("name") String name);
--- End diff --

Great idea, I'll update to use that approach


---

[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[kevdoran]

Github user kevdoran commented on a diff in the pull request:

https://github.com/apache/nifi-registry/pull/10#discussion_r140038396
  
--- Diff: nifi-registry-resources/src/main/resources/conf/logback.xml ---
@@ -68,6 +68,11 @@
 
 
 
+
+
+
+
+
--- End diff --

When I ran the app. I noticed Hibernate log output going to bootstrap.log 
instead of app.log. Is that intended?

Example output in bootstrap.log:

```
2017-09-20 13:27:34,585 INFO [NiFi logging handler] 
org.apache.nifi.registry.StdOut Hibernate: select bucketenti0_.id as id1_0_0_, 
bucketenti0_.created as created2_0_0_, bucketenti0_.description as 
descript3_0_0_, bucketenti0_.name as name4_0_0_ from BUCKET bucketenti0_ where 
bucketenti0_.id=?
```


---

[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[kevdoran]

Github user kevdoran commented on a diff in the pull request:

https://github.com/apache/nifi-registry/pull/10#discussion_r140024359
  
--- Diff: 
nifi-registry-framework/src/main/java/org/apache/nifi/registry/db/repository/BucketRepository.java
 ---
@@ -14,38 +14,21 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.nifi.registry.metadata;
+package org.apache.nifi.registry.db.repository;
 
-import java.util.Set;
+import org.apache.nifi.registry.db.entity.BucketEntity;
+import org.springframework.data.jpa.repository.Query;
+import org.springframework.data.repository.PagingAndSortingRepository;
+import org.springframework.data.repository.query.Param;
+
+import java.util.List;
 
 /**
- * The metadata for a bucket, along with the metadata about any objects 
stored in the bucket, such as flows.
+ * Spring Data Repository for BucketEntity.
  */
-public interface BucketMetadata {
-
-/**
- * @return the identifier of this bucket
- */
-String getIdentifier();
-
-/**
- * @return the name of this bucket
- */
-String getName();
-
-/**
- * @return the timestamp of when this bucket was created
- */
-long getCreatedTimestamp();
-
-/**
- * @return the description of this bucket
- */
-String getDescription();
+public interface BucketRepository extends 
PagingAndSortingRepository {
 
-/**
- * @return the metadata about the flows that are part of this bucket
- */
-Set getFlowMetadata();
+@Query("SELECT b FROM BucketEntity b WHERE LOWER(b.name) = 
LOWER(:name)")
+List findByName(@Param("name") String name);
--- End diff --

This can be rewritten without the `@Query` annotation just by renaming the 
method to `findByNameIgnoreCase`, and then the generated spring-data-jpa code 
will produce the query you want.

The advantage is not having to write and maintain the SQL in the `@Query` 
annotation, which prevents vendor-specific SQL syntax from sneaking in over 
time. It might not be possible to avoid the `@Query` annotation for every 
repository method, for example if you need a specific query optimization that 
spring-data-jpa cannot generate. I still think limiting explicit SQL is good 
practice in order to keep open the possibility that someone might want to 
modify registry to use an alternative to H2.


---

[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[kevdoran]

Github user kevdoran commented on a diff in the pull request:

https://github.com/apache/nifi-registry/pull/10#discussion_r140035681
  
--- Diff: 
nifi-registry-web-api/src/main/java/org/apache/nifi/registry/NiFiRegistryApiApplication.java
 ---
@@ -14,27 +14,41 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.nifi.registry.web;
+package org.apache.nifi.registry;
 
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.boot.builder.SpringApplicationBuilder;
 import org.springframework.boot.web.support.SpringBootServletInitializer;
-import org.springframework.context.annotation.ComponentScan;
+
+import java.util.Properties;
 
 /**
  * Main class for starting the NiFi Registry Web API as a Spring Boot 
application.
  *
- * By default, Spring Boot will only scan in the package this class is 
located in, so we set
- * @ComponentScan to the common parent package to find beans in other 
packages.
+ * This class is purposely in the org.apache.nifi.registry package since 
that is the common base
+ * package across other modules. This is done because spring-boot will use 
the package of this
+ * class to automatically scan for beans/config/entities/etc. and would 
otherwise require
+ * configuring custom packages to scan in several different places.
  */
 @SpringBootApplication
-@ComponentScan("org.apache.nifi.registry")
 public class NiFiRegistryApiApplication extends 
SpringBootServletInitializer {
 
+public static final String NIFI_REGISTRY_PROPERTIES_ATTRIBUTE = 
"nifi-registry.properties";
+
 @Override
 protected SpringApplicationBuilder configure(SpringApplicationBuilder 
application) {
-return application.sources(NiFiRegistryApiApplication.class);
+final Properties fixedProps = new Properties();
+fixedProps.setProperty("spring.jpa.hibernate.ddl-auto", "none");
+
fixedProps.setProperty("spring.jpa.hibernate.naming.physical-strategy", 
"org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl");
+
+// turn on to debug
+fixedProps.setProperty("spring.jpa.show-sql", "true");
+fixedProps.setProperty("spring.h2.console.enabled", "true");
--- End diff --

Did you intend to commit these debug props or was that included 
accidentally?


---

[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[kevdoran]

Github user kevdoran commented on a diff in the pull request:

https://github.com/apache/nifi-registry/pull/10#discussion_r140031965
  
--- Diff: 
nifi-registry-framework/src/main/java/org/apache/nifi/registry/db/DataSourceFactory.java
 ---
@@ -0,0 +1,85 @@
+/*
+ * 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.registry.db;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.registry.properties.NiFiRegistryProperties;
+import org.h2.jdbcx.JdbcConnectionPool;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Primary;
+
+import javax.sql.DataSource;
+import java.io.File;
+
+/**
+ * Overriding Spring Boot's normal automatic creation of a DataSource in 
order to use the properties
+ * from NiFiRegistryProperties rather than the standard 
application.properties/yaml.
+ */
+@Configuration
+public class DataSourceFactory {
+
+private static final String DB_USERNAME_PASSWORD = "nifireg";
--- End diff --

Is there any downside to having the username and password hardcoded? I 
suppose if an admin needs to secure the DB file they can use the OS files 
system permissions to prevent file access. Is there any other use case that a 
configurable user/pass would facilitate?


---

[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[kevdoran]

Github user kevdoran commented on a diff in the pull request:

https://github.com/apache/nifi-registry/pull/10#discussion_r140031392
  
--- Diff: 
nifi-registry-framework/src/main/java/org/apache/nifi/registry/db/repository/FlowRepository.java
 ---
@@ -0,0 +1,34 @@
+/*
+ * 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.registry.db.repository;
+
+import org.apache.nifi.registry.db.entity.FlowEntity;
+import org.springframework.data.jpa.repository.Query;
+import org.springframework.data.repository.PagingAndSortingRepository;
+import org.springframework.data.repository.query.Param;
+
+import java.util.List;
+
+/**
+ * Spring Data Repository for FlowEntity.
+ */
+public interface FlowRepository extends 
PagingAndSortingRepository {
+
+@Query("SELECT f FROM FlowEntity f WHERE LOWER(f.name) = LOWER(:name)")
+List findByName(@Param("name") String name);
--- End diff --

Same as above regarding `@Query` annotation -> `findByNameIgnoreCase`.


---

[GitHub] nifi-registry pull request #10: NIFIREG-18 Refactor MetadataProvider to use ...

[bbende]

GitHub user bbende opened a pull request:

https://github.com/apache/nifi-registry/pull/10

NIFIREG-18 Refactor MetadataProvider to use an embedded database

- Initial plumbing for H2 database
- Setup Flyway with initial migration to define tables
- Setup entity classes with repositories
- Setup unit testing for repositories
- Removed existing MetadataProvider concept
- Removed provider impl module and moved remaining pieces into framework
- Added MetadataService with DatabaseMetadataService implementation
- Refactored RegistryService to use MetadataService
- Introduced verbose flag on some end-points to control loading nested 
objects
- Added ability to pass down paging/sorting params
- Added endpoints for available fields
- Adding ItemResource and ability to retrieve all items, or items by bucket
- Changing from Set to List on retrieval methods

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

$ git pull https://github.com/bbende/nifi-registry NIFIREG-18

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

https://github.com/apache/nifi-registry/pull/10.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 #10


commit 8f394bd73bf9a077b5b41ce897015daadac7
Author: Bryan Bende 
Date:   2017-09-19T14:01:24Z

NIFIREG-18 Initial plumbling for H2 database
- Setup Flyway with initial migration to define tables
- Setup entity classes with repositories
- Setup unit testing for repositories
- Removed existing MetadataProvider concept
- Removed provider impl module and moved remaining pieces into framework
- Added MetadataService with DatabaseMetadataService implementation
- Refactored RegistryService to use MetadataService
- Introduced verbose flag on some end-points to control loading nested 
objects
- Added ability to pass down paging/sorting params
- Added endpoints for available fields
- Adding ItemResource and ability to retrieve all items, or items by bucket
- Changing from Set to List on retrieval methods




---