Fix duplicate entries in records

Tristan-WorkGH · Tristan-WorkGH · commit ebfa248a3051 · 2024-01-09T19:31:23.000+01:00
diff --git a/src/main/java/org/gridsuite/useradmin/server/UserAdminController.java b/src/main/java/org/gridsuite/useradmin/server/UserAdminController.java
@@ -72,4 +72,12 @@ public ResponseEntity<Void> userExists(@PathVariable("sub") String sub) {
     public ResponseEntity<List<ConnectionEntity>> getConnections(@RequestHeader("userId") String userId) {
         return ResponseEntity.ok().contentType(MediaType.APPLICATION_JSON).body(service.getConnections(userId));
     }
+
+    @Deprecated(forRemoval = true)
+    @GetMapping(value = "/connections/deduplicate")
+    @Operation(summary = "get the connections deduplicated", deprecated = true)
+    @ApiResponse(responseCode = "200", description = "The connections list")
+    public ResponseEntity<List<ConnectionEntity>> deduplicateConnections(@RequestHeader("userId") String userId) {
+        return ResponseEntity.ok().contentType(MediaType.APPLICATION_JSON).body(service.getDeduplicatedConnections(userId));
+    }
 }
diff --git a/src/main/java/org/gridsuite/useradmin/server/repository/AbstractUserEntity.java b/src/main/java/org/gridsuite/useradmin/server/repository/AbstractUserEntity.java
@@ -0,0 +1,37 @@
+/**
+ * Copyright (c) 2022, RTE (http://www.rte-france.com)
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+package org.gridsuite.useradmin.server.repository;
+
+import jakarta.persistence.*;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+import lombok.Setter;
+
+import java.util.UUID;
+
+/**
+ * @author Etienne Homer <etienne.homer at rte-france.com>
+ */
+@NoArgsConstructor
+@AllArgsConstructor
+@Getter
+@Setter
+@MappedSuperclass
+abstract class AbstractUserEntity {
+    public AbstractUserEntity(String sub) {
+        this(UUID.randomUUID(), sub);
+    }
+
+    @Id
+    @GeneratedValue(strategy = GenerationType.AUTO)
+    @Column(name = "id")
+    private UUID id;
+
+    @Column(name = "sub", nullable = false, unique = true)
+    private String sub;
+}
diff --git a/src/main/java/org/gridsuite/useradmin/server/repository/ConnectionEntity.java b/src/main/java/org/gridsuite/useradmin/server/repository/ConnectionEntity.java
@@ -6,14 +6,16 @@
  */
 package org.gridsuite.useradmin.server.repository;
 
+import jakarta.persistence.Column;
+import jakarta.persistence.Entity;
+import jakarta.persistence.Index;
+import jakarta.persistence.Table;
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
 import lombok.Setter;
 
-import jakarta.persistence.*;
 import java.time.LocalDateTime;
-import java.util.UUID;
 
 /**
  * @author Etienne Homer <etienne.homer at rte-france.com>
@@ -24,15 +26,7 @@
 @Setter
 @Entity
 @Table(name = "connection", indexes = {@Index(name = "connection_sub_index", columnList = "sub")})
-public class ConnectionEntity {
-
-    @Id
-    @Column(name = "id")
-    private UUID id;
-
-    @Column(name = "sub", nullable = false)
-    private String sub;
-
+public class ConnectionEntity extends AbstractUserEntity {
     @Column(name = "firstConnexionDate", nullable = false)
     private LocalDateTime firstConnexionDate;
 
@@ -43,6 +37,9 @@ public class ConnectionEntity {
     private Boolean connectionAccepted;
 
     public ConnectionEntity(String sub, LocalDateTime firstConnexionDate, LocalDateTime lastConnexionDate, Boolean connectionAccepted) {
-        this(UUID.randomUUID(), sub, firstConnexionDate, lastConnexionDate, connectionAccepted);
+        super(sub);
+        this.firstConnexionDate = firstConnexionDate;
+        this.lastConnexionDate = lastConnexionDate;
+        this.connectionAccepted = connectionAccepted;
     }
 }
diff --git a/src/main/java/org/gridsuite/useradmin/server/repository/ConnectionRepository.java b/src/main/java/org/gridsuite/useradmin/server/repository/ConnectionRepository.java
@@ -6,16 +6,30 @@
  */
 package org.gridsuite.useradmin.server.repository;
 
+import jakarta.transaction.Transactional;
 import org.springframework.data.jpa.repository.JpaRepository;
+import org.springframework.data.jpa.repository.Modifying;
+import org.springframework.lang.NonNull;
 import org.springframework.stereotype.Repository;
 
