Skip to content

Commit efaead7

Browse files
dgraf-ghdgraf-gh
authored
14 sec audit (#17)
* refactor: streamline ImportService and migrate to ImportStatistics for progress tracking - Replaced inline progress tracking methods with centralized `ImportStatistics` class. - Removed redundant print methods and progress reporting logic. - Consolidated `ImportService` phases with cleaner method calls and improved encapsulation. - Migrated service classes into a dedicated `importer` package for improved organization. * refactor: simplify StatsService and update configuration properties - Added in-memory stats DB option and cron-based flush scheduling. - Removed unused `clientIp` field and related logic. - Replaced custom `@SpringBootTest` annotations with `@IntegrationTest` for improved test consistency. - Consolidated stats recording and database initialization logic. - Updated test cases and introduced `StatsServiceIntegrationTest` for better coverage.
1 parent e1fbe7d commit efaead7

22 files changed

Lines changed: 838 additions & 470 deletions

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ The [OpenStreetMap website](https://www.openstreetmap.org/export/) allows you to
114114

115115
| Dataset | original | filtered... | time taken | reduction | during import | imported | time taken | reduction |
116116
|------------|----------|-------------|------------|-----------|---------------|----------|------------|-----------|
117-
| Planet | 86 GB | 33 GB | 40 min | ~60% | ~ 31 GB | 8.15 GB | ~ 16 h | ~90% |
117+
| Planet | 86 GB | 34 GB | 40 min | ~60% | ~ 31 GB | 8.15 GB | ~ 16 h | ~90% |
118118
| Germany | 4.4 GB | 1.8 GB | 2 min | ~59% | ~ 14.4 GB | 3,81 GB | ~ 18 min | ~13% |
119119
| Netherland | 1.4 GB | 394 MB | 30 s | ~70% | ~ 2,69 GB | 705,7 MB | ~ 2 min | ~50% |
120120

‎scripts/import.sh‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,7 @@ java $JVM_ARGS \
174174
-jar "$JAR_FILE" \
175175
--import \
176176
--pbf-file "$PBF_FILE" \
177-
--data-dir "$DATA_DIR" \
178-
--paikka.admin.password test
177+
--data-dir "$DATA_DIR"
179178

180179
EXIT_CODE=$?
181180

‎src/main/java/com/dedicatedcode/paikka/PaikkaApplication.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,10 @@
1616

1717
package com.dedicatedcode.paikka;
1818

19-
import com.dedicatedcode.paikka.service.ImportService;
19+
import com.dedicatedcode.paikka.service.importer.ImportService;
2020
import org.slf4j.Logger;
2121
import org.slf4j.LoggerFactory;
2222
import org.springframework.beans.factory.annotation.Autowired;
23-
import org.springframework.beans.factory.annotation.Value;
2423
import org.springframework.boot.CommandLineRunner;
2524
import org.springframework.boot.SpringApplication;
2625
import org.springframework.boot.autoconfigure.SpringBootApplication;

‎src/main/java/com/dedicatedcode/paikka/config/SecurityConfig.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.dedicatedcode.paikka.config;
1818

1919
import org.springframework.beans.factory.annotation.Value;
20+
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
2021
import org.springframework.context.annotation.Bean;
2122
import org.springframework.context.annotation.Configuration;
2223
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
@@ -31,11 +32,13 @@
3132

3233
@Configuration
3334
@EnableWebSecurity
35+
@ConditionalOnProperty(name = "paikka.import-mode", havingValue = "false", matchIfMissing = true)
3436
public class SecurityConfig {
3537
@Bean
3638
public SecurityFilterChain securityFilterChain(HttpSecurity http, AdminTokenFilter adminTokenFilter) throws Exception {
3739
http
3840
.authorizeHttpRequests(authorize -> authorize
41+
.requestMatchers("/admin/**").hasRole("ADMIN")
3942
.requestMatchers("/", "/login", "/error", "/logout").permitAll()
4043
.requestMatchers("/api/**", "/css/**", "/js/**", "/images/**", "/img/**", "/fonts/**").permitAll()
4144
.anyRequest().authenticated()

‎src/main/java/com/dedicatedcode/paikka/config/StatsInterceptor.java‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ public StatsInterceptor(StatsService statsService) {
4343

4444
@Override
4545
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {
46-
// Only track API endpoints
4746
if (request.getRequestURI().startsWith("/api/v1/")) {
4847
request.setAttribute("startTime", System.currentTimeMillis());
4948
}
@@ -53,8 +52,7 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
5352
@Override
5453
public void afterCompletion(HttpServletRequest request, HttpServletResponse response,
5554
Object handler, Exception ex) {
56-
57-
// Only track API endpoints and successful requests
55+
5856
if (!request.getRequestURI().startsWith("/api/v1/") ||
5957
request.getAttribute("startTime") == null ||
6058
response.getStatus() >= 400) {
@@ -63,17 +61,15 @@ public void afterCompletion(HttpServletRequest request, HttpServletResponse resp
6361

6462
try {
6563
long responseTime = System.currentTimeMillis() - (Long) request.getAttribute("startTime");
66-
67-
// Extract and sort parameters
64+
6865
Map<String, String> sortedParams = request.getParameterMap().entrySet().stream()
6966
.collect(Collectors.toMap(
70-
entry -> entry.getKey(),
67+
Map.Entry::getKey,
7168
entry -> String.join(",", entry.getValue()),
7269
(e1, e2) -> e1,
7370
TreeMap::new
7471
));
75-
76-
// Extract result count from response header (set by controllers)
72+
7773
int resultCount = 0;
7874
String resultCountHeader = response.getHeader("X-Result-Count");
7975
if (resultCountHeader != null) {
@@ -91,7 +87,6 @@ public void afterCompletion(HttpServletRequest request, HttpServletResponse resp
9187
sortedParams,
9288
responseTime,
9389
resultCount,
94-
clientIp,
9590
response.getStatus()
9691
);
9792

‎src/main/java/com/dedicatedcode/paikka/controller/AdminController.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,12 @@
2929
import org.springframework.web.bind.annotation.PostMapping;
3030
import org.springframework.web.bind.annotation.RequestMapping;
3131
import org.springframework.web.bind.annotation.ResponseBody;
32+
import org.springframework.web.bind.annotation.RestController;
3233

3334
import java.util.HashMap;
3435
import java.util.Map;
3536

36-
@Controller
37+
@RestController
3738
@RequestMapping("/admin")
3839
@ConditionalOnProperty(name = "paikka.import-mode", havingValue = "false", matchIfMissing = true)
3940
public class AdminController {
@@ -51,8 +52,6 @@ public AdminController(ReverseGeocodingService reverseGeocodingService, Boundary
5152
}
5253

5354
@PostMapping(value = "/refresh-db", produces = "application/json")
54-
@PreAuthorize("hasRole('ADMIN')")
55-
@ResponseBody
5655
public ResponseEntity<?> refreshDatabase() {
5756
logger.info("Database refresh requested");
5857

‎src/main/java/com/dedicatedcode/paikka/service/PaikkaMetadata.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
import com.fasterxml.jackson.annotation.JsonProperty;
2020

21-
record PaikkaMetadata(
21+
public record PaikkaMetadata(
2222
@JsonProperty("importTimestamp") String importTimestamp,
2323
@JsonProperty("dataVersion") String dataVersion,
2424
@JsonProperty("file") String file,

‎src/main/java/com/dedicatedcode/paikka/service/StatsService.java‎

Lines changed: 67 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,14 @@ public void cleanup() {
7979
}
8080

8181
private void initializeDatabase() throws SQLException {
82-
// Ensure directory exists
83-
File dbFile = new File(config.getStatsDbPath());
84-
dbFile.getParentFile().mkdirs();
85-
86-
// Connect to SQLite database
87-
String url = "jdbc:sqlite:" + config.getStatsDbPath();
88-
connection = DriverManager.getConnection(url);
89-
82+
if (config.getStatsDbPath().equals("memory")) {
83+
connection = DriverManager.getConnection("jdbc:sqlite::memory:");
84+
} else {
85+
File dbFile = new File(config.getStatsDbPath());
86+
dbFile.getParentFile().mkdirs();
87+
String url = "jdbc:sqlite:" + config.getStatsDbPath();
88+
connection = DriverManager.getConnection(url);
89+
}
9090
// Create table if not exists
9191
String createTableSQL = """
9292
CREATE TABLE IF NOT EXISTS query_stats (
@@ -96,7 +96,6 @@ endpoint VARCHAR(100),
9696
parameters TEXT,
9797
response_time_ms INTEGER,
9898
result_count INTEGER,
99-
client_ip VARCHAR(45),
10099
date_only DATE,
101100
hour_bucket INTEGER,
102101
status_code INTEGER
@@ -139,8 +138,8 @@ CREATE TABLE IF NOT EXISTS location_stats (
139138
}
140139

141140
@Async
142-
public void recordQuery(String endpoint, Map<String, String> sortedParams,
143-
long responseTimeMs, int resultCount, String clientIp, int statusCode) {
141+
public void recordQuery(String endpoint, Map<String, String> sortedParams,
142+
long responseTimeMs, int resultCount, int statusCode) {
144143
try {
145144
String parametersJson = objectMapper.writeValueAsString(sortedParams);
146145
LocalDateTime now = LocalDateTime.now();
@@ -150,15 +149,13 @@ public void recordQuery(String endpoint, Map<String, String> sortedParams,
150149
parametersJson,
151150
responseTimeMs,
152151
resultCount,
153-
clientIp,
154152
now.toLocalDate(),
155153
now.getHour(),
156154
statusCode
157155
);
158156

159157
pendingStats.offer(record);
160158

161-
// Record location if this is a reverse geocoding query
162159
if ("/api/v1/reverse".equals(endpoint) && sortedParams.containsKey("lat") && sortedParams.containsKey("lon")) {
163160
try {
164161
double lat = Double.parseDouble(sortedParams.get("lat"));
@@ -198,48 +195,50 @@ ON CONFLICT(rounded_lat, rounded_lon)
198195
logger.error("Failed to record location stats", e);
199196
}
200197
}
201-
202-
@Scheduled(fixedDelay = 10000) // Every 10 seconds
198+
199+
@Scheduled(cron = "${paikka.stats-db.flush}") // Every 10 seconds
203200
public void flushPendingStats() {
204201
if (pendingStats.isEmpty() || connection == null) {
205202
return;
206203
}
207-
208-
List<StatsRecord> batch = new ArrayList<>();
209-
StatsRecord record;
210-
while ((record = pendingStats.poll()) != null && batch.size() < 1000) {
211-
batch.add(record);
212-
}
213-
214-
if (batch.isEmpty()) {
215-
return;
216-
}
217-
218-
String insertSQL = """
219-
INSERT INTO query_stats (endpoint, parameters, response_time_ms, result_count,
220-
client_ip, date_only, hour_bucket, status_code)
221-
VALUES (?, ?, ?, ?, ?, ?, ?, ?)
222-
""";
223-
224-
try (PreparedStatement pstmt = connection.prepareStatement(insertSQL)) {
225-
for (StatsRecord statsRecord : batch) {
226-
pstmt.setString(1, statsRecord.endpoint);
227-
pstmt.setString(2, statsRecord.parametersJson);
228-
pstmt.setLong(3, statsRecord.responseTimeMs);
229-
pstmt.setInt(4, statsRecord.resultCount);
230-
pstmt.setString(5, statsRecord.clientIp);
231-
pstmt.setString(6, statsRecord.dateOnly.toString());
232-
pstmt.setInt(7, statsRecord.hourBucket);
233-
pstmt.setInt(8, statsRecord.statusCode);
234-
pstmt.addBatch();
204+
synchronized (pendingStats) {
205+
206+
List<StatsRecord> batch = new ArrayList<>();
207+
StatsRecord record;
208+
while ((record = pendingStats.poll()) != null && batch.size() < 1000) {
209+
batch.add(record);
210+
}
211+
212+
if (batch.isEmpty()) {
213+
return;
214+
}
215+
216+
String insertSQL = """
217+
INSERT INTO query_stats (endpoint, parameters, response_time_ms, result_count,
218+
date_only, hour_bucket, status_code)
219+
VALUES (?, ?, ?, ?, ?, ?, ?)
220+
""";
221+
222+
try (PreparedStatement pstmt = connection.prepareStatement(insertSQL)) {
223+
for (StatsRecord statsRecord : batch) {
224+
pstmt.setString(1, statsRecord.endpoint);
225+
pstmt.setString(2, statsRecord.parametersJson);
226+
pstmt.setLong(3, statsRecord.responseTimeMs);
227+
pstmt.setInt(4, statsRecord.resultCount);
228+
pstmt.setString(5, statsRecord.dateOnly.toString());
229+
pstmt.setInt(6, statsRecord.hourBucket);
230+
pstmt.setInt(7, statsRecord.statusCode);
231+
pstmt.addBatch();
232+
}
233+
pstmt.executeBatch();
234+
logger.debug("Flushed {} stats records to database", batch.size());
235+
} catch (SQLException e) {
236+
logger.error("Failed to flush stats to database", e);
237+
// Re-add failed records to queue
238+
batch.forEach(pendingStats::offer);
235239
}
236-
pstmt.executeBatch();
237-
logger.debug("Flushed {} stats records to database", batch.size());
238-
} catch (SQLException e) {
239-
logger.error("Failed to flush stats to database", e);
240-
// Re-add failed records to queue
241-
batch.forEach(pendingStats::offer);
242240
}
241+
243242
}
244243

245244
public List<StatsAggregationResponse> getDailyStats(LocalDate startDate, LocalDate endDate, String endpoint) {
@@ -355,11 +354,9 @@ public List<String> getAvailableEndpoints() {
355354

356355
public List<LocationStatsResponse> getLocationStats() {
357356
String sql = """
358-
SELECT rounded_lat, rounded_lon, query_count, last_queried
357+
SELECT rounded_lat, rounded_lon, query_count, last_queried
359358
FROM location_stats
360-
WHERE query_count >= 5
361-
ORDER BY query_count DESC
362-
LIMIT 1000
359+
WHERE query_count >= 5 ORDER BY query_count DESC
363360
""";
364361

365362
List<LocationStatsResponse> results = new ArrayList<>();
@@ -408,46 +405,24 @@ public void cleanupOldStats() {
408405
logger.error("Failed to cleanup old location stats", e);
409406
}
410407
}
411-
412-
public static class LocationStatsResponse {
413-
private final double lat;
414-
private final double lon;
415-
private final int queryCount;
416-
private final String lastQueried;
417-
418-
public LocationStatsResponse(double lat, double lon, int queryCount, String lastQueried) {
419-
this.lat = lat;
420-
this.lon = lon;
421-
this.queryCount = queryCount;
422-
this.lastQueried = lastQueried;
408+
409+
void clearDatabase() {
410+
try (PreparedStatement pstmt = connection.prepareStatement("DELETE FROM location_stats")) {
411+
pstmt.executeUpdate();
412+
} catch (SQLException e) {
413+
logger.error("Failed to cleanup old location stats", e);
423414
}
424-
425-
public double getLat() { return lat; }
426-
public double getLon() { return lon; }
427-
public int getQueryCount() { return queryCount; }
428-
public String getLastQueried() { return lastQueried; }
429-
}
430-
431-
private static class StatsRecord {
432-
final String endpoint;
433-
final String parametersJson;
434-
final long responseTimeMs;
435-
final int resultCount;
436-
final String clientIp;
437-
final LocalDate dateOnly;
438-
final int hourBucket;
439-
final int statusCode;
440-
441-
StatsRecord(String endpoint, String parametersJson, long responseTimeMs,
442-
int resultCount, String clientIp, LocalDate dateOnly, int hourBucket, int statusCode) {
443-
this.endpoint = endpoint;
444-
this.parametersJson = parametersJson;
445-
this.responseTimeMs = responseTimeMs;
446-
this.resultCount = resultCount;
447-
this.clientIp = clientIp;
448-
this.dateOnly = dateOnly;
449-
this.hourBucket = hourBucket;
450-
this.statusCode = statusCode;
415+
try (PreparedStatement pstmt = connection.prepareStatement("DELETE FROM query_stats")) {
416+
pstmt.executeUpdate();
417+
} catch (SQLException e) {
418+
logger.error("Failed to cleanup old location stats", e);
451419
}
452420
}
421+
422+
public record LocationStatsResponse(double lat, double lon, int queryCount, String lastQueried) {
423+
}
424+
425+
private record StatsRecord(String endpoint, String parametersJson, long responseTimeMs, int resultCount,
426+
LocalDate dateOnly, int hourBucket, int statusCode) {
427+
}
453428
}

src/main/java/com/dedicatedcode/paikka/service/GeometrySimplificationService.java renamed to src/main/java/com/dedicatedcode/paikka/service/importer/GeometrySimplificationService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* along with Paikka. If not, see <https://www.gnu.org/licenses/>.
1515
*/
1616

17-
package com.dedicatedcode.paikka.service;
17+
package com.dedicatedcode.paikka.service.importer;
1818

1919
import org.locationtech.jts.geom.Geometry;
2020
import org.locationtech.jts.simplify.DouglasPeuckerSimplifier;

src/main/java/com/dedicatedcode/paikka/service/HierarchyCache.java renamed to src/main/java/com/dedicatedcode/paikka/service/importer/HierarchyCache.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@
1414
* along with Paikka. If not, see <https://www.gnu.org/licenses/>.
1515
*/
1616

17-
package com.dedicatedcode.paikka.service;
17+
package com.dedicatedcode.paikka.service.importer;
1818

1919
import com.dedicatedcode.paikka.flatbuffers.Boundary;
20+
import com.dedicatedcode.paikka.service.S2Helper;
2021
import com.github.benmanes.caffeine.cache.Cache;
2122
import org.locationtech.jts.algorithm.locate.IndexedPointInAreaLocator;
2223
import org.locationtech.jts.geom.Coordinate;

0 commit comments

Comments
 (0)