Merge pull request #246 from bannergress/revert-retry-verification-an…

…d-bypass-cache

Revert retry verification and bypass cache

build.gradle

@@ -24,8 +24,6 @@ dependencies {
 	implementation 'org.springframework.boot:spring-boot-starter-web'
 	implementation 'org.springframework.boot:spring-boot-starter-security'
 	implementation 'org.springframework.boot:spring-boot-starter-validation'
-	implementation 'org.springframework.boot:spring-boot-starter-aop'
-	implementation 'org.springframework.retry:spring-retry'
 	implementation 'org.keycloak:keycloak-spring-boot-starter'
 	implementation "org.hibernate:hibernate-envers"
   implementation 'org.hibernate:hibernate-spatial'

src/main/java/com/bannergress/backend/BannergressBackendApplication.java

@@ -3,13 +3,11 @@
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.boot.builder.SpringApplicationBuilder;
-import org.springframework.retry.annotation.EnableRetry;
 
 /**
  * Main application.
  */
 @SpringBootApplication
-@EnableRetry
 public class BannergressBackendApplication {
     public static void main(String[] args) {
         SpringApplication application = new SpringApplicationBuilder(BannergressBackendApplication.class).build();

src/main/java/com/bannergress/backend/controllers/UserController.java

@@ -2,7 +2,6 @@
 
 import com.bannergress.backend.dto.UserDto;
 import com.bannergress.backend.entities.User;
-import com.bannergress.backend.exceptions.VerificationFailedException;
 import com.bannergress.backend.exceptions.VerificationStateException;
 import com.bannergress.backend.services.AgentService;
 import com.bannergress.backend.services.UserMappingService;
@@ -57,7 +56,7 @@ public UserDto clearClaim(Principal principal) {
     }
 
     @PostMapping("/user/verify")
-    public UserDto verify(Principal principal) throws VerificationStateException, VerificationFailedException {
+    public UserDto verify(Principal principal) throws VerificationStateException {
         String userId = principal.getName();
         userService.verify(userId);
         return get(principal);

src/main/java/com/bannergress/backend/exceptions/VerificationStateException.java

@@ -4,7 +4,7 @@
 import org.springframework.web.bind.annotation.ResponseStatus;
 
 /**
- * Exception for when a verification fails because the user has not started the verification process yet.
+ * Exception for when a verification fails.
  */
 @ResponseStatus(HttpStatus.PRECONDITION_FAILED)
 public class VerificationStateException extends Exception {

src/main/java/com/bannergress/backend/services/UserService.java

@@ -1,9 +1,10 @@
 package com.bannergress.backend.services;
 
 import com.bannergress.backend.entities.User;
-import com.bannergress.backend.exceptions.VerificationFailedException;
 import com.bannergress.backend.exceptions.VerificationStateException;
 
+import java.util.Optional;
+
 /**
  * Service for user-related tasks.
  */
@@ -28,10 +29,10 @@ public interface UserService {
      * Verifies a user.
      *
      * @param userId user to verify.
+     * @return If the verification was successful: agent name, corrected for case sensitivity.
      * @throws VerificationStateException If the verification process hasn't been started.
-     * @throws VerificationFailedException If the verification process fails.
      */
-    public void verify(String userId) throws VerificationStateException, VerificationFailedException;
+    public Optional<String> verify(String userId) throws VerificationStateException;
 
     /**
      * Unlinks a user from an agent name.

src/main/java/com/bannergress/backend/services/VerificationService.java

@@ -1,7 +1,6 @@
 package com.bannergress.backend.services;
 
-import com.bannergress.backend.exceptions.VerificationFailedException;
-
+import java.util.Optional;
 import java.util.UUID;
 
 /**
@@ -13,8 +12,7 @@ public interface VerificationService {
      *
      * @param agent             Agent name to verify control.
      * @param verificationToken Token for agent verification.
-     * @return agent name, corrected for case sensitivity.
-     * @throws VerificationFailedException If the control over the agent name could not be established.
+     * @return If the verification was successful: agent name, corrected for case sensitivity.
      */
-    String verify(String agent, UUID verificationToken) throws VerificationFailedException;
+    Optional<String> verify(String agent, UUID verificationToken);
 }

src/main/java/com/bannergress/backend/services/impl/CommunityForumVerificationServiceImpl.java

@@ -1,16 +1,14 @@
 package com.bannergress.backend.services.impl;
 
 import com.bannergress.backend.dto.RSS;
-import com.bannergress.backend.exceptions.VerificationFailedException;
 import com.bannergress.backend.services.VerificationService;
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.dataformat.xml.XmlMapper;
 import okhttp3.OkHttpClient;
 import okhttp3.Request;
 import okhttp3.Response;
 import org.springframework.beans.factory.annotation.Value;
-import org.springframework.retry.annotation.Backoff;
-import org.springframework.retry.annotation.Retryable;
+import org.springframework.http.HttpHeaders;
 import org.springframework.stereotype.Service;
 
 import java.io.IOException;
@@ -23,21 +21,24 @@ public class CommunityForumVerificationServiceImpl implements VerificationServic
 
     private final String url;
 
+    private final String cacheBypassCookieHeader;
+
     public CommunityForumVerificationServiceImpl(
-        @Value(value = "${verification.url:https://community.ingress.com/en/activity/feed.rss}") String url) {
+        @Value(value = "${verification.url:https://community.ingress.com/en/activity/feed.rss}") String url,
+        @Value(value = "${verification.cookie:vfo_s=dummy}") String cacheBypassCookieHeader) {
         this.client = new OkHttpClient.Builder().cache(null).build();
         this.url = url;
+        this.cacheBypassCookieHeader = cacheBypassCookieHeader;
     }
 
     @Override
-    @Retryable(include = VerificationFailedException.class, maxAttemptsExpression = "${verification.maxAttempts:3}", backoff = @Backoff(delayExpression = "${verification.backoff:60000}"))
-    public String verify(String agent, UUID verificationToken) throws VerificationFailedException {
+    public Optional<String> verify(String agent, UUID verificationToken) {
         RSS rss = loadRssFeed();
-        return verify(agent, verificationToken, rss).orElseThrow(VerificationFailedException::new);
+        return verify(agent, verificationToken, rss);
     }
 
     private RSS loadRssFeed() {
-        Request request = new Request.Builder().url(url).build();
+        Request request = new Request.Builder().url(url).header(HttpHeaders.COOKIE, cacheBypassCookieHeader).build();
         try (Response response = client.newCall(request).execute()) {
             XmlMapper xmlMapper = new XmlMapper();
             xmlMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);

src/main/java/com/bannergress/backend/services/impl/UserServiceImpl.java

@@ -1,7 +1,6 @@
 package com.bannergress.backend.services.impl;
 
 import com.bannergress.backend.entities.User;
-import com.bannergress.backend.exceptions.VerificationFailedException;
 import com.bannergress.backend.exceptions.VerificationStateException;
 import com.bannergress.backend.repositories.UserRepository;
 import com.bannergress.backend.services.UserMappingService;
@@ -45,14 +44,20 @@ public void claim(String userId, String agent) {
     }
 
     @Override
-    public void verify(String userId) throws VerificationStateException, VerificationFailedException {
+    public Optional<String> verify(String userId) throws VerificationStateException {
         User user = getOrCreate(userId);
         if (user.getVerificationAgent() == null || user.getVerificationToken() == null) {
             throw new VerificationStateException();
         }
-        String agentName = verificationService.verify(user.getVerificationAgent(), user.getVerificationToken());
-        userMappingService.setAgentName(userId, agentName);
-        clearClaim(userId);
+        Optional<String> agentName = verificationService.verify(user.getVerificationAgent(),
+            user.getVerificationToken());
+        if (agentName.isPresent()) {
+            userMappingService.setAgentName(userId, agentName.get());
+            clearClaim(userId);
+        } else {
+            throw new VerificationStateException();
+        }
+        return agentName;
     }
 
     @Override

src/test/java/com/bannergress/backend/services/impl/TestVerificationServiceImpl.java

@@ -1,19 +1,19 @@
 package com.bannergress.backend.services.impl;
 
-import com.bannergress.backend.exceptions.VerificationFailedException;
 import com.bannergress.backend.services.VerificationService;
 import org.junit.jupiter.api.Test;
 
+import java.util.Optional;
 import java.util.UUID;
 
-import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.assertj.core.api.Assertions.assertThat;
 
 public class TestVerificationServiceImpl {
     @Test
     public void testCompletion() {
         VerificationService verificationService = new CommunityForumVerificationServiceImpl(
-            "https://community.ingress.com/en/activity/feed.rss");
-        assertThatExceptionOfType(VerificationFailedException.class)
-            .isThrownBy(() -> verificationService.verify("someone", UUID.randomUUID()));
+            "https://community.ingress.com/en/activity/feed.rss", "vfo_s=dummy");
+        Optional<String> agent = verificationService.verify("someone", UUID.randomUUID());
+        assertThat(agent).isEmpty();
     }
 }