🧵 🐛 one update at a time for user applications

User applications go across two resources: - an issue for the application - the user record If two requests happen at the same time, issue references can get dropped/missed, and content updates can be lost. Force this one path to single file for the user. This is especially important at create time.
commonhaus · Jun 10, 2024 · cf70916 · cf70916
1 parent 31f7e2e
commit cf70916
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 23 deletions.
diff --git a/cf-admin-bot/src/main/java/org/commonhaus/automation/admin/AdminDataCache.java b/cf-admin-bot/src/main/java/org/commonhaus/automation/admin/AdminDataCache.java
@@ -18,7 +18,9 @@ public enum AdminDataCache {
 
     KNOWN_USER(b -> b.expireAfterWrite(2, TimeUnit.DAYS)),
 
-    ALIASES(b -> b.expireAfterWrite(6, TimeUnit.HOURS));
+    ALIASES(b -> b.expireAfterWrite(6, TimeUnit.HOURS)),
+
+    APPLICATION_CHECK(b -> b.expireAfterWrite(1, TimeUnit.HOURS));
 
     private QueryCache cache = null;
 

diff --git a/...dmin-bot/src/main/java/org/commonhaus/automation/admin/api/MemberApplicationResource.java b/...dmin-bot/src/main/java/org/commonhaus/automation/admin/api/MemberApplicationResource.java
@@ -1,5 +1,7 @@
 package org.commonhaus.automation.admin.api;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
 import jakarta.ws.rs.GET;
@@ -8,6 +10,7 @@
 import jakarta.ws.rs.Produces;
 import jakarta.ws.rs.core.Response;
 
+import org.commonhaus.automation.admin.AdminDataCache;
 import org.commonhaus.automation.admin.api.ApiResponse.Type;
 import org.commonhaus.automation.admin.api.ApplicationData.ApplicationPost;
 import org.commonhaus.automation.admin.api.CommonhausUser.MemberStatus;
@@ -17,6 +20,7 @@
 
 import io.quarkus.logging.Log;
 import io.quarkus.security.Authenticated;
+import io.smallrye.common.annotation.Blocking;
 
 @Path("/member/apply")
 @Authenticated
@@ -36,36 +40,34 @@ public class MemberApplicationResource {
     @Produces("application/json")
     public Response getApplication() {
         try {
-            CommonhausUser user = datastore.getCommonhausUser(session, false);
-            MembershipApplication application = user.application();
-            if (application == null) {
-                return Response.status(Response.Status.NOT_FOUND).build();
-            }
-            ApplicationData applicationData = ctx.getOpenApplication(session, application.nodeId());
-            if (applicationData == null) {
-                user.application = null;
-                user = datastore.setCommonhausUser(user, session.roles(),
-                        "Remove membership application (not found)", false);
+            CommonhausUser user = datastore.getCommonhausUser(session, false, false);
+            if (user == null) {
                 return Response.status(Response.Status.NOT_FOUND).build();
             }
-            if (!applicationData.isOwner()) {
-                user.application = null;
-                user = datastore.setCommonhausUser(user, session.roles(),
-                        "Remove membership application (not owner)", false);
+            MembershipApplication application = user.application();
+            ApplicationData applicationData = application == null
+                    ? null
+                    : ctx.getOpenApplication(session, application.nodeId());
+
+            if (applicationData == null || !applicationData.isOwner()) {
+                if (application != null) {
+                    user.application = null;
+                    String state = applicationData == null ? "not found" : "not owner";
+                    updateStatus(user, "Remove membership application (" + state + ")", false);
+                }
                 return Response.status(Response.Status.NOT_FOUND).build();
             }
+
             if (user.status().updateToPending()) {
                 // compensate for missing status
                 user.status(MemberStatus.PENDING);
-                user = datastore.setCommonhausUser(user, session.roles(), "Set status to PENDING", true);
+                user = updateStatus(user, "Set status to PENDING", true);
             }
             return user.toResponse()
                     .setData(Type.APPLY, applicationData)
                     .finish();
         } catch (Throwable e) {
-            if (Log.isDebugEnabled()) {
-                e.printStackTrace();
-            }
+            Log.errorf(e, "getApplication: Unable to retrieve application for %s: %s", session.login(), e);
             return Response.status(Response.Status.INTERNAL_SERVER_ERROR).build();
         }
     }
@@ -74,8 +76,38 @@ public Response getApplication() {
     @KnownUser
     @Produces("application/json")
     public Response setApplication(ApplicationPost applicationPost) {
+        AtomicBoolean checkRunning = AdminDataCache.APPLICATION_CHECK.computeIfAbsent(session.login(),
+                (k) -> new AtomicBoolean(false));
+
+        // There are a few separate updates here (dealing with the issue AND (separately) with the user record).
+        // We'll enforce one at a time coming from the UI for this one.
+        return checkRunning.compareAndSet(false, true)
+                ? doApplicationUpdate(applicationPost, checkRunning)
+                : Response.status(Response.Status.TOO_MANY_REQUESTS).build();
+    }
+
+    @Blocking
+    private CommonhausUser updateStatus(CommonhausUser user, String message, boolean history) {
+        AtomicBoolean checkRunning = AdminDataCache.APPLICATION_CHECK.computeIfAbsent(session.login(),
+                (k) -> new AtomicBoolean(false));
+        boolean iAmWriter = checkRunning.compareAndSet(false, true);
+        if (iAmWriter) {
+            try {
+                return datastore.setCommonhausUser(user, session.roles(), message, history);
+            } finally {
+                checkRunning.set(false);
+            }
+        }
+        return user;
+    }
+
+    @Blocking
+    private Response doApplicationUpdate(ApplicationPost applicationPost, AtomicBoolean checkRunning) {
         try {
-            CommonhausUser user = datastore.getCommonhausUser(session, false);
+            CommonhausUser user = datastore.getCommonhausUser(session, false, false);
+            if (user == null) {
+                return Response.status(Response.Status.NOT_FOUND).build();
+            }
             ApplicationData applicationData = ctx.updateApplication(session, user, applicationPost);
             if (applicationData == null) {
                 return Response.status(Response.Status.BAD_REQUEST).build();
@@ -97,10 +129,10 @@ public Response setApplication(ApplicationPost applicationPost) {
                     .setData(Type.APPLY, applicationData)
                     .finish();
         } catch (Throwable e) {
-            if (Log.isDebugEnabled()) {
-                e.printStackTrace();
-            }
+            Log.errorf(e, "doApplicationUpdate: Unable to update user application for %s: %s", session.login(), e);
             return Response.status(Response.Status.INTERNAL_SERVER_ERROR).build();
+        } finally {
+            checkRunning.set(false);
         }
     }
 }