fix(readOnly): fixes read only mode for update queries (#15398)

metadata-io/src/main/java/com/linkedin/metadata/entity/ebean/EbeanAspectDao.java

@@ -194,7 +194,7 @@ public class EbeanAspectDao implements AspectDao, AspectMigrationsDao {
             .collect(Collectors.toList());
 
     final List<EbeanAspectV2> results;
-    if (forUpdate) {
+    if (forUpdate && canWrite) {
       results = server.find(EbeanAspectV2.class).where().idIn(keys).forUpdate().findList();
     } else {
       results = server.find(EbeanAspectV2.class).where().idIn(keys).findList();
@@ -425,7 +425,7 @@ public class EbeanAspectDao implements AspectDao, AspectMigrationsDao {
     }
 
     // Add FOR UPDATE clause only once at the end of the entire statement
-    if (forUpdate) {
+    if (forUpdate && canWrite) {
       sb.append(" FOR UPDATE");
     }
 
@@ -484,7 +484,7 @@ public class EbeanAspectDao implements AspectDao, AspectMigrationsDao {
 
     sb.append(")");
 
-    if (forUpdate) {
+    if (forUpdate && canWrite) {
       sb.append(" FOR UPDATE");
     }
 
@@ -891,7 +891,9 @@ public class EbeanAspectDao implements AspectDao, AspectMigrationsDao {
 
     // forUpdate is required to avoid duplicate key violations (it is used as an indication that the
     // max(version) was invalidated
-    server.find(EbeanAspectV2.class).where().idIn(forUpdateKeys).forUpdate().findList();
+    if (canWrite) {
+      server.find(EbeanAspectV2.class).where().idIn(forUpdateKeys).forUpdate().findList();
+    }
 
     Junction<EbeanAspectV2> queryJunction =
         server

metadata-io/src/test/java/com/linkedin/metadata/entity/ebean/EbeanAspectDaoTest.java

@@ -61,8 +61,17 @@ public class EbeanAspectDaoTest {
     EbeanTestUtils.shutdownDatabase(server);
   }
 
-  @Test
+  @DataProvider(name = "writabilityConfig")
-  public void testGetNextVersionForUpdate() {
+  public Object[][] writabilityConfigProvider() {
+    return new Object[][] {
+      {true, "Writable"}, // canWrite = true, description
+      {false, "ReadOnly"} // canWrite = false, description
+    };
+  }
+
+  @Test(dataProvider = "writabilityConfig")
+  public void testGetNextVersionForUpdate(boolean canWrite, String description) {
+    testDao.setWritable(canWrite);
     LoggedSql.start();
 
     testDao.runInTransactionWithRetryUnlocked(
@@ -79,13 +88,22 @@ public class EbeanAspectDaoTest {
         LoggedSql.stop().stream()
             .filter(str -> str.contains("testGetNextVersionForUpdate"))
             .toList();
-    assertEquals(sql.size(), 2, String.format("Found: %s", sql));
+    if (canWrite) {
-    assertTrue(
+      assertEquals(sql.size(), 2, String.format("Found: %s", sql));
-        sql.get(0).contains("for update;"), String.format("Did not find `for update` in %s ", sql));
+      assertTrue(
+          sql.get(0).contains("for update;"),
+          String.format("Did not find `for update` in %s ", sql));
+    } else {
+      assertEquals(sql.size(), 1, String.format("Found: %s", sql));
+      assertFalse(
+          sql.get(0).contains("for update;"), String.format("Found `for update` in %s ", sql));
+    }
   }
 
-  @Test
+  @Test(dataProvider = "writabilityConfig")
-  public void testGetLatestAspectsForUpdate() throws JsonProcessingException {
+  public void testGetLatestAspectsForUpdate(boolean canWrite, String description)
+      throws JsonProcessingException {
+    testDao.setWritable(canWrite);
     LoggedSql.start();
 
     testDao.runInTransactionWithRetryUnlocked(
@@ -106,12 +124,20 @@ public class EbeanAspectDaoTest {
             .toList();
     assertEquals(
         sql.size(), 1, String.format("Found: %s", new ObjectMapper().writeValueAsString(sql)));
-    assertTrue(
+    if (canWrite) {
-        sql.get(0).contains("for update;"), String.format("Did not find `for update` in %s ", sql));
+      assertTrue(
+          sql.get(0).contains("for update;"),
+          String.format("Did not find `for update` in %s ", sql));
+    } else {
+      assertFalse(
+          sql.get(0).contains("for update;"), String.format("Found `for update` in %s ", sql));
+    }
   }
 
-  @Test
+  @Test(dataProvider = "writabilityConfig")
-  public void testbatchGetForUpdate() throws JsonProcessingException {
+  public void testbatchGetForUpdate(boolean canWrite, String description)
+      throws JsonProcessingException {
+    testDao.setWritable(canWrite);
     LoggedSql.start();
 
     testDao.runInTransactionWithRetryUnlocked(
@@ -142,8 +168,14 @@ public class EbeanAspectDaoTest {
             .toList();
     assertEquals(
         sql.size(), 1, String.format("Found: %s", new ObjectMapper().writeValueAsString(sql)));
-    assertTrue(
+    if (canWrite) {
-        sql.get(0).contains("FOR UPDATE;"), String.format("Did not find `for update` in %s ", sql));
+      assertTrue(
+          sql.get(0).contains("FOR UPDATE;"),
+          String.format("Did not find `for update` in %s ", sql));
+    } else {
+      assertFalse(
+          sql.get(0).contains("FOR UPDATE;"), String.format("Found `for update` in %s ", sql));
+    }
   }
 
   @Test
@@ -217,14 +249,6 @@ public class EbeanAspectDaoTest {
         count3, 2, "Should return count of aspects matching both URN pattern and aspect name");
   }
 
-  @DataProvider(name = "writabilityConfig")
-  public Object[][] writabilityConfigProvider() {
-    return new Object[][] {
-      {true, "Writable"}, // canWrite = true, description
-      {false, "ReadOnly"} // canWrite = false, description
-    };
-  }
-
   @Test(dataProvider = "writabilityConfig")
   public void testUpdateAspectWithWritability(boolean canWrite, String description) {
     // Set writability