-import java.util.List;
+import java.time.LocalDateTime;
+import java.util.Optional;
 import java.util.UUID;
 
 /**
  * @author Etienne Homer <etienne.homer at rte-france.com>
  */
 @Repository
 public interface ConnectionRepository extends JpaRepository<ConnectionEntity, UUID> {
-    List<ConnectionEntity> findBySub(String sub);
+    @NonNull
+    Optional<ConnectionEntity> findBySub/*IgnoreCase*/(@NonNull String sub);
+
+    @Transactional()
+    @Modifying
+    default void recordNewConnection(@NonNull final String sub, final boolean connectionAccepted) {
+        this.findBySub/*IgnoreCase*/(sub).ifPresentOrElse(
+            conn -> this.save(conn.setLastConnexionDate(LocalDateTime.now()).setConnectionAccepted(connectionAccepted)),
+            () -> this.save(new ConnectionEntity(sub, LocalDateTime.now(), LocalDateTime.now(), connectionAccepted))
+        );
+    }
 }
diff --git a/src/main/java/org/gridsuite/useradmin/server/repository/UserInfosEntity.java b/src/main/java/org/gridsuite/useradmin/server/repository/UserInfosEntity.java
@@ -6,33 +6,24 @@
  */
 package org.gridsuite.useradmin.server.repository;
 
-import lombok.AllArgsConstructor;
+import jakarta.persistence.Entity;
+import jakarta.persistence.Index;
+import jakarta.persistence.Table;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
 import lombok.Setter;
 
-import jakarta.persistence.*;
-import java.util.UUID;
-
 /**
  * @author Etienne Homer <etienne.homer at rte-france.com>
  */
 @NoArgsConstructor
-@AllArgsConstructor
+//@AllArgsConstructor
 @Getter
 @Setter
 @Entity
 @Table(name = "user_infos", indexes = {@Index(name = "user_infos_sub_index", columnList = "sub")})
-public class UserInfosEntity {
-
+public class UserInfosEntity extends AbstractUserEntity {
     public UserInfosEntity(String sub) {
-        this(UUID.randomUUID(), sub);
+        super(sub);
     }
-
-    @Id
-    @Column(name = "id")
-    private UUID id;
-
-    @Column(name = "sub", nullable = false)
-    private String sub;
 }
diff --git a/src/main/java/org/gridsuite/useradmin/server/repository/lombok.config b/src/main/java/org/gridsuite/useradmin/server/repository/lombok.config
@@ -0,0 +1 @@
+lombok.accessors.chain = true
diff --git a/src/main/java/org/gridsuite/useradmin/server/service/ConnectionsService.java b/src/main/java/org/gridsuite/useradmin/server/service/ConnectionsService.java
@@ -43,6 +43,7 @@ public void recordConnectionAttempt(String sub, Boolean isAllowed) {
         }
     }
 
