diff --git a/openespi-common/src/main/java/org/greenbuttonalliance/espi/common/repositories/usage/UsagePointRepository.java b/openespi-common/src/main/java/org/greenbuttonalliance/espi/common/repositories/usage/UsagePointRepository.java index 043c7a3a..efb9abfc 100755 --- a/openespi-common/src/main/java/org/greenbuttonalliance/espi/common/repositories/usage/UsagePointRepository.java +++ b/openespi-common/src/main/java/org/greenbuttonalliance/espi/common/repositories/usage/UsagePointRepository.java @@ -35,57 +35,56 @@ /** * Modern Spring Data JPA repository for UsagePoint entities. * Replaces the legacy UsagePointRepositoryImpl with modern Spring Data patterns. + *

+ * Phase 16c Repository Cleanup: + * - Removed queries on non-indexed columns (uri) + * - Removed full table scan queries (findAllIds) + * - Converted @Query to Spring Data JPA derived queries where possible + * - Use built-in JpaRepository methods (existsById, deleteById) instead of custom queries + * - All remaining queries use indexed columns for optimal performance */ @Repository public interface UsagePointRepository extends JpaRepository { /** * Find all usage points for a specific retail customer. + * Uses indexed column: retail_customer_id + * Converted to Spring Data JPA derived query method (Phase 16c). + * + * @param retailCustomerId the retail customer ID + * @return list of usage points for the customer */ - @Query("SELECT up FROM UsagePointEntity up WHERE up.retailCustomer.id = :retailCustomerId") - List findAllByRetailCustomerId(@Param("retailCustomerId") Long retailCustomerId); - - /** - * Find usage point by resource URI. - */ - @Query("SELECT up FROM UsagePointEntity up WHERE up.uri = :uri") - Optional findByResourceUri(@Param("uri") String uri); + List findAllByRetailCustomerId(Long retailCustomerId); /** * Find usage point by related href. + * Uses indexed relationship: usage_point_related_links(usage_point_id) + * + * @param href the related link href + * @return optional usage point matching the href */ @Query("SELECT up FROM UsagePointEntity up JOIN up.relatedLinks rl WHERE rl.href = :href") Optional findByRelatedHref(@Param("href") String href); /** * Find all usage points updated after a given timestamp. + * Uses indexed column: updated + * + * @param lastUpdate the last update timestamp + * @return list of usage points updated after the given time */ - @Query("SELECT up FROM UsagePointEntity up WHERE up.updated > :lastUpdate") - List findAllUpdatedAfter(@Param("lastUpdate") LocalDateTime lastUpdate); + List findAllByUpdatedAfter(LocalDateTime lastUpdate); /** * Find all usage point IDs for a specific retail customer. + * Uses indexed column: retail_customer_id + * + * @param retailCustomerId the retail customer ID + * @return list of usage point IDs for the customer */ @Query("SELECT up.id FROM UsagePointEntity up WHERE up.retailCustomer.id = :retailCustomerId") List findAllIdsByRetailCustomerId(@Param("retailCustomerId") Long retailCustomerId); - /** - * Find all usage point IDs. - */ - @Query("SELECT up.id FROM UsagePointEntity up") - List findAllIds(); - - /** - * Check if usage point exists by UUID. - */ - @Query("SELECT COUNT(up) > 0 FROM UsagePointEntity up WHERE up.id = :uuid") - boolean existsByUuid(@Param("uuid") UUID uuid); - - /** - * Delete usage point by UUID. - */ - @Modifying - @Transactional - @Query("DELETE FROM UsagePointEntity up WHERE up.id = :uuid") - void deleteByUuid(@Param("uuid") UUID uuid); + // Note: Use built-in existsById(UUID id) instead of custom existsByUuid + // Note: Use built-in deleteById(UUID id) instead of custom deleteByUuid } diff --git a/openespi-common/src/test/java/org/greenbuttonalliance/espi/common/repositories/usage/UsagePointRepositoryTest.java b/openespi-common/src/test/java/org/greenbuttonalliance/espi/common/repositories/usage/UsagePointRepositoryTest.java index 2f7f8542..0c64514f 100644 --- a/openespi-common/src/test/java/org/greenbuttonalliance/espi/common/repositories/usage/UsagePointRepositoryTest.java +++ b/openespi-common/src/test/java/org/greenbuttonalliance/espi/common/repositories/usage/UsagePointRepositoryTest.java @@ -287,24 +287,7 @@ void shouldFindAllUsagePointsByRetailCustomerId() { .contains("Customer 2 Usage Point"); } - @Test - @DisplayName("Should find usage point by resource URI") - void shouldFindUsagePointByResourceUri() { - // Arrange - UsagePointEntity usagePoint = TestDataBuilders.createValidUsagePoint(); - usagePoint.setDescription("Usage Point with URI"); - usagePoint.setUri("/espi/1_1/resource/UsagePoint/123"); - usagePointRepository.save(usagePoint); - flushAndClear(); - - // Act - Optional result = usagePointRepository.findByResourceUri("/espi/1_1/resource/UsagePoint/123"); - - // Assert - assertThat(result).isPresent(); - assertThat(result.get().getDescription()).isEqualTo("Usage Point with URI"); - assertThat(result.get().getUri()).isEqualTo("/espi/1_1/resource/UsagePoint/123"); - } + // Phase 16c: Removed test for findByResourceUri (method removed - uri not indexed, legacy field) @Test @DisplayName("Should find usage point by related href") @@ -355,7 +338,7 @@ void shouldFindAllUsagePointsUpdatedAfterTimestamp() { flushAndClear(); // Act - List results = usagePointRepository.findAllUpdatedAfter(cutoffTime); + List results = usagePointRepository.findAllByUpdatedAfter(cutoffTime); // Assert - Should find at least the recent usage point assertThat(results).isNotEmpty(); @@ -386,69 +369,18 @@ void shouldFindAllUsagePointIdsByRetailCustomerId() { assertThat(usagePointIds).contains(savedUsagePoints.get(0).getId(), savedUsagePoints.get(1).getId()); } - @Test - @DisplayName("Should find all usage point IDs") - void shouldFindAllUsagePointIds() { - // Arrange - long initialCount = usagePointRepository.count(); - List usagePoints = TestDataBuilders.createValidEntities(3, TestDataBuilders::createValidUsagePoint); - List savedUsagePoints = usagePointRepository.saveAll(usagePoints); - flushAndClear(); - - // Act - List allIds = usagePointRepository.findAllIds(); - - // Assert - assertThat(allIds).hasSizeGreaterThanOrEqualTo(3); - assertThat(allIds).contains( - savedUsagePoints.get(0).getId(), - savedUsagePoints.get(1).getId(), - savedUsagePoints.get(2).getId() - ); - } - - @Test - @DisplayName("Should check if usage point exists by UUID") - void shouldCheckIfUsagePointExistsByUuid() { - // Arrange - UsagePointEntity usagePoint = TestDataBuilders.createValidUsagePoint(); - UsagePointEntity saved = usagePointRepository.save(usagePoint); - flushAndClear(); - - // Act & Assert - assertThat(usagePointRepository.existsByUuid(saved.getId())).isTrue(); - assertThat(usagePointRepository.existsByUuid(UUID.randomUUID())).isFalse(); - } - - @Test - @DisplayName("Should delete usage point by UUID") - void shouldDeleteUsagePointByUuid() { - // Arrange - UsagePointEntity usagePoint = TestDataBuilders.createValidUsagePoint(); - usagePoint.setDescription("Usage Point to Delete by UUID"); - UsagePointEntity saved = usagePointRepository.save(usagePoint); - UUID usagePointId = saved.getId(); - flushAndClear(); - - // Verify it exists - assertThat(usagePointRepository.existsById(usagePointId)).isTrue(); - - // Act - usagePointRepository.deleteByUuid(usagePointId); - flushAndClear(); - - // Assert - assertThat(usagePointRepository.existsById(usagePointId)).isFalse(); - } + // Phase 16c: Removed test for findAllIds (method removed - full table scan without WHERE clause) + // Phase 16c: Removed test for existsByUuid (use built-in existsById instead) + // Phase 16c: Removed test for deleteByUuid (use built-in deleteById instead) @Test @DisplayName("Should handle empty results gracefully") void shouldHandleEmptyResultsGracefully() { - // Act & Assert + // Act & Assert - Test all indexed query methods with non-existent data assertThat(usagePointRepository.findAllByRetailCustomerId(999999L)).isEmpty(); - assertThat(usagePointRepository.findByResourceUri("nonexistent-uri")).isEmpty(); assertThat(usagePointRepository.findByRelatedHref("nonexistent-href")).isEmpty(); assertThat(usagePointRepository.findAllIdsByRetailCustomerId(999999L)).isEmpty(); + assertThat(usagePointRepository.findAllByUpdatedAfter(java.time.LocalDateTime.now())).isEmpty(); } }