Skip to content

Commit fed5762

Browse files
fix: address PR review comments - timeouts, isConfigured, folder naming, locking, OAuth tests
Agent-Logs-Url: https://github.com/vitorhugo-java/SpringBoot-JobApplyTracker/sessions/04ecdf63-053c-45df-a658-80942795419b Co-authored-by: vitorhugo-java <65777252+vitorhugo-java@users.noreply.github.com>
1 parent eb02e64 commit fed5762

7 files changed

Lines changed: 265 additions & 9 deletions

File tree

src/main/java/com/jobtracker/config/GoogleDriveProperties.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public GoogleDriveProperties(
2222
@Value("${app.google-drive.client-id:}") String clientId,
2323
@Value("${app.google-drive.client-secret:}") String clientSecret,
2424
@Value("${app.google-drive.redirect-uri:}") String redirectUri,
25-
@Value("${app.google-drive.oauth-complete-url:http://localhost:5173/settings/google-drive/callback}") String oauthCompleteUrl,
25+
@Value("${app.google-drive.oauth-complete-url:}") String oauthCompleteUrl,
2626
@Value("${app.google-drive.authorization-uri:https://accounts.google.com/o/oauth2/v2/auth}") String authorizationUri,
2727
@Value("${app.google-drive.token-uri:https://oauth2.googleapis.com/token}") String tokenUri
2828
) {
@@ -68,7 +68,7 @@ public String getScopeValue() {
6868
}
6969

7070
public boolean isConfigured() {
71-
return hasText(clientId) && hasText(clientSecret) && hasText(redirectUri);
71+
return hasText(clientId) && hasText(clientSecret) && hasText(redirectUri) && hasText(oauthCompleteUrl);
7272
}
7373

7474
public void validateConfigured() {

src/main/java/com/jobtracker/repository/ApplicationRepository.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22

33
import com.jobtracker.entity.JobApplication;
44
import com.jobtracker.entity.enums.ApplicationStatus;
5+
import jakarta.persistence.LockModeType;
56
import org.springframework.data.domain.Page;
67
import org.springframework.data.domain.Pageable;
78
import org.springframework.data.jpa.repository.JpaRepository;
89
import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
10+
import org.springframework.data.jpa.repository.Lock;
911
import org.springframework.data.jpa.repository.Query;
1012
import org.springframework.data.repository.query.Param;
1113
import org.springframework.stereotype.Repository;
@@ -20,6 +22,10 @@ public interface ApplicationRepository extends JpaRepository<JobApplication, UUI
2022

2123
Optional<JobApplication> findByIdAndUserId(UUID id, UUID userId);
2224

25+
@Lock(LockModeType.PESSIMISTIC_WRITE)
26+
@Query("SELECT a FROM JobApplication a WHERE a.id = :id AND a.user.id = :userId")
27+
Optional<JobApplication> findByIdAndUserIdForUpdate(@Param("id") UUID id, @Param("userId") UUID userId);
28+
2329
long countByUserIdAndArchivedFalse(UUID userId);
2430

2531
long countByUserIdAndInterviewScheduledTrueAndArchivedFalse(UUID userId);

src/main/java/com/jobtracker/service/DefaultGoogleDriveApiClient.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.jobtracker.config.GoogleDriveProperties;
55
import com.jobtracker.exception.BadRequestException;
66
import org.springframework.http.MediaType;
7+
import org.springframework.http.client.SimpleClientHttpRequestFactory;
78
import org.springframework.stereotype.Component;
89
import org.springframework.util.LinkedMultiValueMap;
910
import org.springframework.util.MultiValueMap;
@@ -26,7 +27,10 @@ public class DefaultGoogleDriveApiClient implements GoogleDriveApiClient {
2627

2728
public DefaultGoogleDriveApiClient(GoogleDriveProperties properties, RestClient.Builder restClientBuilder) {
2829
this.properties = properties;
29-
this.restClient = restClientBuilder.build();
30+
SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory();
31+
requestFactory.setConnectTimeout(Duration.ofSeconds(10));
32+
requestFactory.setReadTimeout(Duration.ofSeconds(30));
33+
this.restClient = restClientBuilder.requestFactory(requestFactory).build();
3034
}
3135

3236
@Override

src/main/java/com/jobtracker/service/GoogleDriveService.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public void deleteBaseResume(UUID baseResumeId) {
116116
public GoogleDriveResumeCopyResponse copyBaseResumeToApplication(UUID applicationId, GoogleDriveResumeCopyRequest request) {
117117
UUID userId = securityUtils.getCurrentUserId();
118118
GoogleDriveConnection connection = getConnectionWithFreshAccessToken();
119-
JobApplication application = applicationRepository.findByIdAndUserId(applicationId, userId)
119+
JobApplication application = applicationRepository.findByIdAndUserIdForUpdate(applicationId, userId)
120120
.orElseThrow(() -> new ResourceNotFoundException("Application not found with id: " + applicationId));
121121

122122
if (!StringUtils.hasText(connection.getRootFolderId())) {
@@ -239,9 +239,10 @@ private String extractGoogleFileId(String rawValue) {
239239
}
240240

241241
private String buildVacancyFolderName(JobApplication application) {
242-
String baseName = firstNonBlank(application.getVacancyName(), application.getOrganization(), "Application");
243-
String suffix = "APP-" + application.getId().toString();
244-
return truncateFileName(sanitizeFileName(baseName + " - " + suffix), 180);
242+
String suffix = " - APP-" + application.getId().toString();
243+
String rawBase = firstNonBlank(application.getVacancyName(), application.getOrganization(), "Application");
244+
String truncatedBase = truncateFileName(sanitizeFileName(rawBase), 180 - suffix.length());
245+
return truncatedBase + suffix;
245246
}
246247

247248
private String buildCopiedDocumentName(JobApplication application, String baseResumeName) {

src/main/resources/application.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ app:
7373
client-id: ${GOOGLE_DRIVE_CLIENT_ID:}
7474
client-secret: ${GOOGLE_DRIVE_CLIENT_SECRET:}
7575
redirect-uri: ${GOOGLE_DRIVE_REDIRECT_URI:http://localhost:8080/api/v1/google-drive/oauth/callback}
76-
oauth-complete-url: ${GOOGLE_DRIVE_OAUTH_COMPLETE_URL:http://localhost:5173/settings/google-drive/callback}
76+
oauth-complete-url: ${GOOGLE_DRIVE_OAUTH_COMPLETE_URL:}
7777
authorization-uri: ${GOOGLE_DRIVE_AUTHORIZATION_URI:https://accounts.google.com/o/oauth2/v2/auth}
7878
token-uri: ${GOOGLE_DRIVE_TOKEN_URI:https://oauth2.googleapis.com/token}
7979

Lines changed: 245 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,245 @@
1+
package com.jobtracker.unit;
2+
3+
import com.jobtracker.config.GoogleDriveProperties;
4+
import com.jobtracker.entity.GoogleDriveBaseResume;
5+
import com.jobtracker.entity.GoogleDriveConnection;
6+
import com.jobtracker.entity.GoogleDriveOAuthState;
7+
import com.jobtracker.entity.User;
8+
import com.jobtracker.repository.GoogleDriveConnectionRepository;
9+
import com.jobtracker.repository.GoogleDriveOAuthStateRepository;
10+
import com.jobtracker.service.GoogleDriveApiClient;
11+
import com.jobtracker.service.GoogleDriveOAuthService;
12+
import com.jobtracker.util.SecurityUtils;
13+
import org.junit.jupiter.api.BeforeEach;
14+
import org.junit.jupiter.api.Test;
15+
import org.junit.jupiter.api.extension.ExtendWith;
16+
import org.mockito.ArgumentCaptor;
17+
import org.mockito.Mock;
18+
import org.mockito.junit.jupiter.MockitoExtension;
19+
20+
import java.time.LocalDateTime;
21+
import java.util.ArrayList;
22+
import java.util.List;
23+
import java.util.Optional;
24+
import java.util.UUID;
25+
26+
import static org.assertj.core.api.Assertions.assertThat;
27+
import static org.mockito.ArgumentMatchers.any;
28+
import static org.mockito.Mockito.*;
29+
30+
@ExtendWith(MockitoExtension.class)
31+
class GoogleDriveOAuthServiceTest {
32+
33+
private static final UUID USER_ID = UUID.fromString("00000000-0000-0000-0000-000000000001");
34+
private static final String VALID_STATE = "valid-state-token";
35+
private static final String AUTH_CODE = "auth-code-123";
36+
37+
@Mock private GoogleDriveApiClient googleDriveApiClient;
38+
@Mock private GoogleDriveConnectionRepository connectionRepository;
39+
@Mock private GoogleDriveOAuthStateRepository oauthStateRepository;
40+
@Mock private SecurityUtils securityUtils;
41+
42+
private GoogleDriveProperties googleDriveProperties;
43+
private GoogleDriveOAuthService oauthService;
44+
45+
private User user;
46+
private GoogleDriveOAuthState validOAuthState;
47+
48+
@BeforeEach
49+
void setUp() {
50+
googleDriveProperties = new GoogleDriveProperties(
51+
"client-id",
52+
"client-secret",
53+
"http://localhost:8080/api/v1/google-drive/oauth/callback",
54+
"http://localhost:5173/settings/google-drive/callback",
55+
"https://accounts.google.com/o/oauth2/v2/auth",
56+
"https://oauth2.googleapis.com/token"
57+
);
58+
oauthService = new GoogleDriveOAuthService(
59+
googleDriveApiClient,
60+
googleDriveProperties,
61+
connectionRepository,
62+
oauthStateRepository,
63+
securityUtils
64+
);
65+
66+
user = new User();
67+
user.setId(USER_ID);
68+
user.setEmail("user@example.com");
69+
70+
validOAuthState = new GoogleDriveOAuthState();
71+
validOAuthState.setUser(user);
72+
validOAuthState.setStateToken(VALID_STATE);
73+
validOAuthState.setExpiresAt(LocalDateTime.now().plusMinutes(5));
74+
}
75+
76+
@Test
77+
void handleCallback_shouldRedirectWithError_whenGoogleReturnsError() {
78+
String result = oauthService.handleCallback(VALID_STATE, null, "access_denied");
79+
80+
assertThat(result).contains("status=error");
81+
assertThat(result).contains("Google");
82+
assertThat(result).contains("OAuth");
83+
verifyNoInteractions(googleDriveApiClient);
84+
verifyNoInteractions(connectionRepository);
85+
}
86+
87+
@Test
88+
void handleCallback_shouldRedirectWithError_whenStateIsExpired() {
89+
validOAuthState.setExpiresAt(LocalDateTime.now().minusMinutes(1));
90+
when(oauthStateRepository.findByStateToken(VALID_STATE)).thenReturn(Optional.of(validOAuthState));
91+
92+
String result = oauthService.handleCallback(VALID_STATE, AUTH_CODE, null);
93+
94+
assertThat(result).contains("status=error");
95+
assertThat(result).containsIgnoringCase("expired");
96+
verifyNoInteractions(googleDriveApiClient);
97+
verifyNoInteractions(connectionRepository);
98+
}
99+
100+
@Test
101+
void handleCallback_shouldRedirectWithError_whenStateIsInvalid() {
102+
when(oauthStateRepository.findByStateToken("unknown-state")).thenReturn(Optional.empty());
103+
104+
String result = oauthService.handleCallback("unknown-state", AUTH_CODE, null);
105+
106+
assertThat(result).contains("status=error");
107+
assertThat(result).contains("Invalid");
108+
verifyNoInteractions(googleDriveApiClient);
109+
verifyNoInteractions(connectionRepository);
110+
}
111+
112+
@Test
113+
void handleCallback_shouldRedirectWithError_whenMissingState() {
114+
String result = oauthService.handleCallback(null, AUTH_CODE, null);
115+
116+
assertThat(result).contains("status=error");
117+
assertThat(result).contains("Missing");
118+
verifyNoInteractions(googleDriveApiClient);
119+
verifyNoInteractions(connectionRepository);
120+
}
121+
122+
@Test
123+
void handleCallback_shouldRedirectWithError_whenMissingCode() {
124+
String result = oauthService.handleCallback(VALID_STATE, null, null);
125+
126+
assertThat(result).contains("status=error");
127+
assertThat(result).contains("Missing");
128+
verifyNoInteractions(googleDriveApiClient);
129+
verifyNoInteractions(connectionRepository);
130+
}
131+
132+
@Test
133+
void handleCallback_shouldCreateNewConnection_onFirstConnect() {
134+
when(oauthStateRepository.findByStateToken(VALID_STATE)).thenReturn(Optional.of(validOAuthState));
135+
when(googleDriveApiClient.exchangeAuthorizationCode(AUTH_CODE)).thenReturn(
136+
new GoogleDriveApiClient.OAuthTokens("access-token", "refresh-token",
137+
LocalDateTime.now().plusHours(1), "https://www.googleapis.com/auth/drive")
138+
);
139+
when(googleDriveApiClient.getCurrentAccount("access-token")).thenReturn(
140+
new GoogleDriveApiClient.GoogleDriveAccountProfile("perm-1", "user@gmail.com", "User")
141+
);
142+
when(connectionRepository.findByUserId(USER_ID)).thenReturn(Optional.empty());
143+
when(connectionRepository.save(any(GoogleDriveConnection.class))).thenAnswer(inv -> inv.getArgument(0));
144+
145+
String result = oauthService.handleCallback(VALID_STATE, AUTH_CODE, null);
146+
147+
assertThat(result).contains("status=success");
148+
ArgumentCaptor<GoogleDriveConnection> captor = ArgumentCaptor.forClass(GoogleDriveConnection.class);
149+
verify(connectionRepository).save(captor.capture());
150+
GoogleDriveConnection saved = captor.getValue();
151+
assertThat(saved.getGoogleEmail()).isEqualTo("user@gmail.com");
152+
assertThat(saved.getRefreshToken()).isEqualTo("refresh-token");
153+
verify(oauthStateRepository).delete(validOAuthState);
154+
}
155+
156+
@Test
157+
void handleCallback_shouldClearRootFolderAndBaseResumes_whenDifferentAccountConnects() {
158+
GoogleDriveConnection existingConnection = new GoogleDriveConnection();
159+
existingConnection.setGoogleAccountId("old-account-id");
160+
existingConnection.setRootFolderId("old-root-folder");
161+
existingConnection.setRootFolderName("Old Root");
162+
existingConnection.setBaseResumes(new ArrayList<>(List.of(new GoogleDriveBaseResume())));
163+
164+
when(oauthStateRepository.findByStateToken(VALID_STATE)).thenReturn(Optional.of(validOAuthState));
165+
when(googleDriveApiClient.exchangeAuthorizationCode(AUTH_CODE)).thenReturn(
166+
new GoogleDriveApiClient.OAuthTokens("new-access", "new-refresh",
167+
LocalDateTime.now().plusHours(1), "https://www.googleapis.com/auth/drive")
168+
);
169+
when(googleDriveApiClient.getCurrentAccount("new-access")).thenReturn(
170+
new GoogleDriveApiClient.GoogleDriveAccountProfile("new-account-id", "new@gmail.com", "New User")
171+
);
172+
when(connectionRepository.findByUserId(USER_ID)).thenReturn(Optional.of(existingConnection));
173+
when(connectionRepository.save(any(GoogleDriveConnection.class))).thenAnswer(inv -> inv.getArgument(0));
174+
175+
String result = oauthService.handleCallback(VALID_STATE, AUTH_CODE, null);
176+
177+
assertThat(result).contains("status=success");
178+
ArgumentCaptor<GoogleDriveConnection> captor = ArgumentCaptor.forClass(GoogleDriveConnection.class);
179+
verify(connectionRepository).save(captor.capture());
180+
GoogleDriveConnection saved = captor.getValue();
181+
assertThat(saved.getRootFolderId()).isNull();
182+
assertThat(saved.getRootFolderName()).isNull();
183+
assertThat(saved.getBaseResumes()).isEmpty();
184+
assertThat(saved.getGoogleEmail()).isEqualTo("new@gmail.com");
185+
}
186+
187+
@Test
188+
void handleCallback_shouldPreserveRootFolder_whenSameAccountReconnects() {
189+
GoogleDriveConnection existingConnection = new GoogleDriveConnection();
190+
existingConnection.setGoogleAccountId("same-account-id");
191+
existingConnection.setRootFolderId("existing-root-folder");
192+
existingConnection.setRootFolderName("Job Tracker Root");
193+
existingConnection.setBaseResumes(new ArrayList<>());
194+
195+
when(oauthStateRepository.findByStateToken(VALID_STATE)).thenReturn(Optional.of(validOAuthState));
196+
when(googleDriveApiClient.exchangeAuthorizationCode(AUTH_CODE)).thenReturn(
197+
new GoogleDriveApiClient.OAuthTokens("new-access", "new-refresh",
198+
LocalDateTime.now().plusHours(1), "https://www.googleapis.com/auth/drive")
199+
);
200+
when(googleDriveApiClient.getCurrentAccount("new-access")).thenReturn(
201+
new GoogleDriveApiClient.GoogleDriveAccountProfile("same-account-id", "user@gmail.com", "User")
202+
);
203+
when(connectionRepository.findByUserId(USER_ID)).thenReturn(Optional.of(existingConnection));
204+
when(connectionRepository.save(any(GoogleDriveConnection.class))).thenAnswer(inv -> inv.getArgument(0));
205+
206+
String result = oauthService.handleCallback(VALID_STATE, AUTH_CODE, null);
207+
208+
assertThat(result).contains("status=success");
209+
ArgumentCaptor<GoogleDriveConnection> captor = ArgumentCaptor.forClass(GoogleDriveConnection.class);
210+
verify(connectionRepository).save(captor.capture());
211+
GoogleDriveConnection saved = captor.getValue();
212+
assertThat(saved.getRootFolderId()).isEqualTo("existing-root-folder");
213+
}
214+
215+
@Test
216+
void handleCallback_shouldDeleteStateEvenOnError() {
217+
validOAuthState.setExpiresAt(LocalDateTime.now().minusMinutes(1));
218+
when(oauthStateRepository.findByStateToken(VALID_STATE)).thenReturn(Optional.of(validOAuthState));
219+
220+
oauthService.handleCallback(VALID_STATE, AUTH_CODE, null);
221+
222+
verify(oauthStateRepository).delete(validOAuthState);
223+
}
224+
225+
@Test
226+
void disconnect_shouldDeleteExistingConnection() {
227+
GoogleDriveConnection connection = new GoogleDriveConnection();
228+
when(securityUtils.getCurrentUserId()).thenReturn(USER_ID);
229+
when(connectionRepository.findByUserId(USER_ID)).thenReturn(Optional.of(connection));
230+
231+
oauthService.disconnect();
232+
233+
verify(connectionRepository).delete(connection);
234+
}
235+
236+
@Test
237+
void disconnect_shouldSucceed_whenNoConnectionExists() {
238+
when(securityUtils.getCurrentUserId()).thenReturn(USER_ID);
239+
when(connectionRepository.findByUserId(USER_ID)).thenReturn(Optional.empty());
240+
241+
oauthService.disconnect();
242+
243+
verify(connectionRepository, never()).delete(any());
244+
}
245+
}

src/test/java/com/jobtracker/unit/GoogleDriveServiceTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ void copyBaseResumeToApplication_shouldCreateFolderAndCopyDocument() {
162162
connection.setRootFolderId("root-folder-id");
163163
when(securityUtils.getCurrentUserId()).thenReturn(USER_ID);
164164
when(connectionRepository.findByUserId(USER_ID)).thenReturn(Optional.of(connection));
165-
when(applicationRepository.findByIdAndUserId(APPLICATION_ID, USER_ID)).thenReturn(Optional.of(application));
165+
when(applicationRepository.findByIdAndUserIdForUpdate(APPLICATION_ID, USER_ID)).thenReturn(Optional.of(application));
166166
when(baseResumeRepository.findByIdAndConnectionUserId(RESUME_ID, USER_ID)).thenReturn(Optional.of(baseResume));
167167
when(googleDriveApiClient.getFileMetadata("access-token", "root-folder-id"))
168168
.thenReturn(new GoogleDriveApiClient.DriveFileMetadata(

0 commit comments

Comments
 (0)