Skip to content
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

Refactoring: use namedJdbcTemplate bean instead of internal new object #2864

Merged
merged 3 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.oauth2.provider.ClientDetails;
import org.springframework.stereotype.Component;
Expand Down Expand Up @@ -41,7 +41,7 @@ public class JdbcQueryableClientDetailsService

public JdbcQueryableClientDetailsService(
final @Qualifier("jdbcClientDetailsService") MultitenantJdbcClientDetailsService delegate,
final JdbcTemplate jdbcTemplate,
final @Qualifier("namedJdbcTemplate") NamedParameterJdbcTemplate jdbcTemplate,
final JdbcPagingListFactory pagingListFactory) {
super(jdbcTemplate, pagingListFactory, new ClientDetailsRowMapper());
this.delegate = delegate;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
import org.springframework.util.StringUtils;
Expand All @@ -17,7 +16,7 @@

public abstract class AbstractQueryable<T> implements Queryable<T> {

private NamedParameterJdbcTemplate namedParameterJdbcTemplate;
protected final NamedParameterJdbcTemplate namedParameterJdbcTemplate;

protected final JdbcPagingListFactory pagingListFactory;

Expand All @@ -29,10 +28,10 @@ public abstract class AbstractQueryable<T> implements Queryable<T> {

private int pageSize = 200;

protected AbstractQueryable(final JdbcTemplate jdbcTemplate,
protected AbstractQueryable(final NamedParameterJdbcTemplate namedParameterJdbcTemplate,
final JdbcPagingListFactory pagingListFactory,
final RowMapper<T> rowMapper) {
this.namedParameterJdbcTemplate = new NamedParameterJdbcTemplate(jdbcTemplate);
this.namedParameterJdbcTemplate = namedParameterJdbcTemplate;
this.pagingListFactory = pagingListFactory;
this.rowMapper = rowMapper;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import java.util.List;
import java.util.Map;

import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;

Expand All @@ -30,8 +29,8 @@ public class JdbcPagingListFactory {
private NamedParameterJdbcTemplate jdbcTemplate;
private LimitSqlAdapter limitSqlAdapter;

public JdbcPagingListFactory(JdbcTemplate jdbcTemplate, LimitSqlAdapter limitSqlAdapter) {
this.jdbcTemplate = new NamedParameterJdbcTemplate(jdbcTemplate);
public JdbcPagingListFactory(NamedParameterJdbcTemplate jdbcTemplate, LimitSqlAdapter limitSqlAdapter) {
this.jdbcTemplate = jdbcTemplate;
this.limitSqlAdapter = limitSqlAdapter;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.springframework.dao.EmptyResultDataAccessException;
import org.springframework.dao.IncorrectResultSizeDataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;

import java.sql.SQLException;
import java.sql.Timestamp;
Expand Down Expand Up @@ -73,12 +74,12 @@ public Logger getLogger() {
private JdbcIdentityZoneProvisioning jdbcIdentityZoneProvisioning;

public JdbcScimGroupProvisioning(
final JdbcTemplate jdbcTemplate,
final NamedParameterJdbcTemplate namedJdbcTemplate,
final JdbcPagingListFactory pagingListFactory,
final DbUtils dbUtils) throws SQLException {
super(jdbcTemplate, pagingListFactory, new ScimGroupRowMapper());
super(namedJdbcTemplate, pagingListFactory, new ScimGroupRowMapper());

this.jdbcTemplate = jdbcTemplate;
this.jdbcTemplate = namedJdbcTemplate.getJdbcTemplate();

final String quotedGroupsTableName = dbUtils.getQuotedIdentifier(GROUP_TABLE, jdbcTemplate);
updateGroupSql = String.format(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,15 @@ public Logger getLogger() {
private boolean useCaseInsensitiveQueries = false;

public JdbcScimUserProvisioning(
final JdbcTemplate jdbcTemplate,
final NamedParameterJdbcTemplate namedJdbcTemplate,
final JdbcPagingListFactory pagingListFactory,
final PasswordEncoder passwordEncoder,
final IdentityZoneManager identityZoneManager,
final JdbcIdentityZoneProvisioning jdbcIdentityZoneProvisioning
) {
super(jdbcTemplate, pagingListFactory, mapper);
Assert.notNull(jdbcTemplate);
this.jdbcTemplate = jdbcTemplate;
super(namedJdbcTemplate, pagingListFactory, mapper);
Assert.notNull(namedJdbcTemplate, "JdbcTemplate required");
this.jdbcTemplate = namedJdbcTemplate.getJdbcTemplate();
setQueryConverter(new SimpleSearchQueryConverter());
this.passwordEncoder = passwordEncoder;
this.jdbcIdentityZoneProvisioning = jdbcIdentityZoneProvisioning;
Expand Down Expand Up @@ -251,7 +251,6 @@ public String[] mapFromInternal(final String[] attr) {
return pagingListFactory.createJdbcPagingList(sql, where.getParams(), rowMapper, getPageSize());
}

final NamedParameterJdbcTemplate namedParameterJdbcTemplate = new NamedParameterJdbcTemplate(jdbcTemplate);
return namedParameterJdbcTemplate.query(sql, where.getParams(), rowMapper);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,13 @@ public class MultitenantJdbcClientDetailsService extends MultitenantClientServic
private JdbcListFactory listFactory;

public MultitenantJdbcClientDetailsService(
final JdbcTemplate jdbcTemplate,
final NamedParameterJdbcTemplate jdbcTemplate,
final IdentityZoneManager identityZoneManager,
final @Qualifier("cachingPasswordEncoder") PasswordEncoder passwordEncoder) {
super(identityZoneManager);
Assert.notNull(jdbcTemplate, "JDbcTemplate required");
this.jdbcTemplate = jdbcTemplate;
this.listFactory = new DefaultJdbcListFactory(new NamedParameterJdbcTemplate(jdbcTemplate));
this.jdbcTemplate = jdbcTemplate.getJdbcTemplate();
this.listFactory = new DefaultJdbcListFactory(jdbcTemplate);
this.passwordEncoder = passwordEncoder;
}

Expand Down
6 changes: 5 additions & 1 deletion server/src/main/resources/spring/data-source.xml
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,12 @@
<property name="dataSource" ref="dataSource"/>
</bean>

<bean id="namedJdbcTemplate" class="org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate">
<constructor-arg name="classicJdbcTemplate" ref="jdbcTemplate"/>
</bean>

<bean id="jdbcPagingListFactory" class="org.cloudfoundry.identity.uaa.resources.jdbc.JdbcPagingListFactory">
<constructor-arg ref="jdbcTemplate"/>
<constructor-arg ref="namedJdbcTemplate"/>
<constructor-arg ref="limitSqlAdapter"/>
</bean>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
import org.springframework.security.authentication.BadCredentialsException;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.oauth2.common.util.RandomValueStringGenerator;
Expand All @@ -44,11 +45,11 @@ class PasswordChangeEndpointTests {
private PasswordEncoder passwordEncoder;

@BeforeEach
void setup(@Autowired JdbcTemplate jdbcTemplate) {
void setup(@Autowired JdbcTemplate jdbcTemplate, @Autowired NamedParameterJdbcTemplate namedJdbcTemplate) {
mockIdentityZoneManager = mock(IdentityZoneManager.class);
jdbcScimUserProvisioning = new JdbcScimUserProvisioning(
jdbcTemplate,
new JdbcPagingListFactory(jdbcTemplate, LimitSqlAdapterFactory.getLimitSqlAdapter()),
namedJdbcTemplate,
new JdbcPagingListFactory(namedJdbcTemplate, LimitSqlAdapterFactory.getLimitSqlAdapter()),
passwordEncoder, mockIdentityZoneManager, new JdbcIdentityZoneProvisioning(jdbcTemplate));

final RandomValueStringGenerator generator = new RandomValueStringGenerator();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.security.authentication.BadCredentialsException;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
Expand Down Expand Up @@ -50,7 +50,7 @@ class UaaClientAuthenticationProviderTest {
private JwtClientAuthentication jwtClientAuthentication;

@Autowired
JdbcTemplate jdbcTemplate;
private NamedParameterJdbcTemplate namedJdbcTemplate;

@Autowired
private PasswordEncoder passwordEncoder;
Expand All @@ -61,7 +61,7 @@ void setUpForClientTests() {
jwtClientAuthentication = mock(JwtClientAuthentication.class);
when(mockIdentityZoneManager.getCurrentIdentityZoneId()).thenReturn(IdentityZone.getUaaZoneId());

jdbcClientDetailsService = new MultitenantJdbcClientDetailsService(jdbcTemplate, mockIdentityZoneManager, passwordEncoder);
jdbcClientDetailsService = new MultitenantJdbcClientDetailsService(namedJdbcTemplate, mockIdentityZoneManager, passwordEncoder);
UaaClientDetailsUserDetailsService clientDetailsService = new UaaClientDetailsUserDetailsService(jdbcClientDetailsService);
client = createClient();
authenticationProvider = new ClientDetailsAuthenticationProvider(clientDetailsService, passwordEncoder, jwtClientAuthentication);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;

Expand Down Expand Up @@ -58,12 +59,13 @@ class LdapGroupMappingAuthorizationManagerTests {
@BeforeEach
void initLdapGroupMappingAuthorizationManagerTests(
@Autowired JdbcTemplate jdbcTemplate,
@Autowired LimitSqlAdapter limitSqlAdapter
@Autowired LimitSqlAdapter limitSqlAdapter,
@Autowired NamedParameterJdbcTemplate namedJdbcTemplate
) throws SQLException {
TestUtils.cleanAndSeedDb(jdbcTemplate);
JdbcPagingListFactory pagingListFactory = new JdbcPagingListFactory(jdbcTemplate, limitSqlAdapter);
JdbcPagingListFactory pagingListFactory = new JdbcPagingListFactory(namedJdbcTemplate, limitSqlAdapter);
DbUtils dbUtils = new DbUtils();
gDB = new JdbcScimGroupProvisioning(jdbcTemplate, pagingListFactory, dbUtils);
gDB = new JdbcScimGroupProvisioning(namedJdbcTemplate, pagingListFactory, dbUtils);
eDB = new JdbcScimGroupExternalMembershipManager(jdbcTemplate, dbUtils);
((JdbcScimGroupExternalMembershipManager) eDB).setScimGroupProvisioning(gDB);
assertEquals(0, gDB.retrieveAll(IdentityZoneHolder.get().getId()).size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.oauth2.common.util.RandomValueStringGenerator;
Expand All @@ -28,6 +29,8 @@ public class ClientAdminBootstrapMultipleSecretsUpdateTests {

@Autowired
private JdbcTemplate jdbcTemplate;
@Autowired
private NamedParameterJdbcTemplate namedJdbcTemplate;
private RandomValueStringGenerator randomValueStringGenerator;
private String autoApproveId;
private String allowPublicId;
Expand All @@ -47,7 +50,7 @@ void passwordHashFirstSecretDidNotChangeButSecondIsNullDuringBootstrap() throws
PasswordEncoder encoder = new BackwardsCompatibleDelegatingPasswordEncoder(new BCryptPasswordEncoder(10));
IdentityZoneManager mockIdentityZoneManager = mock(IdentityZoneManager.class);
when(mockIdentityZoneManager.getCurrentIdentityZoneId()).thenReturn(IdentityZone.getUaaZoneId());
MultitenantJdbcClientDetailsService localJdbcClientDetailsService = new MultitenantJdbcClientDetailsService(jdbcTemplate, mockIdentityZoneManager, encoder);
MultitenantJdbcClientDetailsService localJdbcClientDetailsService = new MultitenantJdbcClientDetailsService(namedJdbcTemplate, mockIdentityZoneManager, encoder);
ClientMetadataProvisioning localMetadataProvisioning = new JdbcClientMetadataProvisioning(localJdbcClientDetailsService, jdbcTemplate);
ClientAdminBootstrap localAdminBootstrap = new ClientAdminBootstrap(
encoder,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.event.ContextRefreshedEvent;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.AuthorityUtils;
import org.springframework.security.crypto.password.PasswordEncoder;
Expand Down Expand Up @@ -77,6 +78,9 @@ class ClientAdminBootstrapTests {
@Autowired
private JdbcTemplate jdbcTemplate;

@Autowired
private NamedParameterJdbcTemplate namedJdbcTemplate;

private String autoApproveId;

private String allowPublicId;
Expand All @@ -89,7 +93,7 @@ void setUpClientAdminTests() {
IdentityZoneManager mockIdentityZoneManager = mock(IdentityZoneManager.class);
when(mockIdentityZoneManager.getCurrentIdentityZoneId()).thenReturn(IdentityZone.getUaaZoneId());

multitenantJdbcClientDetailsService = spy(new MultitenantJdbcClientDetailsService(jdbcTemplate, mockIdentityZoneManager, passwordEncoder));
multitenantJdbcClientDetailsService = spy(new MultitenantJdbcClientDetailsService(namedJdbcTemplate, mockIdentityZoneManager, passwordEncoder));

clientMetadataProvisioning = new JdbcClientMetadataProvisioning(multitenantJdbcClientDetailsService, jdbcTemplate);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.dao.EmptyResultDataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
import org.springframework.security.crypto.password.PasswordEncoder;

import java.net.URL;
Expand Down Expand Up @@ -41,6 +42,9 @@ class JdbcClientMetadataProvisioningTest {
@Autowired
private JdbcTemplate jdbcTemplate;

@Autowired
private NamedParameterJdbcTemplate namedJdbcTemplate;

@Autowired
private PasswordEncoder passwordEncoder;

Expand All @@ -51,7 +55,7 @@ void createDatasource() {
identityZoneId = "identityZoneId-" + randomValueStringGenerator.generate();
clientId = "clientId-" + randomValueStringGenerator.generate();

MultitenantJdbcClientDetailsService clientService = new MultitenantJdbcClientDetailsService(jdbcTemplate, null, passwordEncoder);
MultitenantJdbcClientDetailsService clientService = new MultitenantJdbcClientDetailsService(namedJdbcTemplate, null, passwordEncoder);
jdbcClientMetadataProvisioning = new JdbcClientMetadataProvisioning(clientService, jdbcTemplate);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationContext;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
import org.springframework.security.crypto.password.PasswordEncoder;

import java.sql.SQLException;
Expand All @@ -34,7 +35,7 @@ class JdbcQueryableClientDetailsServiceTests {
private MultitenantJdbcClientDetailsService multitenantJdbcClientDetailsService;

@Autowired
private JdbcTemplate jdbcTemplate;
private NamedParameterJdbcTemplate namedParameterJdbcTemplate;

@Autowired
private LimitSqlAdapter limitSqlAdapter;
Expand All @@ -45,14 +46,14 @@ class JdbcQueryableClientDetailsServiceTests {
@BeforeEach
void setUp() {
multitenantJdbcClientDetailsService = new MultitenantJdbcClientDetailsService(
jdbcTemplate,
namedParameterJdbcTemplate,
null,
passwordEncoder);
jdbcQueryableClientDetailsService = new JdbcQueryableClientDetailsService(
multitenantJdbcClientDetailsService,
jdbcTemplate,
namedParameterJdbcTemplate,
new JdbcPagingListFactory(
jdbcTemplate,
namedParameterJdbcTemplate,
limitSqlAdapter));
}

Expand Down Expand Up @@ -100,25 +101,25 @@ private static void addClient(

@Test
void queryEquals() {
verifyScimEquality(jdbcTemplate, jdbcQueryableClientDetailsService, "zoneOneId");
verifyScimEquality(namedParameterJdbcTemplate.getJdbcTemplate(), jdbcQueryableClientDetailsService, "zoneOneId");
}

@Test
void queryExists() {
verifyScimPresent(jdbcTemplate, jdbcQueryableClientDetailsService, "zoneOneId");
verifyScimPresent(namedParameterJdbcTemplate.getJdbcTemplate(), jdbcQueryableClientDetailsService, "zoneOneId");
}

@Test
void queryEqualsInAnotherZone() {
verifyScimEquality(jdbcTemplate, jdbcQueryableClientDetailsService, "zoneOneId");
verifyScimEquality(jdbcTemplate, jdbcQueryableClientDetailsService, "otherZoneId");
verifyScimEquality(namedParameterJdbcTemplate.getJdbcTemplate(), jdbcQueryableClientDetailsService, "zoneOneId");
verifyScimEquality(namedParameterJdbcTemplate.getJdbcTemplate(), jdbcQueryableClientDetailsService, "otherZoneId");
assertEquals(8, multitenantJdbcClientDetailsService.getTotalCount());
}

@Test
void queryExistsInAnotherZone() {
verifyScimPresent(jdbcTemplate, jdbcQueryableClientDetailsService, "zoneOneId");
verifyScimPresent(jdbcTemplate, jdbcQueryableClientDetailsService, "otherZoneId");
verifyScimPresent(namedParameterJdbcTemplate.getJdbcTemplate(), jdbcQueryableClientDetailsService, "zoneOneId");
verifyScimPresent(namedParameterJdbcTemplate.getJdbcTemplate(), jdbcQueryableClientDetailsService, "otherZoneId");
assertEquals(8, multitenantJdbcClientDetailsService.getTotalCount());
}

Expand Down
Loading
Loading