Skip to content

Commit 948ed09

Browse files
dfcoffinclaude
andauthored
feat: ESPI 4.0 Schema Compliance - Phase 16c: UsagePoint Repository Cleanup (#85)
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 <[email protected]>
1 parent 0b91686 commit 948ed09

File tree

2 files changed

+35
-104
lines changed

2 files changed

+35
-104
lines changed

openespi-common/src/main/java/org/greenbuttonalliance/espi/common/repositories/usage/UsagePointRepository.java

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -35,57 +35,56 @@
3535
/**
3636
* Modern Spring Data JPA repository for UsagePoint entities.
3737
* Replaces the legacy UsagePointRepositoryImpl with modern Spring Data patterns.
38+
* <p>
39+
* Phase 16c Repository Cleanup:
40+
* - Removed queries on non-indexed columns (uri)
41+
* - Removed full table scan queries (findAllIds)
42+
* - Converted @Query to Spring Data JPA derived queries where possible
43+
* - Use built-in JpaRepository methods (existsById, deleteById) instead of custom queries
44+
* - All remaining queries use indexed columns for optimal performance
3845
*/
3946
@Repository
4047
public interface UsagePointRepository extends JpaRepository<UsagePointEntity, UUID> {
4148

4249
/**
4350
* Find all usage points for a specific retail customer.
51+
* Uses indexed column: retail_customer_id
52+
* Converted to Spring Data JPA derived query method (Phase 16c).
53+
*
54+
* @param retailCustomerId the retail customer ID
55+
* @return list of usage points for the customer
4456
*/
45-
@Query("SELECT up FROM UsagePointEntity up WHERE up.retailCustomer.id = :retailCustomerId")
46-
List<UsagePointEntity> findAllByRetailCustomerId(@Param("retailCustomerId") Long retailCustomerId);
47-
48-
/**
49-
* Find usage point by resource URI.
50-
*/
51-
@Query("SELECT up FROM UsagePointEntity up WHERE up.uri = :uri")
52-
Optional<UsagePointEntity> findByResourceUri(@Param("uri") String uri);
57+
List<UsagePointEntity> findAllByRetailCustomerId(Long retailCustomerId);
5358

5459
/**
5560
* Find usage point by related href.
61+
* Uses indexed relationship: usage_point_related_links(usage_point_id)
62+
*
63+
* @param href the related link href
64+
* @return optional usage point matching the href
5665
*/
5766
@Query("SELECT up FROM UsagePointEntity up JOIN up.relatedLinks rl WHERE rl.href = :href")
5867
Optional<UsagePointEntity> findByRelatedHref(@Param("href") String href);
5968

6069
/**
6170
* Find all usage points updated after a given timestamp.
71+
* Uses indexed column: updated
72+
*
73+
* @param lastUpdate the last update timestamp
74+
* @return list of usage points updated after the given time
6275
*/
63-
@Query("SELECT up FROM UsagePointEntity up WHERE up.updated > :lastUpdate")
64-
List<UsagePointEntity> findAllUpdatedAfter(@Param("lastUpdate") LocalDateTime lastUpdate);
76+
List<UsagePointEntity> findAllByUpdatedAfter(LocalDateTime lastUpdate);
6577

6678
/**
6779
* Find all usage point IDs for a specific retail customer.
80+
* Uses indexed column: retail_customer_id
81+
*
82+
* @param retailCustomerId the retail customer ID
83+
* @return list of usage point IDs for the customer
6884
*/
6985
@Query("SELECT up.id FROM UsagePointEntity up WHERE up.retailCustomer.id = :retailCustomerId")
7086
List<UUID> findAllIdsByRetailCustomerId(@Param("retailCustomerId") Long retailCustomerId);
7187

72-
/**
73-
* Find all usage point IDs.
74-
*/
75-
@Query("SELECT up.id FROM UsagePointEntity up")
76-
List<UUID> findAllIds();
77-
78-
/**
79-
* Check if usage point exists by UUID.
80-
*/
81-
@Query("SELECT COUNT(up) > 0 FROM UsagePointEntity up WHERE up.id = :uuid")
82-
boolean existsByUuid(@Param("uuid") UUID uuid);
83-
84-
/**
85-
* Delete usage point by UUID.
86-
*/
87-
@Modifying
88-
@Transactional
89-
@Query("DELETE FROM UsagePointEntity up WHERE up.id = :uuid")
90-
void deleteByUuid(@Param("uuid") UUID uuid);
88+
// Note: Use built-in existsById(UUID id) instead of custom existsByUuid
89+
// Note: Use built-in deleteById(UUID id) instead of custom deleteByUuid
9190
}

openespi-common/src/test/java/org/greenbuttonalliance/espi/common/repositories/usage/UsagePointRepositoryTest.java

Lines changed: 7 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -287,24 +287,7 @@ void shouldFindAllUsagePointsByRetailCustomerId() {
287287
.contains("Customer 2 Usage Point");
288288
}
289289

290-
@Test
291-
@DisplayName("Should find usage point by resource URI")
292-
void shouldFindUsagePointByResourceUri() {
293-
// Arrange
294-
UsagePointEntity usagePoint = TestDataBuilders.createValidUsagePoint();
295-
usagePoint.setDescription("Usage Point with URI");
296-
usagePoint.setUri("/espi/1_1/resource/UsagePoint/123");
297-
usagePointRepository.save(usagePoint);
298-
flushAndClear();
299-
300-
// Act
301-
Optional<UsagePointEntity> result = usagePointRepository.findByResourceUri("/espi/1_1/resource/UsagePoint/123");
302-
303-
// Assert
304-
assertThat(result).isPresent();
305-
assertThat(result.get().getDescription()).isEqualTo("Usage Point with URI");
306-
assertThat(result.get().getUri()).isEqualTo("/espi/1_1/resource/UsagePoint/123");
307-
}
290+
// Phase 16c: Removed test for findByResourceUri (method removed - uri not indexed, legacy field)
308291

309292
@Test
310293
@DisplayName("Should find usage point by related href")
@@ -355,7 +338,7 @@ void shouldFindAllUsagePointsUpdatedAfterTimestamp() {
355338
flushAndClear();
356339

357340
// Act
358-
List<UsagePointEntity> results = usagePointRepository.findAllUpdatedAfter(cutoffTime);
341+
List<UsagePointEntity> results = usagePointRepository.findAllByUpdatedAfter(cutoffTime);
359342

360343
// Assert - Should find at least the recent usage point
361344
assertThat(results).isNotEmpty();
@@ -386,69 +369,18 @@ void shouldFindAllUsagePointIdsByRetailCustomerId() {
386369
assertThat(usagePointIds).contains(savedUsagePoints.get(0).getId(), savedUsagePoints.get(1).getId());
387370
}
388371

389-
@Test
390-
@DisplayName("Should find all usage point IDs")
391-
void shouldFindAllUsagePointIds() {
392-
// Arrange
393-
long initialCount = usagePointRepository.count();
394-
List<UsagePointEntity> usagePoints = TestDataBuilders.createValidEntities(3, TestDataBuilders::createValidUsagePoint);
395-
List<UsagePointEntity> savedUsagePoints = usagePointRepository.saveAll(usagePoints);
396-
flushAndClear();
397-
398-
// Act
399-
List<UUID> allIds = usagePointRepository.findAllIds();
400-
401-
// Assert
402-
assertThat(allIds).hasSizeGreaterThanOrEqualTo(3);
403-
assertThat(allIds).contains(
404-
savedUsagePoints.get(0).getId(),
405-
savedUsagePoints.get(1).getId(),
406-
savedUsagePoints.get(2).getId()
407-
);
408-
}
409-
410-
@Test
411-
@DisplayName("Should check if usage point exists by UUID")
412-
void shouldCheckIfUsagePointExistsByUuid() {
413-
// Arrange
414-
UsagePointEntity usagePoint = TestDataBuilders.createValidUsagePoint();
415-
UsagePointEntity saved = usagePointRepository.save(usagePoint);
416-
flushAndClear();
417-
418-
// Act & Assert
419-
assertThat(usagePointRepository.existsByUuid(saved.getId())).isTrue();
420-
assertThat(usagePointRepository.existsByUuid(UUID.randomUUID())).isFalse();
421-
}
422-
423-
@Test
424-
@DisplayName("Should delete usage point by UUID")
425-
void shouldDeleteUsagePointByUuid() {
426-
// Arrange
427-
UsagePointEntity usagePoint = TestDataBuilders.createValidUsagePoint();
428-
usagePoint.setDescription("Usage Point to Delete by UUID");
429-
UsagePointEntity saved = usagePointRepository.save(usagePoint);
430-
UUID usagePointId = saved.getId();
431-
flushAndClear();
432-
433-
// Verify it exists
434-
assertThat(usagePointRepository.existsById(usagePointId)).isTrue();
435-
436-
// Act
437-
usagePointRepository.deleteByUuid(usagePointId);
438-
flushAndClear();
439-
440-
// Assert
441-
assertThat(usagePointRepository.existsById(usagePointId)).isFalse();
442-
}
372+
// Phase 16c: Removed test for findAllIds (method removed - full table scan without WHERE clause)
373+
// Phase 16c: Removed test for existsByUuid (use built-in existsById instead)
374+
// Phase 16c: Removed test for deleteByUuid (use built-in deleteById instead)
443375

444376
@Test
445377
@DisplayName("Should handle empty results gracefully")
446378
void shouldHandleEmptyResultsGracefully() {
447-
// Act & Assert
379+
// Act & Assert - Test all indexed query methods with non-existent data
448380
assertThat(usagePointRepository.findAllByRetailCustomerId(999999L)).isEmpty();
449-
assertThat(usagePointRepository.findByResourceUri("nonexistent-uri")).isEmpty();
450381
assertThat(usagePointRepository.findByRelatedHref("nonexistent-href")).isEmpty();
451382
assertThat(usagePointRepository.findAllIdsByRetailCustomerId(999999L)).isEmpty();
383+
assertThat(usagePointRepository.findAllByUpdatedAfter(java.time.LocalDateTime.now())).isEmpty();
452384
}
453385
}
454386

0 commit comments

Comments
 (0)