-
Notifications
You must be signed in to change notification settings - Fork 0
Implement persistence layer with binary serializer and state management #11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
354c1bf
fa52afe
a55720a
814ee9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,10 @@ | ||
| package com.smartjam.smartjamanalyzer.application; | ||
|
|
||
| import java.nio.file.Path; | ||
| import java.util.UUID; | ||
|
|
||
| import com.smartjam.common.model.AudioProcessingStatus; | ||
| import com.smartjam.smartjamanalyzer.domain.model.AnalysisResult; | ||
| import com.smartjam.smartjamanalyzer.domain.model.FeatureSequence; | ||
| import com.smartjam.smartjamanalyzer.domain.port.*; | ||
| import lombok.RequiredArgsConstructor; | ||
|
|
@@ -19,12 +22,21 @@ public class AudioAnalysisUseCase { | |
| private final WorkspaceFactory workspaceFactory; | ||
| private final FeatureExtractor featureExtractor; | ||
|
|
||
| private final PerformanceEvaluator performanceEvaluator; | ||
| private final ReferenceRepository referenceRepository; | ||
| private final ResultRepository resultRepository; | ||
| private final DebugVisualizer debugVisualizer; | ||
|
|
||
| public void execute(String bucket, String fileKey) { | ||
| try (Workspace workspace = workspaceFactory.create()) { | ||
| UUID entityId = extractUuid(fileKey); | ||
|
|
||
| // TODO: Добавить обработку(проверку типа) входящего файла | ||
|
|
||
| // TODO: Добавить нормальный сбор метрик | ||
|
|
||
| // TODO: Добавить обработку(проверку типа) входящего файла | ||
| try (Workspace workspace = workspaceFactory.create()) { | ||
| updateStatus(bucket, entityId, AudioProcessingStatus.ANALYZING, null); | ||
|
|
||
| // TODO: Добавить нормальный сбор метрик | ||
| StopWatch watch = new StopWatch(fileKey); | ||
|
|
||
| log.info("=== Начало обработки файла: {} из бакета {} ===", fileKey, bucket); | ||
|
|
@@ -37,24 +49,84 @@ public void execute(String bucket, String fileKey) { | |
| Path cleanWavFile = audioConverter.convertToStandardWav(localFile, workspace); | ||
| watch.stop(); | ||
|
|
||
| watch.start("Business Logic (Math)"); | ||
|
|
||
| watch.start("Feature Extraction"); | ||
| FeatureSequence features = featureExtractor.extract(cleanWavFile); | ||
| watch.stop(); | ||
|
|
||
| log.info("Extracted {} feature frames", features.frames().size()); | ||
|
|
||
| watch.start("Evaluation & Persistence"); | ||
| if ("references".equals(bucket)) { | ||
| log.info("Действие: Обработка ЭТАЛОНА учителя..."); | ||
| handleTeacherReference(entityId, features); | ||
| } else if ("submissions".equals(bucket)) { | ||
| log.info("Действие: Обработка ПОПЫТКИ ученика..."); | ||
| handleStudentSubmission(entityId, features); | ||
| } | ||
| watch.stop(); | ||
|
|
||
| log.info("Результаты обработки {}: \n{}", fileKey, watch.prettyPrint()); | ||
|
|
||
| } catch (Exception e) { | ||
| log.error("Ошибка в UseCase для файла {}: {}", fileKey, e.getMessage()); | ||
|
|
||
| String errorMsg = | ||
| e.getMessage() != null ? e.getMessage() : e.getClass().getSimpleName(); | ||
|
|
||
| log.error("Ошибка в UseCase для файла {}: {}", fileKey, errorMsg, e); | ||
| updateStatus(bucket, entityId, AudioProcessingStatus.FAILED, errorMsg); | ||
|
|
||
| throw new RuntimeException("Business logic failed", e); | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| private void updateStatus(String bucket, UUID id, AudioProcessingStatus status, String error) { | ||
| try { | ||
| if ("references".equals(bucket)) { | ||
| referenceRepository.updateStatus(id, status, error); | ||
| } else { | ||
| resultRepository.updateStatus(id, status, error); | ||
| } | ||
| } catch (Exception ex) { | ||
| log.error("Failed to update status in DB for {}: {}", id, ex.getMessage()); | ||
| } | ||
| } | ||
|
|
||
| private void handleTeacherReference(UUID assignmentId, FeatureSequence teacherFeatures) { | ||
| log.info("Saving teacher reference features for assignment: {}", assignmentId); | ||
| referenceRepository.save(assignmentId, teacherFeatures); | ||
| } | ||
|
|
||
| private void handleStudentSubmission(UUID submissionId, FeatureSequence studentFeatures) { | ||
| log.info("Evaluating student submission: {}", submissionId); | ||
|
|
||
| UUID assignmentId = resultRepository | ||
| .findAssignmentIdBySubmissionId(submissionId) | ||
| .orElseThrow(() -> | ||
| new IllegalStateException("Submission " + submissionId + " is not linked to any assignment")); | ||
|
|
||
| FeatureSequence teacherFeatures = referenceRepository | ||
| .findFeaturesById(assignmentId) | ||
| .orElseThrow(() -> new IllegalStateException( | ||
| "Teacher reference features not found for assignment: " + assignmentId)); | ||
|
|
||
| AnalysisResult result = performanceEvaluator.evaluate(teacherFeatures, studentFeatures); | ||
|
|
||
| resultRepository.save(submissionId, result); | ||
|
|
||
| try { | ||
| debugVisualizer.generateHeatmap(result, "debug_" + submissionId + ".png"); | ||
| } catch (Exception e) { | ||
| log.error("Не удалось сгенерировать тепловую карту {}: {}", submissionId, e.getMessage()); | ||
| } | ||
|
|
||
| log.info("Submission {} evaluation completed.", submissionId); | ||
| } | ||
|
|
||
| private UUID extractUuid(String fileKey) { | ||
| try { | ||
| String rawUuid = fileKey.contains(".") ? fileKey.substring(0, fileKey.lastIndexOf(".")) : fileKey; | ||
|
|
||
| return UUID.fromString(rawUuid); | ||
| } catch (Exception e) { | ||
| throw new IllegalArgumentException("Invalid S3 file key. Expected UUID, got: " + fileKey); | ||
| } | ||
|
Comment on lines
+123
to
+130
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
for name in StorageService.java S3StorageListener.java; do
fd "^${name}$" backend -x sh -c '
echo "--- $1 ---"
sed -n "1,220p" "$1"
' sh {}
done
echo "--- key generation / upload sites ---"
rg -n --type java -g '!**/test/**' 'UUID\.randomUUID|putObject|fileKey|objectKey|upload.*(Audio|File)' backend/smartjam-api backend/smartjam-analyzerRepository: Satlykovs/SmartJam Length of output: 6019 🏁 Script executed: #!/bin/bash
# Find StorageService in API (key generation)
fd "StorageService.java" backend/smartjam-api -x cat -n {}
# Check for UUID generation in API
echo "--- UUID generation in API ---"
rg -n --type java 'UUID\.randomUUID' backend/smartjam-api
# Find upload endpoints and key assignment logic
echo "--- Upload logic and key assignment ---"
rg -n --type java -A5 -B2 'putObject|fileKey|objectKey|@PostMapping.*upload|@PostMapping.*audio' backend/smartjam-api | head -100Repository: Satlykovs/SmartJam Length of output: 132 🏁 Script executed: #!/bin/bash
# Find all modules/directories
echo "=== Repository structure ==="
ls -la backend/
echo -e "\n=== Java files in smartjam-api ==="
find backend/smartjam-api -name "*.java" -type f | head -30
echo -e "\n=== All classes with 'Upload' or 'Storage' in smartjam-api ==="
find backend/smartjam-api -name "*.java" -type f | xargs grep -l "Upload\|Storage" 2>/dev/null
echo -e "\n=== Key generation patterns across entire backend ==="
rg -n --type java 'UUID\.randomUUID|S3|putObject' backend --max-count=20Repository: Satlykovs/SmartJam Length of output: 8099 🏁 Script executed: #!/bin/bash
echo "=== S3StorageListenerTest key examples ==="
cat -n backend/smartjam-analyzer/src/test/java/com/smartjam/smartjamanalyzer/api/listener/S3StorageListenerTest.java
echo -e "\n=== Check docker-compose for clues about upload source ==="
grep -n -A5 -B5 "upload\|minio\|s3" docker-compose.yaml
echo -e "\n=== Check if there's a frontend or external documentation ==="
find . -name "*.md" -o -name "*.txt" | xargs grep -l "upload\|S3 key\|fileKey" 2>/dev/null | head -5Repository: Satlykovs/SmartJam Length of output: 4477 🏁 Script executed: #!/bin/bash
echo "=== AudioAnalysisUseCase tests ==="
find backend/smartjam-analyzer -name "*AudioAnalysisUseCase*Test*.java" -o -name "*Test*AudioAnalysisUseCase*.java" | xargs cat -n 2>/dev/null
echo -e "\n=== Full AudioAnalysisUseCase.execute() and extractUuid() ==="
sed -n '30,132p' backend/smartjam-analyzer/src/main/java/com/smartjam/smartjamanalyzer/application/AudioAnalysisUseCase.java | cat -n
echo -e "\n=== Check if there are prefixed keys anywhere ==="
rg -n 'folder/|uploads/|audio/|prefix' backend --type javaRepository: Satlykovs/SmartJam Length of output: 11339 UUID format enforcement happens too early, before producer-side migration to UUID keys is complete. The Verify that the producer side (smartjam-api uploader) now guarantees all S3 object keys are bare UUIDs before this strict parsing is safe. If uploads still use arbitrary filenames or folder prefixes, this will silently drop uploads without proper error tracking. 🤖 Prompt for AI Agents
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But if the key is wrong, we will be unable to find the record with that key, because it's invalid UUID There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'll withdraw this concern — the current implementation is correct. 🧠 Learnings used |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,36 @@ | ||
| package com.smartjam.smartjamanalyzer.domain.port; | ||
|
|
||
| import java.util.Optional; | ||
| import java.util.UUID; | ||
|
|
||
| import com.smartjam.common.model.AudioProcessingStatus; | ||
| import com.smartjam.smartjamanalyzer.domain.model.FeatureSequence; | ||
|
|
||
| /** Domain port for managing teacher reference features. */ | ||
| public interface ReferenceRepository { | ||
| void save(UUID targetId, FeatureSequence features); | ||
|
|
||
| FeatureSequence findById(UUID targetId); | ||
| /** | ||
| * Saves the spectral features of a teacher's reference track. | ||
| * | ||
| * @param assignmentId Unique identifier of the assignment. | ||
| * @param features Extracted features to persist. | ||
| */ | ||
| void save(UUID assignmentId, FeatureSequence features); | ||
|
|
||
| /** | ||
| * Retrieves reference features for comparison. | ||
| * | ||
| * @param assignmentId Unique identifier of the assignment. | ||
| * @return The feature sequence if presented. | ||
| */ | ||
| Optional<FeatureSequence> findFeaturesById(UUID assignmentId); | ||
|
|
||
| /** | ||
| * Performs an optimized status update for an assignment, optionally recording an error message. | ||
| * | ||
| * @param assignmentId unique identifier of the submission. | ||
| * @param status the new processing state. | ||
| * @param errorMessage description of the failure, if applicable. | ||
| */ | ||
| void updateStatus(UUID assignmentId, AudioProcessingStatus status, String errorMessage); | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,40 @@ | ||
| package com.smartjam.smartjamanalyzer.domain.port; | ||
|
|
||
| import java.util.Optional; | ||
| import java.util.UUID; | ||
|
|
||
| import com.smartjam.common.model.AudioProcessingStatus; | ||
| import com.smartjam.smartjamanalyzer.domain.model.AnalysisResult; | ||
|
|
||
| /** | ||
| * Port for managing student submissions and their analysis results. Handles the persistence of evaluation scores and | ||
| * detailed feedback events. | ||
| */ | ||
| public interface ResultRepository { | ||
|
|
||
| /** | ||
| * Persists the final analysis result for a specific submission. | ||
| * | ||
| * @param submissionId unique identifier of the student submission. | ||
| * @param result calculated scores and feedback events. | ||
| */ | ||
| void save(UUID submissionId, AnalysisResult result); | ||
|
|
||
| /** | ||
| * Resolves the teacher's assignment ID associated with a given student submission. | ||
| * | ||
| * @param submissionId unique identifier of the submission. | ||
| * @return the UUID of the parent assignment. | ||
| * @throws RuntimeException if no linked assignment ID found. | ||
| */ | ||
| Optional<UUID> findAssignmentIdBySubmissionId(UUID submissionId); | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Update status for a submission, optionally recording an error message. | ||
| * | ||
| * @param submissionId unique identifier of the submission. | ||
| * @param status the new processing state. | ||
| * @param errorMessage description of the failure, if applicable. | ||
| */ | ||
| void updateStatus(UUID submissionId, AudioProcessingStatus status, String errorMessage); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package com.smartjam.smartjamanalyzer.infrastructure.persistence.adapter; | ||
|
|
||
| import java.util.Optional; | ||
| import java.util.UUID; | ||
|
|
||
| import com.smartjam.common.model.AudioProcessingStatus; | ||
| import com.smartjam.smartjamanalyzer.domain.model.FeatureSequence; | ||
| import com.smartjam.smartjamanalyzer.domain.port.ReferenceRepository; | ||
| import com.smartjam.smartjamanalyzer.infrastructure.persistence.entity.AssignmentEntity; | ||
| import com.smartjam.smartjamanalyzer.infrastructure.persistence.repository.JpaAssignmentRepository; | ||
| import com.smartjam.smartjamanalyzer.infrastructure.utils.FeatureBinarySerializer; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| /** | ||
| * JPA implementation of {@link ReferenceRepository}. Bridges the domain logic with the database using binary | ||
| * serialization for spectral data. | ||
| */ | ||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class AssignmentPersistenceAdapter implements ReferenceRepository { | ||
| private final JpaAssignmentRepository repository; | ||
|
|
||
| /** | ||
| * Packs spectral features into a binary format and saves them to the assignment record. Transition the status to | ||
| * {@link AudioProcessingStatus#COMPLETED}. | ||
| */ | ||
| @Override | ||
| @Transactional | ||
| public void save(UUID assignmentId, FeatureSequence features) { | ||
| byte[] bytes = FeatureBinarySerializer.serialize(features); | ||
|
|
||
| AssignmentEntity entity = repository | ||
| .findById(assignmentId) | ||
| .orElseThrow(() -> new IllegalStateException("Assignment metadata missing for ID: " + assignmentId | ||
| + ". It might have " + "been deleted or not created yet.")); | ||
|
|
||
| entity.setReferenceSpectreCache(bytes); | ||
| entity.setStatus(AudioProcessingStatus.COMPLETED); | ||
| entity.setErrorMessage(null); | ||
|
|
||
| repository.save(entity); | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves and unpacks binary features from the database. | ||
| * | ||
| * @return an Optional containing the FeatureSequence, or empty if not found. | ||
| */ | ||
| @Override | ||
| @Transactional(readOnly = true) | ||
| public Optional<FeatureSequence> findFeaturesById(UUID assignmentId) { | ||
| return repository | ||
| .findById(assignmentId) | ||
| .map(e -> FeatureBinarySerializer.deserialize(e.getReferenceSpectreCache())); | ||
| } | ||
|
|
||
| @Override | ||
| @Transactional | ||
| public void updateStatus(UUID id, AudioProcessingStatus status, String errorMessage) { | ||
| repository.updateStatus(id, status, errorMessage); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| package com.smartjam.smartjamanalyzer.infrastructure.persistence.adapter; | ||
|
|
||
| import java.util.Optional; | ||
| import java.util.UUID; | ||
|
|
||
| import com.smartjam.common.model.AudioProcessingStatus; | ||
| import com.smartjam.smartjamanalyzer.domain.model.AnalysisResult; | ||
| import com.smartjam.smartjamanalyzer.domain.port.ResultRepository; | ||
| import com.smartjam.smartjamanalyzer.infrastructure.persistence.entity.SubmissionEntity; | ||
| import com.smartjam.smartjamanalyzer.infrastructure.persistence.repository.JpaSubmissionRepository; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| /** | ||
| * JPA implementation of {@link ResultRepository}. Manages storage of evaluation results and coordinates mapping between | ||
| * domain results and JSONB database columns. | ||
| */ | ||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class SubmissionPersistenceAdapter implements ResultRepository { | ||
| private final JpaSubmissionRepository repository; | ||
|
|
||
| /** | ||
| * Updates the submission record with scores and feedback. Transition the status to | ||
| * {@link AudioProcessingStatus#COMPLETED}. | ||
| */ | ||
| @Override | ||
| @Transactional | ||
| public void save(UUID submissionId, AnalysisResult result) { | ||
| SubmissionEntity entity = repository | ||
| .findById(submissionId) | ||
| .orElseThrow(() -> new IllegalStateException("Submission record missing for ID: " + submissionId)); | ||
|
|
||
| entity.setTotalScore(result.totalScore()); | ||
| entity.setPitchScore(result.pitchScore()); | ||
| entity.setRhythmScore(result.rhythmScore()); | ||
| entity.setAnalysisFeedback(result.feedback()); | ||
| entity.setStatus(AudioProcessingStatus.COMPLETED); | ||
| entity.setErrorMessage(null); | ||
|
|
||
| repository.save(entity); | ||
| } | ||
|
|
||
| @Override | ||
| @Transactional(readOnly = true) | ||
| public Optional<UUID> findAssignmentIdBySubmissionId(UUID submissionId) { | ||
| return repository.findById(submissionId).map(SubmissionEntity::getAssignmentId); | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| @Override | ||
| @Transactional | ||
| public void updateStatus(UUID submissionId, AudioProcessingStatus status, String errorMessage) { | ||
| repository.updateStatus(submissionId, status, errorMessage); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| package com.smartjam.smartjamanalyzer.infrastructure.persistence.entity; | ||
|
|
||
| import java.util.UUID; | ||
|
|
||
| import jakarta.persistence.*; | ||
|
|
||
| import com.smartjam.common.model.AudioProcessingStatus; | ||
| import lombok.*; | ||
|
|
||
| /** Database model for teacher assignments. Stores heavy spectral data as raw bytes (BYTEA) to optimize performance. */ | ||
| @Entity | ||
| @Table(name = "assignments") | ||
| @Getter | ||
| @Setter | ||
| @Builder | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| public class AssignmentEntity { | ||
| @Id | ||
| private UUID id; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| private AudioProcessingStatus status; | ||
|
|
||
| @Column(name = "reference_spectre_cache") | ||
| private byte[] referenceSpectreCache; | ||
|
|
||
| @Column(name = "error_message", columnDefinition = "TEXT") | ||
| private String errorMessage; | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
Comment on lines
+11
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Add a versioned DB migration for these new columns/tables. Relying on 🤖 Prompt for AI Agents
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I know about it, will fix later when Liquibase will be added There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Would you like me to open a GitHub issue to track the Liquibase migration work so it isn't forgotten? If so, I can create one now. 🧠 Learnings used |
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reject unknown buckets before starting the pipeline.
Lines 58-63 only handle
"references"and"submissions", but Lines 80-86 route every other bucket toresultRepository. An unexpected bucket can still be markedANALYZING, skip both save paths, and return without ever reachingCOMPLETED. Validate the bucket once up front and keepupdateStatus(...)aligned with that same two-value contract.🛠️ Proposed fix
public void execute(String bucket, String fileKey) { + if (!"references".equals(bucket) && !"submissions".equals(bucket)) { + throw new IllegalArgumentException("Unsupported bucket: " + bucket); + } + UUID entityId = extractUuid(fileKey); // TODO: Добавить обработку(проверку типа) входящего файла @@ private void updateStatus(String bucket, UUID id, AudioProcessingStatus status, String error) { try { if ("references".equals(bucket)) { referenceRepository.updateStatus(id, status, error); - } else { + } else if ("submissions".equals(bucket)) { resultRepository.updateStatus(id, status, error); } } catch (Exception ex) {Also applies to: 58-63, 80-86
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Length of output: 5480
@Satlykovs, thanks for the update! However, looking at the latest commit (4a130f4), the fix appears to be only partially applied. Two issues from the original comment remain:execute— an unknown bucket still reachesupdateStatus(ANALYZING, null)before any validation.updateStatusstill uses a bareelse— any bucket that isn't"references"(including unknown ones) still routes toresultRepository.updateStatus(...).The downstream save block (
else if ("submissions"...) is already correct, but the two items above still need to be addressed:Could you check if the changes were pushed correctly?
🧠 Learnings used