From 7b7ac32d0e69966b9aa689d01c5cb6f3873be910 Mon Sep 17 00:00:00 2001 From: "Donald F. Coffin" Date: Thu, 15 Jan 2026 18:17:06 -0500 Subject: [PATCH] feat: ESPI 4.0 Schema Compliance - Phase 16c: UsagePoint Repository Cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR implements Phase 16c of the UsagePoint schema compliance work, cleaning up the repository to remove non-indexed queries and convert to Spring Data JPA derived query methods for optimal performance. ## Changes in Phase 16c ### Repository Query Optimization Removed methods that query non-indexed columns or perform full table scans: - **Removed `findByResourceUri(String uri)`** - Queries `uri` field which is NOT indexed and is a legacy field NOT in ESPI 4.0 XSD - **Removed `findAllIds()`** - Full table scan without WHERE clause (performance risk) - **Removed `existsByUuid(UUID uuid)`** - Use built-in `existsById(UUID id)` instead - **Removed `deleteByUuid(UUID uuid)`** - Use built-in `deleteById(UUID id)` instead ### Spring Data JPA Derived Queries Converted `@Query` annotations to Spring Data JPA derived query methods: - **`findAllByRetailCustomerId(Long retailCustomerId)`** - Uses indexed `retail_customer_id` - **`findAllByUpdatedAfter(LocalDateTime lastUpdate)`** - Uses indexed `updated` column ### Retained Methods (All Use Indexed Columns) - `findByRelatedHref(String href)` - Uses indexed relationship table - `findAllByRetailCustomerId(Long retailCustomerId)` - Uses indexed `retail_customer_id` - `findAllByUpdatedAfter(LocalDateTime lastUpdate)` - Uses indexed `updated` - `findAllIdsByRetailCustomerId(Long retailCustomerId)` - Uses indexed `retail_customer_id` ### Database Index Analysis Current indexed columns on `usage_points` table: - `kind` - Indexed but NOT in ESPI 4.0 XSD (legacy field, no queries use it) - `status` - Indexed (no queries currently use it) - `retail_customer_id` - Indexed ✅ USED by queries - `service_delivery_point_id` - Indexed (no queries currently use it) - `local_time_parameters_id` - Indexed (no queries currently use it) - `created` - Indexed (no queries currently use it) - `updated` - Indexed ✅ USED by queries ### Test Updates - Removed 4 test methods for deleted repository methods: - `shouldFindUsagePointByResourceUri()` - `shouldFindAllUsagePointIds()` - `shouldCheckIfUsagePointExistsByUuid()` - `shouldDeleteUsagePointByUuid()` - Updated `shouldHandleEmptyResultsGracefully()` to test only retained methods - Updated method call from `findAllUpdatedAfter()` to `findAllByUpdatedAfter()` ## Technical Details ### Performance Benefits All remaining queries use indexed columns: - Eliminates full table scans (removed `findAllIds`) - Uses database indexes for O(log n) lookups instead of O(n) scans - Removes queries on non-indexed legacy field (`uri`) ### Spring Data JPA Patterns - Derived query methods (e.g., `findAllByRetailCustomerId`) are automatically implemented by Spring Data JPA at runtime - Reduces custom JPQL code and improves maintainability - Type-safe method names reduce query errors ### Legacy Field Review **`kind` field** (NOT in ESPI 4.0 XSD): - Has database index but is NOT used by any queries - Field exists in entity but not in XSD specification - No repository methods query this field - Consider for removal in future phase **`uri` field** (NOT in ESPI 4.0 XSD): - Legacy field documented in Phase 16b - No longer queryable (removed `findByResourceUri`) - Consider for removal in future phase ## Test Results ``` Tests run: 580, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` *Note: Was 584 tests, now 580 (removed 4 tests for deleted methods)* ## Related - Issue #28 - Phase 16: UsagePoint - PR #83 - Phase 16a: Add Missing Boolean/String Fields (merged) - PR #84 - Phase 16b: Add Enum Fields & Reorder (merged) - Part 3 of 5 sub-phases for complete UsagePoint compliance ## Next Steps (Phase 16d) - Mapper bidirectional updates - Update Entity-to-DTO mappings - Update DTO-to-Entity mappings - Handle all embedded SummaryMeasurement mappings Co-Authored-By: Claude Sonnet 4.5 --- .../usage/UsagePointRepository.java | 57 +++++++------ .../usage/UsagePointRepositoryTest.java | 82 ++----------------- 2 files changed, 35 insertions(+), 104 deletions(-) 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(); } }