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.

Sign up for GitHub

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

fix: Clear all session for Spring upgrade #36695

Merged
merged 3 commits into from
Oct 7, 2024
+10 −21
Merged

fix: Clear all session for Spring upgrade #36695

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2636a3c
fix: Modified session clear migration to work with client APIs
nidhi-nair Oct 4, 2024
171f100
fix: Modified session clear migration to work with client APIs
nidhi-nair Oct 4, 2024
42bc1fc
fix: Modified session clear migration to work with client APIs
nidhi-nair Oct 4, 2024
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
31 changes: 10 additions & 21 deletions ...rc/main/java/com/appsmith/server/migrations/db/ce/Migration063CacheBustSpringBoot3_3.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,36 +3,25 @@
import io.mongock.api.annotations.ChangeUnit;
import io.mongock.api.annotations.Execution;
import io.mongock.api.annotations.RollbackExecution;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.data.redis.core.ReactiveRedisOperations;
import org.springframework.data.redis.core.script.RedisScript;
import reactor.core.publisher.Flux;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.data.redis.core.ReactiveRedisTemplate;

@RequiredArgsConstructor
@Slf4j
@ChangeUnit(order = "063", id = "reset_session_oauth2_spring_3_3")
public class Migration063CacheBustSpringBoot3_3 {

private final ReactiveRedisOperations<String, String> reactiveRedisOperations;

@RollbackExecution
public void rollbackExecution() {}

@Execution
public void execute() {
doClearRedisOAuth2AuthClientKeys(reactiveRedisOperations);
}

public static void doClearRedisOAuth2AuthClientKeys(
ReactiveRedisOperations<String, String> reactiveRedisOperations) {
final String authorizedClientsKey =
"sessionAttr:org.springframework.security.oauth2.client.web.server.WebSessionServerOAuth2AuthorizedClientRepository.AUTHORIZED_CLIENTS";
final String script =
"for _,k in ipairs(redis.call('keys','spring:session:sessions:*')) do local fieldExists = redis.call('hexists', k, '"
+ authorizedClientsKey + "'); if fieldExists == 1 then redis.call('del', k) end end";
final Flux<Object> flushdb = reactiveRedisOperations.execute(RedisScript.of(script));

flushdb.blockLast();
public void execute(
@Qualifier("reactiveRedisTemplate") final ReactiveRedisTemplate<String, Object> reactiveRedisTemplate) {
reactiveRedisTemplate
.getConnectionFactory()
.getReactiveConnection()
.serverCommands()
.flushDb()
.block();
Comment on lines +18 to +25
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure proper error handling for the Redis flush operation

It's important to handle potential exceptions when performing critical operations like flushing the Redis database. Without proper error handling, any failures during this operation could lead to unhandled exceptions and impact the application's stability.

Consider enhancing your code to gracefully handle any errors:

reactiveRedisTemplate
        .getConnectionFactory()
        .getReactiveConnection()
        .serverCommands()
        .flushDb()
        .doOnError(error -> log.error("Failed to flush Redis database during migration", error))
        .block();

This way, you log any exceptions, allowing for easier debugging and ensuring the migration process can handle failures appropriately.

🛠️ Refactor suggestion

Avoid blocking calls in reactive streams

Remember, using block() in reactive programming can negate the benefits of non-blocking, asynchronous execution. It can lead to thread blocking and reduce application performance.

Consider refactoring the code to use non-blocking alternatives. For example:

reactiveRedisTemplate
        .getConnectionFactory()
        .getReactiveConnection()
        .serverCommands()
        .flushDb()
        .subscribe(
            unused -> log.info("Redis database flushed successfully during migration"),
            error -> log.error("Failed to flush Redis database during migration", error)
        );

This approach allows the operation to proceed asynchronously, handling success and error cases without blocking the thread.

}
}
Loading