You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/04/14 13:07:27 UTC
[GitHub] [ignite-extensions] SammyVimes opened a new pull request #55: IGNITE-13169 Remove Ignite bean name requirement for Spring Data Repository
SammyVimes opened a new pull request #55:
URL: https://github.com/apache/ignite-extensions/pull/55
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-extensions] sashapolo commented on pull request #55: IGNITE-13169 Remove Ignite bean name requirement for Spring Data Repository
Posted by GitBox <gi...@apache.org>.
sashapolo commented on pull request #55:
URL: https://github.com/apache/ignite-extensions/pull/55#issuecomment-820485061
All comments are applicable for all three copy-pasted modules
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-extensions] asfgit closed pull request #55: IGNITE-13169 Remove Ignite bean name requirement for Spring Data Repository
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #55:
URL: https://github.com/apache/ignite-extensions/pull/55
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-extensions] tkalkirill commented on a change in pull request #55: IGNITE-13169 Remove Ignite bean name requirement for Spring Data Repository
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on a change in pull request #55:
URL: https://github.com/apache/ignite-extensions/pull/55#discussion_r614084200
##########
File path: modules/spring-data-2.0-ext/src/main/java/org/apache/ignite/springdata20/repository/support/IgniteProxyFactory.java
##########
@@ -137,4 +139,29 @@ private IgniteProxy createIgniteProxy(Class<?> repoInterface) {
private String evaluateExpression(String spelExpression) {
return (String)expressionResolver.evaluate(spelExpression, beanExpressionCtx);
}
+
+ /**
+ * Helper interface that wraps getBean method.
Review comment:
Add getBean as @link
##########
File path: modules/spring-data-2.0-ext/src/main/java/org/apache/ignite/springdata20/repository/support/IgniteProxyFactory.java
##########
@@ -137,4 +139,29 @@ private IgniteProxy createIgniteProxy(Class<?> repoInterface) {
private String evaluateExpression(String spelExpression) {
return (String)expressionResolver.evaluate(spelExpression, beanExpressionCtx);
}
+
+ /**
+ * Helper interface that wraps getBean method.
+ */
+ @FunctionalInterface
+ private interface BeanFinder {
+ /**
+ * Get bean.
+ * @return Bean or null if {@link BeansException} was thrown.
+ */
+ default Object getBean() {
Review comment:
@nullable forget
##########
File path: modules/spring-data-2.0-ext/src/main/java/org/apache/ignite/springdata20/repository/support/IgniteProxyFactory.java
##########
@@ -137,4 +139,29 @@ private IgniteProxy createIgniteProxy(Class<?> repoInterface) {
private String evaluateExpression(String spelExpression) {
return (String)expressionResolver.evaluate(spelExpression, beanExpressionCtx);
}
+
+ /**
+ * Helper interface that wraps getBean method.
+ */
+ @FunctionalInterface
+ private interface BeanFinder {
+ /**
+ * Get bean.
+ * @return Bean or null if {@link BeansException} was thrown.
+ */
+ default Object getBean() {
+ try {
+ return get();
+ } catch (BeansException ex) {
+ return null;
+ }
+ }
+
+ /**
+ * Get bean.
Review comment:
Maybe: Getting a bean.
New line after description.
##########
File path: modules/spring-data-2.0-ext/src/main/java/org/apache/ignite/springdata20/repository/support/IgniteProxyFactory.java
##########
@@ -137,4 +139,29 @@ private IgniteProxy createIgniteProxy(Class<?> repoInterface) {
private String evaluateExpression(String spelExpression) {
return (String)expressionResolver.evaluate(spelExpression, beanExpressionCtx);
}
+
+ /**
+ * Helper interface that wraps getBean method.
+ */
+ @FunctionalInterface
+ private interface BeanFinder {
+ /**
+ * Get bean.
Review comment:
Maybe: Getting a bean.
New line after description.
##########
File path: modules/spring-data-2.0-ext/src/test/java/org/apache/ignite/springdata/IgniteSpringDataConnectionConfigurationTest.java
##########
@@ -75,6 +76,30 @@ public void testRepositoryWithClientConfiguration() {
checkRepositoryConfiguration(ClientConfigurationApplication.class, IgniteClientConfigRepository.class);
}
+ /** Tests repository configuration in case {@link Ignite} with non default is used to access the Ignite cluster. */
+ @Test
+ public void testRepositoryWithoutIgniteInstanceParameter() {
+ checkRepositoryConfiguration(DefaultIgniteBeanApplication.class, IgniteRepositoryWithoutExplicitIgnite.class);
+ }
+
+ /** Tests repository configuration in case {@link IgniteClient} with non default name is used to access the Ignite cluster. */
Review comment:
Why 1 line?
##########
File path: modules/spring-data-2.0-ext/src/test/java/org/apache/ignite/springdata/IgniteSpringDataConnectionConfigurationTest.java
##########
@@ -75,6 +76,30 @@ public void testRepositoryWithClientConfiguration() {
checkRepositoryConfiguration(ClientConfigurationApplication.class, IgniteClientConfigRepository.class);
}
+ /** Tests repository configuration in case {@link Ignite} with non default is used to access the Ignite cluster. */
+ @Test
+ public void testRepositoryWithoutIgniteInstanceParameter() {
+ checkRepositoryConfiguration(DefaultIgniteBeanApplication.class, IgniteRepositoryWithoutExplicitIgnite.class);
+ }
+
+ /** Tests repository configuration in case {@link IgniteClient} with non default name is used to access the Ignite cluster. */
+ @Test
+ public void testRepositoryWithoutIgniteClientInstanceParameter() {
+ checkRepositoryConfiguration(DefaultIgniteClientBeanApplication.class, IgniteRepositoryWithoutExplicitIgnite.class);
+ }
+
+ /** Tests repository configuration in case {@link ClientConfiguration} with non default name is used to access the Ignite cluster. */
Review comment:
Why 1 line?
##########
File path: modules/spring-data-2.0-ext/src/test/java/org/apache/ignite/springdata/IgniteSpringDataConnectionConfigurationTest.java
##########
@@ -75,6 +76,30 @@ public void testRepositoryWithClientConfiguration() {
checkRepositoryConfiguration(ClientConfigurationApplication.class, IgniteClientConfigRepository.class);
}
+ /** Tests repository configuration in case {@link Ignite} with non default is used to access the Ignite cluster. */
+ @Test
+ public void testRepositoryWithoutIgniteInstanceParameter() {
+ checkRepositoryConfiguration(DefaultIgniteBeanApplication.class, IgniteRepositoryWithoutExplicitIgnite.class);
+ }
+
+ /** Tests repository configuration in case {@link IgniteClient} with non default name is used to access the Ignite cluster. */
+ @Test
+ public void testRepositoryWithoutIgniteClientInstanceParameter() {
+ checkRepositoryConfiguration(DefaultIgniteClientBeanApplication.class, IgniteRepositoryWithoutExplicitIgnite.class);
+ }
+
+ /** Tests repository configuration in case {@link ClientConfiguration} with non default name is used to access the Ignite cluster. */
+ @Test
+ public void testRepositoryWithoutIgnitClientConfigurationParameter() {
+ checkRepositoryConfiguration(DefaultIgniteClientConfigurationBeanApplication.class, IgniteRepositoryWithoutExplicitIgnite.class);
+ }
+
+ /** Tests repository configuration in case {@link IgniteConfiguration} with non default name is used to access the Ignite cluster. */
Review comment:
Why 1 line?
##########
File path: modules/spring-data-2.0-ext/src/test/java/org/apache/ignite/springdata/IgniteSpringDataConnectionConfigurationTest.java
##########
@@ -75,6 +76,30 @@ public void testRepositoryWithClientConfiguration() {
checkRepositoryConfiguration(ClientConfigurationApplication.class, IgniteClientConfigRepository.class);
}
+ /** Tests repository configuration in case {@link Ignite} with non default is used to access the Ignite cluster. */
Review comment:
Why 1 line?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-extensions] sashapolo commented on a change in pull request #55: IGNITE-13169 Remove Ignite bean name requirement for Spring Data Repository
Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #55:
URL: https://github.com/apache/ignite-extensions/pull/55#discussion_r614076190
##########
File path: modules/spring-data-2.0-ext/examples/main/java/org/apache/ignite/springdata20/examples/SpringApplicationConfiguration.java
##########
@@ -29,10 +29,10 @@
/**
* Every {@link IgniteRepository} is bound to a specific Apache Ignite that it communicates to in order to mutate and
Review comment:
```suggestion
* Every {@link IgniteRepository} is bound to a specific Apache Ignite that it communicates with in order to mutate and
```
##########
File path: modules/spring-data-2.0-ext/examples/main/java/org/apache/ignite/springdata20/examples/SpringApplicationConfiguration.java
##########
@@ -29,10 +29,10 @@
/**
* Every {@link IgniteRepository} is bound to a specific Apache Ignite that it communicates to in order to mutate and
* read data via Spring Data API. To pass an instance of Apache Ignite cache to an {@link IgniteRepository} it's
Review comment:
```suggestion
* read data via Spring Data API. To pass an instance of an Apache Ignite cache to an {@link IgniteRepository} it's
```
##########
File path: modules/spring-data-2.0-ext/src/main/java/org/apache/ignite/springdata20/repository/support/IgniteProxyFactory.java
##########
@@ -103,29 +109,25 @@ public IgniteProxy igniteProxy(Class<?> repoInterface) {
private IgniteProxy createIgniteProxy(Class<?> repoInterface) {
RepositoryConfig repoCfg = getRepositoryConfiguration(repoInterface);
- Object connCfg;
-
- try {
- connCfg = ctx.getBean(evaluateExpression(repoCfg.igniteInstance()));
- }
- catch (BeansException ex) {
- try {
- connCfg = ctx.getBean(evaluateExpression(repoCfg.igniteCfg()));
- }
- catch (BeansException ex2) {
- try {
- connCfg = ctx.getBean(evaluateExpression(repoCfg.igniteSpringCfgPath()), String.class);
- }
- catch (BeansException ex3) {
- throw new IllegalArgumentException("Invalid configuration for repository " +
- repoInterface.getName() + ". No beans were found that provide connection configuration to the" +
- " Ignite cluster. Check \"igniteInstance\", \"igniteCfg\", \"igniteSpringCfgPath\" parameters" +
- " of " + RepositoryConfig.class.getName() + " repository annotation.");
- }
- }
- }
-
- return IgniteProxy.of(connCfg);
+ return Stream.<BeanFinder>of(
+ () -> ctx.getBean(evaluateExpression(repoCfg.igniteInstance())),
+ () -> ctx.getBean(evaluateExpression(repoCfg.igniteCfg())),
+ () -> ctx.getBean(evaluateExpression(repoCfg.igniteSpringCfgPath()), String.class),
+ () -> ctx.getBean(Ignite.class),
+ () -> ctx.getBean(IgniteClient.class),
+ () -> ctx.getBean(IgniteConfiguration.class),
+ () -> ctx.getBean(ClientConfiguration.class)
+ ).map(BeanFinder::getBean)
+ .filter(Objects::nonNull)
+ .findFirst()
+ .map(IgniteProxy::of)
+ .orElseThrow(() -> {
Review comment:
You can remove curly braces here and simply write `() -> new IllegalArgumentException...`
##########
File path: modules/spring-data-2.0-ext/src/main/java/org/apache/ignite/springdata20/repository/support/IgniteRepositoryFactoryBean.java
##########
@@ -34,8 +34,8 @@
* The {@link org.apache.ignite.springdata20.repository.config.RepositoryConfig} requires to define one of the
* parameters below in your Spring application configuration in order to get an access to Apache Ignite cluster:
Review comment:
```suggestion
* parameters below in your Spring application configuration in order to get an access to the Apache Ignite cluster:
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org