Skip to content

Commit 82726f8

Browse files
gravelldgravelld
committed
Fixed daneren2005#912 made insert faster, about doubly as quick. Only call getMusicDirectory once
1 parent cb70e9d commit 82726f8

2 files changed

Lines changed: 141 additions & 44 deletions

File tree

‎app/src/main/java/github/daneren2005/dsub/util/FileUtil.java‎

Lines changed: 129 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -113,29 +113,64 @@ public static File getEntryFile(Context context, MusicDirectory.Entry entry) {
113113
}
114114

115115
public static File getSongFile(Context context, MusicDirectory.Entry song) {
116-
File dir = getAlbumDirectory(context, song);
116+
File rootMusicDir = getMusicDirectory(context); // Fetches and validates the root
117+
if (rootMusicDir == null) {
118+
String msg = "getSongFile: Root music directory is null. Cannot proceed.";
119+
Log.e(TAG, msg);
120+
throw new IllegalStateException(msg);
121+
}
122+
return getSongFile(context, song, rootMusicDir);
123+
}
117124

118-
StringBuilder fileName = new StringBuilder();
119-
Integer track = song.getTrack();
120-
if (track != null) {
121-
if (track < 10) {
122-
fileName.append("0");
123-
}
124-
fileName.append(track).append("-");
125-
}
125+
/**
126+
* Gets the song file using a pre-validated root music directory.
127+
* This method avoids multiple validations of the root music directory by
128+
* passing it to the optimized getAlbumDirectory.
129+
*
130+
* @param context The application context.
131+
* @param song The music entry for the song.
132+
* @param validatedRootMusicDir The root music directory, already validated.
133+
* @return The song File object, or null if an error occurs.
134+
*/
135+
public static File getSongFile(Context context, MusicDirectory.Entry song, File validatedRootMusicDir) {
136+
// Call the NEW overloaded getAlbumDirectory, passing the validatedRootMusicDir
137+
File albumDir = getAlbumDirectory(context, song, validatedRootMusicDir);
138+
139+
if (albumDir == null) {
140+
Log.e(TAG, "Optimized getSongFile: Failed to get album directory for " + song.getTitle());
141+
// Fallback or error: if albumDir couldn't be determined even with validated root,
142+
// something is wrong. Could call original getSongFile as a last resort,
143+
// but that defeats optimization for this specific call.
144+
return null; // Or: return getSongFile(context, song); to revert to old behavior for this one case.
145+
}
146+
147+
// Now, use the exact filename construction logic from your ORIGINAL getSongFile method.
148+
// The 'dir' variable in your original getSongFile is now 'albumDir'.
126149

127-
fileName.append(fileSystemSafe(song.getTitle()));
128-
if(fileName.length() >= MAX_FILENAME_LENGTH) {
150+
StringBuilder fileName = new StringBuilder();
151+
Integer track = song.getTrack();
152+
if (track != null) {
153+
if (track < 10) {
154+
fileName.append("0");
155+
}
156+
fileName.append(track).append("-");
157+
}
158+
159+
// Assuming fileSystemSafe is available and MAX_FILENAME_LENGTH is a static final int
160+
fileName.append(fileSystemSafe(song.getTitle()));
161+
if (fileName.length() >= MAX_FILENAME_LENGTH) {
129162
fileName.setLength(MAX_FILENAME_LENGTH);
130163
}
131164

132165
fileName.append(".");
133-
if(song.isVideo()) {
134-
String videoPlayerType = Util.getVideoPlayerType(context);
135-
if("hls".equals(videoPlayerType)) {
166+
// Assuming song.isVideo(), Util.getVideoPlayerType(), song.getSuffix(), song.getTranscodedSuffix()
167+
// are available as per your original getSongFile.
168+
if (song.isVideo()) {
169+
String videoPlayerType = Util.getVideoPlayerType(context); // Assuming Util class
170+
if ("hls".equals(videoPlayerType)) {
136171
// HLS should be able to transcode to mp4 automatically
137172
fileName.append("mp4");
138-
} else if("raw".equals(videoPlayerType)) {
173+
} else if ("raw".equals(videoPlayerType)) {
139174
// Download the original video without any transcoding
140175
fileName.append(song.getSuffix());
141176
}
@@ -147,8 +182,8 @@ public static File getSongFile(Context context, MusicDirectory.Entry song) {
147182
}
148183
}
149184

150-
return new File(dir, fileName.toString());
151-
}
185+
return new File(albumDir, fileName.toString());
186+
}
152187

153188
public static File getPlaylistFile(Context context, String server, String name) {
154189
File playlistDir = getPlaylistDirectory(context, server);
@@ -343,37 +378,94 @@ public static File getArtistDirectory(Context context, MusicDirectory.Entry arti
343378
}
344379

345380
public static File getAlbumDirectory(Context context, MusicDirectory.Entry entry) {
346-
File dir = null;
347-
if (entry.getPath() != null) {
348-
File f = new File(fileSystemSafeDir(entry.getPath()));
349-
String folder = getMusicDirectory(context).getPath();
350-
if(entry.isDirectory()) {
381+
File rootMusicDir = getMusicDirectory(context); // Fetches and validates the root
382+
if (rootMusicDir == null) {
383+
String msg = "getAlbumDirectory: Root music directory is null. Cannot proceed.";
384+
Log.e(TAG, msg);
385+
throw new IllegalStateException(msg);
386+
}
387+
return getAlbumDirectory(context, entry, rootMusicDir);
388+
}
389+
390+
/**
391+
* Gets the album directory using a pre-validated root music directory.
392+
* This method avoids calling getMusicDirectory() again.
393+
*
394+
* @param context The application context.
395+
* @param entry The music entry.
396+
* @param validatedRootMusicDir The root music directory, already validated by a call to getMusicDirectory().
397+
* @return The album directory File object, or null if an error occurs.
398+
*/
399+
public static File getAlbumDirectory(Context context, MusicDirectory.Entry entry, File validatedRootMusicDir) {
400+
if (validatedRootMusicDir == null || !validatedRootMusicDir.isDirectory()) {
401+
Log.e(TAG, "Validated root music directory is invalid. Falling back to original getAlbumDirectory.");
402+
// Fallback to original behavior if the passed directory is problematic
403+
return getAlbumDirectory(context, entry); // Calls the original getAlbumDirectory
404+
}
405+
406+
// Example structure, ADAPT THIS TO YOUR ORIGINAL getAlbumDirectory's LOGIC:
407+
File dir = null;
408+
if (entry.getPath() != null) {
409+
// If entry.getPath() is absolute, this logic might need care.
410+
// If entry.getPath() is relative to the music root, then this is correct:
411+
File f = new File(fileSystemSafeDir(entry.getPath()));
412+
String folder = validatedRootMusicDir.getPath(); // Use the validated one
413+
if (entry.isDirectory()) {
351414
folder += "/" + f.getPath();
352-
} else if(f.getParent() != null) {
415+
} else if (f.getParent() != null) {
353416
folder += "/" + f.getParent();
354417
}
355-
dir = new File(folder);
356-
} else {
418+
dir = new File(folder);
419+
} else {
420+
// This part comes from your original getAlbumDirectory in the initial context
357421
MusicDirectory.Entry firstSong;
358-
if(!Util.isOffline(context)) {
359-
firstSong = lookupChild(context, entry, false);
360-
if(firstSong != null) {
361-
File songFile = FileUtil.getSongFile(context, firstSong);
362-
dir = songFile.getParentFile();
422+
if (!Util.isOffline(context)) { // Assuming Util.isOffline is available
423+
firstSong = lookupChild(context, entry, false); // Assuming Util.lookupChild
424+
if (firstSong != null) {
425+
// If lookupChild gives an entry that getSongFile can determine a path from,
426+
// this needs careful handling to avoid re-calling the non-optimized getSongFile.
427+
// This branch shows complexity if getAlbumDirectory relies on resolving
428+
// a child's path via the full getSongFile -> getAlbumDirectory chain.
429+
430+
// For now, let's assume we can determine the *album* path from 'firstSong'
431+
// relative to 'validatedRootMusicDir'.
432+
// This might mean getSongFile's logic for *album path part* needs to be usable here.
433+
// A simpler approach for this else branch if it's just artist/album:
434+
String artist = fileSystemSafe(entry.getArtist());
435+
String album = fileSystemSafe(entry.getAlbum());
436+
if ("unnamed".equals(album)) { // Using "unnamed" as per your original file
437+
album = fileSystemSafe(entry.getTitle());
438+
}
439+
dir = new File(validatedRootMusicDir, artist + File.separator + album);
363440
}
364441
}
365-
366-
if(dir == null) {
442+
// If dir is still null after the above, construct from artist/album
443+
if (dir == null) {
367444
String artist = fileSystemSafe(entry.getArtist());
368445
String album = fileSystemSafe(entry.getAlbum());
369-
if("unnamed".equals(album)) {
446+
if ("unnamed".equals(album)) {
370447
album = fileSystemSafe(entry.getTitle());
371448
}
372-
dir = new File(getMusicDirectory(context).getPath() + "/" + artist + "/" + album);
449+
// Use validatedRootMusicDir as the base
450+
dir = new File(validatedRootMusicDir, artist + File.separator + album);
373451
}
374-
}
375-
return dir;
376-
}
452+
}
453+
454+
// Crucially, ensure this specific album directory (dir) exists.
455+
// This uses the same mechanism your original getAlbumDirectory would use
456+
// (e.g., mkdirs directly, or it's implicitly created by file operations later).
457+
// If your original getAlbumDirectory calls ensureDirectoryExistsAndIsReadWritable on 'dir',
458+
// then do that here too. If it just relies on mkdirs, that's fine.
459+
// Based on the provided snippets, direct mkdirs() is more likely for subdirectories.
460+
if (!dir.exists()) {
461+
if (!dir.mkdirs()) {
462+
Log.w(TAG, "Failed to create album directory: " + dir.getPath());
463+
// return null; // Or handle error as your original method does
464+
}
465+
}
466+
// If your original method had further checks (isDirectory, canRead, canWrite) on `dir`, replicate them.
467+
return dir;
468+
}
377469

378470
public static MusicDirectory.Entry lookupChild(Context context, MusicDirectory.Entry entry, boolean allowDir) {
379471
// Initialize lookupMap if first time called

‎app/src/main/java/github/daneren2005/dsub/util/SongDBHandler.java‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121
import android.database.Cursor;
2222
import android.database.sqlite.SQLiteDatabase;
2323
import android.database.sqlite.SQLiteOpenHelper;
24+
import android.database.sqlite.SQLiteStatement;
2425

26+
import java.io.File;
2527
import java.util.ArrayList;
2628
import java.util.List;
2729

@@ -107,10 +109,10 @@ protected synchronized void addSongImpl(SQLiteDatabase db, int serverKey, String
107109

108110
public synchronized void addSongs(int instance, List<MusicDirectory.Entry> entries) {
109111
SQLiteDatabase db = this.getWritableDatabase();
110-
112+
File musicDirectory = FileUtil.getMusicDirectory(context);
111113
List<Pair<String, String>> pairs = new ArrayList<>();
112114
for(MusicDirectory.Entry entry: entries) {
113-
pairs.add(new Pair<>(entry.getId(), FileUtil.getSongFile(context, entry).getAbsolutePath()));
115+
pairs.add(new Pair<>(entry.getId(), FileUtil.getSongFile(context, entry, musicDirectory).getAbsolutePath()));
114116
}
115117
addSongs(db, instance, pairs);
116118

@@ -120,15 +122,18 @@ public synchronized void addSongs(SQLiteDatabase db, int instance, List<Pair<Str
120122
addSongsImpl(db, Util.getRestUrlHash(context, instance), entries);
121123
}
122124
protected synchronized void addSongsImpl(SQLiteDatabase db, int serverKey, List<Pair<String, String>> entries) {
125+
String sql = "insert or ignore into " + TABLE_SONGS + " (" + SONGS_SERVER_KEY + ", " + SONGS_SERVER_ID + ", " + SONGS_COMPLETE_PATH + ") values (?, ?, ?);";
126+
123127
db.beginTransaction();
128+
SQLiteStatement stmt = db.compileStatement(sql);
124129
try {
125130
for (Pair<String, String> entry : entries) {
126-
ContentValues values = new ContentValues();
127-
values.put(SONGS_SERVER_KEY, serverKey);
128-
values.put(SONGS_SERVER_ID, entry.getFirst());
129-
values.put(SONGS_COMPLETE_PATH, entry.getSecond());
131+
stmt.bindLong(1, serverKey);
132+
stmt.bindString(2, entry.getFirst());
133+
stmt.bindString(3, entry.getSecond());
130134

131-
db.insertWithOnConflict(TABLE_SONGS, null, values, SQLiteDatabase.CONFLICT_IGNORE);
135+
long entryID = stmt.executeInsert();
136+
stmt.clearBindings();
132137
}
133138

134139
db.setTransactionSuccessful();

0 commit comments

Comments
 (0)