+    @Deprecated(forRemoval = true)
     @Transactional
     public List<ConnectionEntity> removeDuplicates() {
         Map<String, List<ConnectionEntity>> connectionsBySub = connectionRepository.findAll().stream().collect(Collectors.groupingBy(ConnectionEntity::getSub));
diff --git a/src/main/java/org/gridsuite/useradmin/server/service/UserAdminService.java b/src/main/java/org/gridsuite/useradmin/server/service/UserAdminService.java
@@ -6,35 +6,34 @@
  */
 package org.gridsuite.useradmin.server.service;
 
+import lombok.RequiredArgsConstructor;
 import org.gridsuite.useradmin.server.UserAdminApplicationProps;
 import org.gridsuite.useradmin.server.UserAdminException;
 import org.gridsuite.useradmin.server.repository.ConnectionEntity;
+import org.gridsuite.useradmin.server.repository.ConnectionRepository;
 import org.gridsuite.useradmin.server.repository.UserAdminRepository;
 import org.gridsuite.useradmin.server.repository.UserInfosEntity;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 
-import java.util.*;
+import java.util.List;
+import java.util.UUID;
 
 import static org.gridsuite.useradmin.server.UserAdminException.Type.FORBIDDEN;
 
 /**
  * @author Etienne Homer <etienne.homer at rte-france.com>
  */
+@RequiredArgsConstructor
 @Service
 public class UserAdminService {
-    private UserAdminRepository userAdminRepository;
-
-    private ConnectionsService connectionsService;
+    private final UserAdminRepository userAdminRepository;
+    private final ConnectionRepository connectionRepository;
+    private final ConnectionsService connectionsService;
 
     @Autowired
     private UserAdminApplicationProps applicationProps;
 
-    public UserAdminService(UserAdminRepository userAdminRepository, ConnectionsService connectionsService) {
-        this.userAdminRepository = Objects.requireNonNull(userAdminRepository);
-        this.connectionsService = Objects.requireNonNull(connectionsService);
-    }
-
     public List<UserInfosEntity> getUsers(String userId) {
         if (!isAdmin(userId)) {
             throw new UserAdminException(FORBIDDEN);
@@ -43,6 +42,14 @@ public List<UserInfosEntity> getUsers(String userId) {
     }
 
     public List<ConnectionEntity> getConnections(String userId) {
+        if (!isAdmin(userId)) {
+            throw new UserAdminException(FORBIDDEN);
+        }
+        return connectionRepository.findAll();
+    }
+
+    @Deprecated(forRemoval = true)
+    public List<ConnectionEntity> getDeduplicatedConnections(String userId) {
         if (!isAdmin(userId)) {
             throw new UserAdminException(FORBIDDEN);
         }
@@ -65,7 +72,9 @@ public void delete(UUID id, String userId) {
     }
 
     public boolean subExists(String sub) {
-        Boolean isAllowed = applicationProps.getAdmins().isEmpty() && userAdminRepository.count() == 0 || applicationProps.getAdmins().contains(sub) || !userAdminRepository.findAllBySub(sub).isEmpty();
+        Boolean isAllowed = applicationProps.getAdmins().isEmpty() && userAdminRepository.count() == 0
+                || applicationProps.getAdmins().contains(sub)
+                || !userAdminRepository.findAllBySub(sub).isEmpty();
         connectionsService.recordConnectionAttempt(sub, isAllowed);
         return isAllowed.booleanValue();
     }
diff --git a/src/main/resources/db/changelog/changesets/changelog_20240109T161028Z.xml b/src/main/resources/db/changelog/changesets/changelog_20240109T161028Z.xml
@@ -0,0 +1,33 @@
+<?xml version="1.1" encoding="UTF-8" standalone="no"?>
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog" xmlns:ext="http://www.liquibase.org/xml/ns/dbchangelog-ext" xmlns:pro="http://www.liquibase.org/xml/ns/pro" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog-ext http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-ext.xsd http://www.liquibase.org/xml/ns/pro http://www.liquibase.org/xml/ns/pro/liquibase-pro-latest.xsd http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd">
+    <changeSet author="chuinetri" id="1704818243021-0">
+        <comment>Deduplicate rows before adding unique constraint</comment>
+        <delete tableName="user_infos">
+            <where>id in (select id from (select row_number() over (partition by sub) as rnb, * from user_infos) subquery where rnb > 1)</where>
+        </delete>
+
+        <sql stripComments="true" dbms="!oracle">
+            update connection c1 set
+                first_connexion_date = (select min(c2.first_connexion_date) from connection c2 where c2.sub=c1.sub group by c2.sub),
+                last_connexion_date =  (select max(c2.last_connexion_date) from connection c2 where c2.sub=c1.sub group by c2.sub),
+                connection_accepted =  (select connection_accepted from connection c2 where (c2.sub, c2.last_connexion_date) in (
+                    select c3.sub, max(c3.last_connexion_date) from connection c3 where c3.sub=c1.sub group by c3.sub) limit 1);
+        </sql>
+        <sql stripComments="true" dbms="oracle">
+            --oracle use fetch instead of limit
+            update connection c1 set
+            first_connexion_date = (select min(c2.first_connexion_date) from connection c2 where c2.sub=c1.sub group by c2.sub),
+            last_connexion_date =  (select max(c2.last_connexion_date) from connection c2 where c2.sub=c1.sub group by c2.sub),
+            connection_accepted =  (select connection_accepted from connection c2 where (c2.sub, c2.last_connexion_date) in (
+                select c3.sub, max(c3.last_connexion_date) from connection c3 where c3.sub=c1.sub group by c3.sub) FETCH FIRST 1 ROW ONLY);
+        </sql>
+        <delete tableName="connection">
+            <where>id in (select id from (select row_number() over (partition by sub) as rnb, * from connection) subquery where rnb > 1)</where>
+        </delete>
+    </changeSet>
+
+    <changeSet author="chuinetri" id="1704818243021-1">
+        <addUniqueConstraint columnNames="sub" constraintName="UC_USER_INFOSSUB_COL" tableName="user_infos"/>
+        <addUniqueConstraint columnNames="sub" constraintName="UC_CONNECTIONSUB_COL" tableName="connection"/>
+    </changeSet>
+</databaseChangeLog>
diff --git a/src/main/resources/db/changelog/db.changelog-master.yaml b/src/main/resources/db/changelog/db.changelog-master.yaml
@@ -1,9 +1,10 @@
 databaseChangeLog:
-
   - include:
       file: changesets/changelog_20220921T091049Z.xml
       relativeToChangelogFile: true
-
   - include:
       file: changesets/changelog_20220930T080149Z.xml
       relativeToChangelogFile: true
+  - include:
+      file: changesets/changelog_20240109T161028Z.xml
+      relativeToChangelogFile: true
diff --git a/src/test/java/org/gridsuite/useradmin/server/UserAdminTest.java b/src/test/java/org/gridsuite/useradmin/server/UserAdminTest.java
@@ -114,16 +114,16 @@ public void testUserAdmin() throws Exception {
                 .andExpect(status().isNoContent())
                 .andReturn();
         assertEquals(3, connectionRepository.findAll().size());
-        assertTrue(connectionRepository.findBySub(USER_SUB).get(0).getConnectionAccepted());
-        assertFalse(connectionRepository.findBySub("UNKNOWN").get(0).getConnectionAccepted());
-        LocalDateTime firstConnectionDate = connectionRepository.findBySub(USER_SUB).get(0).getFirstConnexionDate();
+        assertTrue(connectionRepository.findBySub(USER_SUB).get().getConnectionAccepted());
+        assertFalse(connectionRepository.findBySub("UNKNOWN").get().getConnectionAccepted());
+        LocalDateTime firstConnectionDate = connectionRepository.findBySub(USER_SUB).get().getFirstConnexionDate();
         //firstConnectionDate and lastConnectionDate are equals cause this is the first connection for this user
-        assertTrue(firstConnectionDate.toEpochSecond(ZoneOffset.UTC) < connectionRepository.findBySub(USER_SUB).get(0).getLastConnexionDate().toEpochSecond(ZoneOffset.UTC) + 2);
+        assertTrue(firstConnectionDate.toEpochSecond(ZoneOffset.UTC) < connectionRepository.findBySub(USER_SUB).get().getLastConnexionDate().toEpochSecond(ZoneOffset.UTC) + 2);
 
         mockMvc.perform(head("/" + UserAdminApi.API_VERSION + "/users/{sub}", USER_SUB))
                 .andExpect(status().isOk())
                 .andReturn();
-        assertEquals(firstConnectionDate, connectionRepository.findBySub(USER_SUB).get(0).getFirstConnexionDate());
+        assertEquals(firstConnectionDate, connectionRepository.findBySub(USER_SUB).get().getFirstConnexionDate());
 
         mockMvc.perform(delete("/" + UserAdminApi.API_VERSION + "/users/{id}", userId)
                         .header("userId", ADMIN_USER)

Original file line number	Diff line number	Diff line change
`@@ -72,4 +72,12 @@ public ResponseEntity<Void> userExists(@PathVariable("sub") String sub) {`
`72`	`72`	`public ResponseEntity<List<ConnectionEntity>> getConnections(@RequestHeader("userId") String userId) {`
`73`	`73`	`return ResponseEntity.ok().contentType(MediaType.APPLICATION_JSON).body(service.getConnections(userId));`
`74`	`74`	`}`
	`75`	`+`
	`76`	`+ @Deprecated(forRemoval = true)`
	`77`	`+ @GetMapping(value = "/connections/deduplicate")`
	`78`	`+ @Operation(summary = "get the connections deduplicated", deprecated = true)`
	`79`	`+ @ApiResponse(responseCode = "200", description = "The connections list")`
	`80`	`+ public ResponseEntity<List<ConnectionEntity>> deduplicateConnections(@RequestHeader("userId") String userId) {`
	`81`	`+ return ResponseEntity.ok().contentType(MediaType.APPLICATION_JSON).body(service.getDeduplicatedConnections(userId));`
	`82`	`+ }`
`75`	`83`	`}`
Original file line number	Diff line number	Diff line change
`@@ -43,6 +43,7 @@ public void recordConnectionAttempt(String sub, Boolean isAllowed) {`
`43`	`43`	`}`
`44`	`44`	`}`
`45`	`45`
	`46`	`+ @Deprecated(forRemoval = true)`
`46`	`47`	`@Transactional`
`47`	`48`	`public List<ConnectionEntity> removeDuplicates() {`
`48`	`49`	`Map<String, List<ConnectionEntity>> connectionsBySub = connectionRepository.findAll().stream().collect(Collectors.groupingBy(ConnectionEntity::getSub));`