refactor: apply critical code review fixes
- draft_service: GeminiService is now passed as a required parameter to analyzeDraft/analyzeAllDrafts instead of being directly instantiated, ensuring consistent Riverpod-managed injection - gemini_provider: correct misleading comment (rate limiting is due to static field, not Provider; Provider enables future safe refactoring) - analysis_cache_service: cleanupExpired now also removes orphaned brand index entries from _brandIndexBox after deleting expired _box entries - analysis_cache_service: keysToDelete type corrected from List<dynamic> to List<String>, removing unnecessary as String cast - analysis_cache_service: _normalizeBrandName comment clarified to note that .toLowerCase() only affects ASCII characters, not Japanese text - camera_screen: add explicit ignore comment with rationale for showDialog after async gap (mounted check immediately precedes it) - camera_screen: remove leaked Cursor instruction comment from line 96 Made-with: Cursor
This commit is contained in:
parent
94f7ee20ea
commit
c8ffe2626f
|
|
@ -3,9 +3,11 @@ import '../services/gemini_service.dart';
|
|||
|
||||
/// GeminiService のシングルトンプロバイダー
|
||||
///
|
||||
/// アプリ全体で同一インスタンスを共有する。
|
||||
/// レート制限の状態(_lastApiCallTime)がインスタンス間で共有されるため、
|
||||
/// 複数画面から同時に呼び出しても連打防止が正しく機能する。
|
||||
/// アプリ全体で同一インスタンスを共有し、依存関係の注入を明示化する。
|
||||
/// レート制限の状態(_lastApiCallTime)は GeminiService の static フィールドの
|
||||
/// ため、Provider を経由しなくても共有される。しかし Provider を通じて注入する
|
||||
/// ことで「誰が GeminiService を使うか」を追跡可能にし、将来の static 除去時に
|
||||
/// 安全にリファクタリングできる設計を維持する。
|
||||
///
|
||||
/// 使用例:
|
||||
/// ```dart
|
||||
|
|
|
|||
|
|
@ -93,8 +93,6 @@ class _CameraScreenState extends ConsumerState<CameraScreen> with SingleTickerPr
|
|||
super.dispose();
|
||||
}
|
||||
|
||||
// ... (Keep existing methods: _capturedImages, _takePicture, _analyzeImages)
|
||||
|
||||
double? _pendingExposureValue;
|
||||
bool _isUpdatingExposure = false;
|
||||
|
||||
|
|
@ -424,6 +422,8 @@ class _CameraScreenState extends ConsumerState<CameraScreen> with SingleTickerPr
|
|||
|
||||
// オンライン時: 通常の解析フロー
|
||||
if (!mounted) return;
|
||||
// ignore: use_build_context_synchronously
|
||||
// 直前の mounted チェックにより BuildContext の有効性は保証されている
|
||||
showDialog(
|
||||
context: context,
|
||||
barrierDismissible: false,
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import 'package:flutter/material.dart';
|
|||
import 'package:flutter_riverpod/flutter_riverpod.dart';
|
||||
import 'package:lucide_icons/lucide_icons.dart';
|
||||
import '../models/sake_item.dart';
|
||||
import '../providers/gemini_provider.dart';
|
||||
import '../services/draft_service.dart';
|
||||
import '../services/network_service.dart';
|
||||
import '../theme/app_colors.dart';
|
||||
|
|
@ -79,6 +80,7 @@ class _PendingAnalysisScreenState extends ConsumerState<PendingAnalysisScreen> {
|
|||
|
||||
try {
|
||||
final result = await DraftService.analyzeAllDrafts(
|
||||
geminiService: ref.read(geminiServiceProvider),
|
||||
onProgress: (progress, total) {
|
||||
if (mounted) {
|
||||
setState(() {
|
||||
|
|
|
|||
|
|
@ -137,12 +137,13 @@ class AnalysisCacheService {
|
|||
///
|
||||
/// アプリ起動時またはバックグラウンドで呼び出す。
|
||||
/// 旧形式エントリ(タイムスタンプなし)は対象外として保持する。
|
||||
/// _brandIndexBox の孤立エントリ(削除されたハッシュを参照するもの)も合わせて削除する。
|
||||
static Future<void> cleanupExpired({int ttlDays = 30}) async {
|
||||
await init();
|
||||
if (_box == null) return;
|
||||
|
||||
final cutoff = DateTime.now().subtract(Duration(days: ttlDays));
|
||||
final keysToDelete = <dynamic>[];
|
||||
final keysToDelete = <String>[];
|
||||
|
||||
for (final key in _box!.keys) {
|
||||
try {
|
||||
|
|
@ -163,6 +164,22 @@ class AnalysisCacheService {
|
|||
if (keysToDelete.isNotEmpty) {
|
||||
await _box!.deleteAll(keysToDelete);
|
||||
debugPrint('Cache cleanup: ${keysToDelete.length} expired entries removed');
|
||||
|
||||
// 削除したハッシュを参照している _brandIndexBox の孤立エントリも削除する
|
||||
if (_brandIndexBox != null) {
|
||||
final deletedHashes = keysToDelete.toSet();
|
||||
final brandKeysToDelete = <String>[];
|
||||
for (final brandKey in _brandIndexBox!.keys) {
|
||||
final hash = _brandIndexBox!.get(brandKey as String);
|
||||
if (hash != null && deletedHashes.contains(hash)) {
|
||||
brandKeysToDelete.add(brandKey);
|
||||
}
|
||||
}
|
||||
if (brandKeysToDelete.isNotEmpty) {
|
||||
await _brandIndexBox!.deleteAll(brandKeysToDelete);
|
||||
debugPrint('Cache cleanup: ${brandKeysToDelete.length} orphaned brand index entries removed');
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -225,12 +242,14 @@ class AnalysisCacheService {
|
|||
|
||||
/// 銘柄名の正規化
|
||||
///
|
||||
/// スペース除去、小文字化で表記ゆれを吸収
|
||||
/// 全角・半角スペースを除去し、ASCII文字を小文字に統一する。
|
||||
/// 例: "獺祭 純米大吟醸" → "獺祭純米大吟醸"
|
||||
/// 例: "DASSAI" → "dassai"
|
||||
/// 日本語文字には大文字・小文字の区別がないため、.toLowerCase() は ASCII のみに作用する。
|
||||
static String _normalizeBrandName(String name) {
|
||||
return name
|
||||
.replaceAll(RegExp(r'\s+'), '') // 全角・半角スペース除去
|
||||
.toLowerCase();
|
||||
.toLowerCase(); // ASCII ローマ字の表記ゆれを吸収(日本語には無効)
|
||||
}
|
||||
|
||||
/// 銘柄名インデックスをクリア(デバッグ用)
|
||||
|
|
|
|||
|
|
@ -122,7 +122,10 @@ class DraftService {
|
|||
/// print('解析失敗: $e');
|
||||
/// }
|
||||
/// ```
|
||||
static Future<SakeAnalysisResult> analyzeDraft(dynamic itemKey) async {
|
||||
static Future<SakeAnalysisResult> analyzeDraft(
|
||||
dynamic itemKey, {
|
||||
required GeminiService geminiService,
|
||||
}) async {
|
||||
final box = Hive.box<SakeItem>('sake_items');
|
||||
final item = box.get(itemKey);
|
||||
|
||||
|
|
@ -139,14 +142,11 @@ class DraftService {
|
|||
throw Exception('Draft has no photo path: $itemKey');
|
||||
}
|
||||
|
||||
debugPrint('🔍 Analyzing draft: $itemKey (${item.displayData.displayName})');
|
||||
debugPrint('Analyzing draft: $itemKey (${item.displayData.displayName})');
|
||||
|
||||
// 🔧 FIX: 複数画像がある場合はすべて使用
|
||||
final imagePaths = item.displayData.imagePaths;
|
||||
final pathsToAnalyze = imagePaths.isNotEmpty ? imagePaths : [photoPath];
|
||||
|
||||
// Gemini Vision API呼び出し
|
||||
final geminiService = GeminiService();
|
||||
final result = await geminiService.analyzeSakeLabel(pathsToAnalyze);
|
||||
|
||||
debugPrint('✅ Analysis completed: ${result.name}');
|
||||
|
|
@ -193,13 +193,14 @@ class DraftService {
|
|||
/// print('成功: ${result['success']}, 失敗: ${result['failed']}');
|
||||
/// ```
|
||||
static Future<Map<String, dynamic>> analyzeAllDrafts({
|
||||
required GeminiService geminiService,
|
||||
Function(int progress, int total)? onProgress,
|
||||
}) async {
|
||||
final drafts = await getPendingDrafts();
|
||||
final total = drafts.length;
|
||||
|
||||
if (total == 0) {
|
||||
debugPrint('📭 No pending drafts to analyze');
|
||||
debugPrint('No pending drafts to analyze');
|
||||
return {'success': 0, 'failed': 0, 'errors': []};
|
||||
}
|
||||
|
||||
|
|
@ -207,27 +208,24 @@ class DraftService {
|
|||
int failedCount = 0;
|
||||
final List<String> errors = [];
|
||||
|
||||
debugPrint('🚀 Analyzing $total drafts...');
|
||||
|
||||
for (int i = 0; i < total; i++) {
|
||||
final draft = drafts[i];
|
||||
final itemKey = draft.key;
|
||||
|
||||
try {
|
||||
onProgress?.call(i + 1, total);
|
||||
await analyzeDraft(itemKey);
|
||||
await analyzeDraft(itemKey, geminiService: geminiService);
|
||||
successCount++;
|
||||
debugPrint('✅ [${i+1}/$total] Success: ${draft.displayData.displayName}');
|
||||
debugPrint('[${i+1}/$total] Success: ${draft.displayData.displayName}');
|
||||
} catch (e) {
|
||||
failedCount++;
|
||||
final errorMsg = '[${i+1}/$total] ${draft.displayData.displayName}: $e';
|
||||
errors.add(errorMsg);
|
||||
debugPrint('❌ $errorMsg');
|
||||
// 失敗してもループは継続(他のDraftを解析)
|
||||
debugPrint(errorMsg);
|
||||
}
|
||||
}
|
||||
|
||||
debugPrint('🎉 Batch analysis completed: $successCount成功, $failedCount失敗');
|
||||
debugPrint('Batch analysis completed: $successCount success, $failedCount failed');
|
||||
|
||||
return {
|
||||
'success': successCount,
|
||||
|
|
|
|||
Loading…
Reference in New Issue