-
Notifications
You must be signed in to change notification settings - Fork 346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add jdbcAggregateOperationsRef
to @EnableJdbcRepositories
#1704
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ | |
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.springframework.data.jdbc.repository.config; | ||
package org.springframework.data.jdbc.dialect; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved package to avoid circular dependency between package There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, in r2dbc we already have it in |
||
|
||
import java.sql.Connection; | ||
import java.sql.DatabaseMetaData; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,6 @@ | |
import org.apache.commons.logging.Log; | ||
import org.apache.commons.logging.LogFactory; | ||
import org.springframework.beans.BeansException; | ||
import org.springframework.beans.factory.NoSuchBeanDefinitionException; | ||
import org.springframework.context.ApplicationContext; | ||
import org.springframework.context.ApplicationContextAware; | ||
import org.springframework.context.annotation.Bean; | ||
|
@@ -40,6 +39,7 @@ | |
import org.springframework.data.jdbc.core.dialect.JdbcDialect; | ||
import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; | ||
import org.springframework.data.jdbc.core.mapping.JdbcSimpleTypes; | ||
import org.springframework.data.jdbc.dialect.DialectResolver; | ||
import org.springframework.data.mapping.model.SimpleTypeHolder; | ||
import org.springframework.data.relational.RelationalManagedTypes; | ||
import org.springframework.data.relational.core.conversion.RelationalConverter; | ||
|
@@ -61,6 +61,7 @@ | |
* @author Christoph Strobl | ||
* @author Myeonghyeon Lee | ||
* @author Chirag Tailor | ||
* @author Tomohiko Ozawa | ||
* @since 1.1 | ||
*/ | ||
@Configuration(proxyBeanMethods = false) | ||
|
@@ -103,7 +104,7 @@ public RelationalManagedTypes jdbcManagedTypes() throws ClassNotFoundException { | |
* | ||
* @param namingStrategy optional {@link NamingStrategy}. Use | ||
* {@link org.springframework.data.relational.core.mapping.DefaultNamingStrategy#INSTANCE} as fallback. | ||
* @param customConversions see {@link #jdbcCustomConversions()}. | ||
* @param customConversions see {@link #jdbcCustomConversions(Optional)}. | ||
* @param jdbcManagedTypes JDBC managed types, typically discovered through {@link #jdbcManagedTypes() an entity | ||
* scan}. | ||
* @return must not be {@literal null}. | ||
|
@@ -124,7 +125,7 @@ public JdbcMappingContext jdbcMappingContext(Optional<NamingStrategy> namingStra | |
* {@link #jdbcMappingContext(Optional, JdbcCustomConversions, RelationalManagedTypes)}. | ||
* | ||
* @see #jdbcMappingContext(Optional, JdbcCustomConversions, RelationalManagedTypes) | ||
* @see #jdbcCustomConversions() | ||
* @see #jdbcCustomConversions(Optional) | ||
* @return must not be {@literal null}. | ||
*/ | ||
@Bean | ||
|
@@ -147,23 +148,17 @@ public JdbcConverter jdbcConverter(JdbcMappingContext mappingContext, NamedParam | |
* @return will never be {@literal null}. | ||
*/ | ||
@Bean | ||
public JdbcCustomConversions jdbcCustomConversions() { | ||
|
||
try { | ||
|
||
Dialect dialect = applicationContext.getBean(Dialect.class); | ||
SimpleTypeHolder simpleTypeHolder = dialect.simpleTypes().isEmpty() ? JdbcSimpleTypes.HOLDER | ||
: new SimpleTypeHolder(dialect.simpleTypes(), JdbcSimpleTypes.HOLDER); | ||
|
||
return new JdbcCustomConversions( | ||
CustomConversions.StoreConversions.of(simpleTypeHolder, storeConverters(dialect)), userConverters()); | ||
|
||
} catch (NoSuchBeanDefinitionException exception) { | ||
|
||
public JdbcCustomConversions jdbcCustomConversions(Optional<Dialect> dialect) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional bean dependency can be expressed by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change does not affect the feature and introduces the non-compatible change. Let's segregate it into separate PR so it can be merged in the next major release There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I'll revert this change for now, and then create another PR that adds a factory class later. |
||
return dialect.map(d -> { | ||
SimpleTypeHolder simpleTypeHolder = d.simpleTypes().isEmpty() | ||
? JdbcSimpleTypes.HOLDER | ||
: new SimpleTypeHolder(d.simpleTypes(), JdbcSimpleTypes.HOLDER); | ||
return new JdbcCustomConversions(CustomConversions.StoreConversions.of(simpleTypeHolder, storeConverters(d)), | ||
userConverters()); | ||
}).orElseGet(() -> { | ||
LOG.warn("No dialect found; CustomConversions will be configured without dialect specific conversions"); | ||
|
||
return new JdbcCustomConversions(); | ||
} | ||
}); | ||
} | ||
|
||
protected List<?> userConverters() { | ||
|
@@ -191,7 +186,7 @@ private List<Object> storeConverters(Dialect dialect) { | |
public JdbcAggregateTemplate jdbcAggregateTemplate(ApplicationContext applicationContext, | ||
JdbcMappingContext mappingContext, JdbcConverter converter, DataAccessStrategy dataAccessStrategy) { | ||
|
||
return new JdbcAggregateTemplate(applicationContext, mappingContext, converter, dataAccessStrategy); | ||
return new JdbcAggregateTemplate(applicationContext, converter, dataAccessStrategy); | ||
} | ||
|
||
/** | ||
|
@@ -207,8 +202,7 @@ public DataAccessStrategy dataAccessStrategyBean(NamedParameterJdbcOperations op | |
|
||
SqlGeneratorSource sqlGeneratorSource = new SqlGeneratorSource(context, jdbcConverter, dialect); | ||
DataAccessStrategyFactory factory = new DataAccessStrategyFactory(sqlGeneratorSource, jdbcConverter, operations, | ||
new SqlParametersFactory(context, jdbcConverter), | ||
new InsertStrategyFactory(operations, dialect)); | ||
new SqlParametersFactory(context, jdbcConverter), new InsertStrategyFactory(operations, dialect)); | ||
|
||
return factory.create(); | ||
} | ||
|
@@ -219,8 +213,7 @@ public DataAccessStrategy dataAccessStrategyBean(NamedParameterJdbcOperations op | |
* @param operations the {@link NamedParameterJdbcOperations} allowing access to a {@link java.sql.Connection}. | ||
* @return the {@link Dialect} to be used. | ||
* @since 2.0 | ||
* @throws org.springframework.data.jdbc.repository.config.DialectResolver.NoDialectException if the {@link Dialect} | ||
* cannot be determined. | ||
* @throws DialectResolver.NoDialectException if the {@link Dialect} cannot be determined. | ||
*/ | ||
@Bean | ||
public Dialect jdbcDialect(NamedParameterJdbcOperations operations) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed RelationalMappingContext because it can be obtained by JdbcConverter instance, but should it be kept for backward compatibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better to create a new constructor here and mark the previous as deprecated in favor of new. We should not change public API for backward compatibility