Skip to content

fix for issue #25323 with test #25558

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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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 @@ -256,6 +256,11 @@ protected boolean isExistingTransaction(Object transaction) {

@Override
protected void doBegin(Object transaction, TransactionDefinition definition) {
// Added line of code to allow users to get readOnly status earlier
// (i.e. in case @prepareSynchronization() doesn't do anything
// because status.isNewSynchronization() == false
// and so read only status is not set in the next method)
TransactionSynchronizationManager.setCurrentTransactionReadOnly(definition.isReadOnly());
DataSourceTransactionObject txObject = (DataSourceTransactionObject) transaction;
Connection con = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.sql.DatabaseMetaData;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.sql.SQLTimeoutException;
import java.sql.Savepoint;
import java.sql.Statement;

Expand Down Expand Up @@ -551,6 +552,70 @@ protected void doInTransactionWithoutResult(TransactionStatus status) throws Run
verify(con).close();
verify(con2).close();
}
/**
* This test checks added line of code in the beginning of doBegin() method of DataSourceTransactionManager class
* which allows users to check readonly status earlier of transaction earlier than they otherwise could in some circumstances
* and then for example if readOnly re-use that connection multiple times for different nodes of a DataSource
* @throws Exception
*/
@Test
public void testEarlyInitOfReadonlyStatusForCustomDataSource() throws Exception {
// Ensures that prepareSynchronization() will not do anything when startTransaction() is called
// (because status.isNewSynchronization()==false)
// to properly simulate the case where added code is needed in DataSourceTransactionManager
tm.setTransactionSynchronization(DataSourceTransactionManager.SYNCHRONIZATION_NEVER);
//need a DataSource to test user case that datasource can get same connection multiple times if readOnly
TestCustomRODataSource dsCustom = new TestCustomRODataSource();
DefaultTransactionDefinition transDef = new DefaultTransactionDefinition();
TransactionStatus tsNotReadonly = tm.getTransaction(transDef);
assertThat(TransactionSynchronizationManager.isCurrentTransactionReadOnly()).isFalse();
tm.rollback(tsNotReadonly);//needed to reset for next transaction/test
transDef.setReadOnly(true);
TransactionStatus tsIsReadonly = tm.getTransaction(transDef);
assertThat(TransactionSynchronizationManager.isCurrentTransactionReadOnly()).isTrue();
dsCustom.getConnection();//this is where user wants to check readonly
tm.rollback(tsIsReadonly);//needed to reset for next transaction/test
verify(con, times(2)).rollback();
verify(con, times(2)).close();
// need this to clean up state so @AfterEach validation succeeds
TransactionSynchronizationManager.setCurrentTransactionReadOnly(false);
}
private static class TestCustomRODataSource extends AbstractDataSource {
/**
* <p>Attempts to establish a connection with the data source that
* this {@code DataSource} object represents.
*
* @return a connection to the data source
* @throws SQLException if a database access error occurs
* @throws SQLTimeoutException when the driver has determined that the
* timeout value specified by the {@code setLoginTimeout} method has
* been exceeded and has at least tried to cancel the current database connection attempt
*/
@Override
public Connection getConnection() throws SQLException {
assertThat(TransactionSynchronizationManager.isCurrentTransactionReadOnly()).isTrue();
return null;
}
/**
* <p>Attempts to establish a connection with the data source that
* this {@code DataSource} object represents.
*
* @param username the database user on whose behalf the connection is
* being made
* @param password the user's password
* @return a connection to the data source
* @throws SQLException if a database access error occurs
* @throws SQLTimeoutException when the driver has determined that the timeout
* value specified by the {@code setLoginTimeout} method has been exceeded and
* has at least tried to cancel the current database connection attempt
* @since 1.4
*/
@Override
public Connection getConnection(String username, String password) throws SQLException {
assertThat(TransactionSynchronizationManager.isCurrentTransactionReadOnly()).isTrue();
return null;
}
}

@Test
public void testParticipatingTransactionWithRollbackOnlyAndInnerSynch() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ public UriComponents build() {
/**
* Build a {@code UriComponents} instance from the various components
* contained in this builder.
* @param encoded whether all the components set in this builder are
* @param encoded asserts whether all the components set in this builder are already
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise here, this looks accidentally included here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I accidentally included two separate fixes in one PR. Will submit it in a new one.

* encoded ({@code true}) or not ({@code false})
* @return the URI components
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,18 @@
*/
class UriComponentsBuilderTests {

@Test
public void fromUriStringQueryParamContainingBracket() throws URISyntaxException {
URI uri = new URI("http://example.com/some/path?query=[fromto]");
URI finalUri = uri;
assertThatIllegalArgumentException().isThrownBy(() -> UriComponentsBuilder.fromUri(finalUri).build(true));
//ok, then encode it
uri = UriComponentsBuilder.fromUri(uri).build(false).encode().toUri();
//confirm encoded
URI checkUri = UriComponentsBuilder.fromUri(uri).build(true).toUri();
assertThat(uri.toString()).isEqualTo(checkUri.toString());
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you accidentally included this, which I recognize as a scenario from an unrelated issue.

@Test
void plain() throws URISyntaxException {
UriComponentsBuilder builder = UriComponentsBuilder.newInstance();
Expand Down