Skip to content

Commit

Permalink
Refactor: Clarify some variable names
Browse files Browse the repository at this point in the history 
Now that we support repo subdirectories, what previously could be
referred to as a "file name" should now (in most places) be referred to
as a "repo-relative path", i.e. a path relative to the repository root.
  • Loading branch information
@amberin
amberin committed Aug 21, 2024
1 parent 120fa64 commit eb4b983
Show file tree
Hide file tree
Showing 25 changed files with 201 additions and 197 deletions.
4 changes: 2 additions & 2 deletions app/src/androidTest/java/com/orgzly/android/repos/DirectoryRepoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void testStoringFile() throws IOException {

assertEquals(1, books.size());
assertEquals("booky", BookName.fromRook(books.get(0)).getName());
assertEquals("booky.org", BookName.fromRook(books.get(0)).getFileName());
assertEquals("booky.org", BookName.fromRook(books.get(0)).getRepoRelativePath());
assertEquals(repoUriString, books.get(0).getRepoUri().toString());
assertEquals(repoUriString + "/booky.org", books.get(0).getUri().toString());
}
Expand All @@ -80,7 +80,7 @@ public void testExtension() throws IOException {

assertEquals(1, books.size());
assertEquals("03", BookName.fromRook(books.get(0)).getName());
assertEquals("03.org", BookName.fromRook(books.get(0)).getFileName());
assertEquals("03.org", BookName.fromRook(books.get(0)).getRepoRelativePath());
assertEquals(13, books.get(0).getRepoId());
assertEquals(repoUriString, books.get(0).getRepoUri().toString());
assertEquals(repoUriString + "/03.org", books.get(0).getUri().toString());
Expand Down
2 changes: 1 addition & 1 deletion app/src/androidTest/java/com/orgzly/android/repos/LocalDbRepoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public void testStoringBook() throws IOException {
try {
new NotesOrgExporter(dataRepository).exportBook(book, tmpFile);
repo = testUtils.repoInstance(RepoType.MOCK, "mock://repo-a");
repo.storeBook(tmpFile, BookName.fileName(book.getName(), BookFormat.ORG));
repo.storeBook(tmpFile, BookName.repoRelativePath(book.getName(), BookFormat.ORG));
} finally {
tmpFile.delete();
}
Expand Down
4 changes: 2 additions & 2 deletions app/src/androidTest/java/com/orgzly/android/repos/RepoIgnoreNodeTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import java.util.HashMap
class RepoIgnoreNodeTest : OrgzlyTest() {

class MockRepoWithMockIgnoreFile : MockRepo(repoWithProps, null) {
override fun openRepoFileInputStream(filePath: String): InputStream {
if (filePath == RepoIgnoreNode.IGNORE_FILE) {
override fun openRepoFileInputStream(repoRelativePath: String): InputStream {
if (repoRelativePath == RepoIgnoreNode.IGNORE_FILE) {
val ignoreFileContents = """
IgnoredAnywhere.org
/OnlyIgnoredInRoot.org
Expand Down
2 changes: 1 addition & 1 deletion app/src/androidTest/java/com/orgzly/android/repos/SyncTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ public void testDirectoryFileRename() throws IOException {

assertEquals(1, repo.getBooks().size());
assertEquals(repo.getUri() + "/notebook-renamed.org", repo.getBooks().get(0).getUri().toString());
assertEquals("notebook-renamed.org", BookName.fromRook(repo.getBooks().get(0)).getFileName());
assertEquals("notebook-renamed.org", BookName.fromRook(repo.getBooks().get(0)).getRepoRelativePath());

LocalStorage.deleteRecursive(new File(repoDir));
}
Expand Down
55 changes: 34 additions & 21 deletions app/src/main/java/com/orgzly/android/BookName.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.orgzly.BuildConfig;
import com.orgzly.android.db.entity.BookView;
import com.orgzly.android.repos.Rook;
import com.orgzly.android.repos.VersionedRook;
import com.orgzly.android.util.LogUtils;

import java.util.regex.Matcher;
Expand All @@ -22,34 +23,39 @@ public class BookName {
private static final Pattern PATTERN = Pattern.compile("(.*)\\.(org)(\\.txt)?$");
private static final Pattern SKIP_PATTERN = Pattern.compile("^\\.#.*");

private final String mFileName;
private final String mRepoRelativePath;
private final String mName;
private final BookFormat mFormat;

private BookName(String fileName, String name, BookFormat format) {
mFileName = fileName;
private BookName(String repoRelativePath, String name, BookFormat format) {
mRepoRelativePath = repoRelativePath;
mName = name;
mFormat = format;
}

public static String getFileName(BookView bookView) {
public static String getRepoRelativePath(BookView bookView) {
if (bookView.getSyncedTo() != null) {
return getFileName(bookView.getSyncedTo().getRepoUri(), bookView.getSyncedTo().getUri());

VersionedRook vrook = bookView.getSyncedTo();
return getRepoRelativePath(vrook.getRepoUri(), vrook.getUri());
} else {
return fileName(bookView.getBook().getName(), BookFormat.ORG);
// There is no remote book; we can only guess the repo path from the book's name.
return repoRelativePath(bookView.getBook().getName(), BookFormat.ORG);
}
}

/**
* Used when creating a Book from an imported file.
* @param context Used for getting a DocumentFile, if possible
* @param uri URI provided by the file picker
* @return The book's file name
*/
public static String getFileName(Context context, Uri uri) {
String fileName;

DocumentFile documentFile = DocumentFile.fromSingleUri(context, uri);

if ("content".equals(uri.getScheme()) && documentFile != null) {
// Try using DocumentFile first (KitKat and above)
fileName = documentFile.getName();

} else { // Just get the last path segment
fileName = uri.getLastPathSegment();
}
Expand All @@ -63,7 +69,7 @@ public static String getFileName(Context context, Uri uri) {
return fileName;
}

public static String getFileName(Uri repoUri, Uri fileUri) {
public static String getRepoRelativePath(Uri repoUri, Uri fileUri) {
/* The content:// repository type requires special handling */
if ("content".equals(repoUri.getScheme())) {
String repoUriLastSegment = repoUri.toString().replaceAll("^.*/", "");
Expand All @@ -79,37 +85,44 @@ public static String getFileName(Uri repoUri, Uri fileUri) {
}

public static BookName fromRook(Rook rook) {
return fromFileName(getFileName(rook.getRepoUri(), rook.getUri()));
return fromRepoRelativePath(getRepoRelativePath(rook.getRepoUri(), rook.getUri()));
}

public static boolean isSupportedFormatFileName(String fileName) {
return PATTERN.matcher(fileName).matches() && !SKIP_PATTERN.matcher(fileName).matches();
public static boolean isSupportedFormatFileName(String path) {
return PATTERN.matcher(path).matches() && !SKIP_PATTERN.matcher(path).matches();
}

public static String fileName(String name, BookFormat format) {
public static String repoRelativePath(String name, BookFormat format) {
if (format == BookFormat.ORG) {
return name + ".org";
} else {
throw new IllegalArgumentException("Unsupported format " + format);
}
}

public static String lastPathSegment(String name, BookFormat format) {
if (format == BookFormat.ORG) {
return Uri.parse(name).getLastPathSegment() + ".org";
} else {
throw new IllegalArgumentException("Unsupported format " + format);
}
}

public static BookName fromFileName(String fileName) {
if (fileName != null) {
Matcher m = PATTERN.matcher(fileName);
public static BookName fromRepoRelativePath(String repoRelativePath) {
if (repoRelativePath != null) {
Matcher m = PATTERN.matcher(repoRelativePath);

if (m.find()) {
String name = m.group(1);
String extension = m.group(2);

if (extension.equals("org")) {
return new BookName(fileName, name, BookFormat.ORG);
return new BookName(repoRelativePath, name, BookFormat.ORG);
}
}
}

throw new IllegalArgumentException("Unsupported book file name " + fileName);
throw new IllegalArgumentException("Unsupported book file name " + repoRelativePath);
}

public String getName() {
Expand All @@ -120,8 +133,8 @@ public BookFormat getFormat() {
return mFormat;
}

public String getFileName() {
return mFileName;
public String getRepoRelativePath() {
return mRepoRelativePath;
}

}
2 changes: 1 addition & 1 deletion app/src/main/java/com/orgzly/android/LocalStorage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public LocalStorage(Context context) {
* @throws IOException if external directory is not available
*/
public File getExportFile(String name, BookFormat format) throws IOException {
return new File(downloadsDirectory(), BookName.fileName(name, format));
return new File(downloadsDirectory(), BookName.repoRelativePath(name, format));
}

/**
Expand Down
22 changes: 11 additions & 11 deletions app/src/main/java/com/orgzly/android/data/DataRepository.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ class DataRepository @Inject constructor(
BookAction.Type.PROGRESS,
resources.getString(R.string.force_loading_from_uri, book.linkRepo.url)))

val fileName = BookName.getFileName(book)
val repoRelativePath = BookName.getRepoRelativePath(book)

val loadedBook = loadBookFromRepo(book.linkRepo.id, book.linkRepo.type, book.linkRepo.url, fileName)
val loadedBook = loadBookFromRepo(book.linkRepo.id, book.linkRepo.type, book.linkRepo.url, repoRelativePath)

setBookLastActionAndSyncStatus(loadedBook!!.book.id, BookAction.forNow(
BookAction.Type.INFO,
Expand All @@ -103,7 +103,7 @@ class DataRepository @Inject constructor(
val book = getBookView(bookId)
?: throw IOException(resources.getString(R.string.book_does_not_exist_anymore))

val repositoryPath: String = BookName.getFileName(book)
val repositoryPath: String = BookName.getRepoRelativePath(book)

try {
/* Prefer link. */
Expand Down Expand Up @@ -386,7 +386,7 @@ class DataRepository @Inject constructor(

/* Do not rename if the new filename will be ignored */
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O)
RepoUtils.ensureFileNameIsNotIgnored(repo, BookName.fileName(name, BookFormat.ORG))
RepoUtils.ensurePathIsNotIgnored(repo, BookName.repoRelativePath(name, BookFormat.ORG))

val movedVrook = repo.renameBook(vrook.uri, name)

Expand Down Expand Up @@ -513,8 +513,8 @@ class DataRepository @Inject constructor(
// Ensure that the resulting file name is not ignored in this repo
val syncRepo = getRepoInstance(repo.id, repo.type, repo.url)
val bookName = getBook(bookId)!!.name
val fileName = BookName.fileName(bookName, BookFormat.ORG)
RepoUtils.ensureFileNameIsNotIgnored(syncRepo, fileName)
val repoRelativePath = BookName.repoRelativePath(bookName, BookFormat.ORG)
RepoUtils.ensurePathIsNotIgnored(syncRepo, repoRelativePath)
}

db.bookLink().upsert(bookId, repoId)
Expand Down Expand Up @@ -1625,23 +1625,23 @@ class DataRepository @Inject constructor(

@Throws(IOException::class)
fun loadBookFromRepo(rook: Rook): BookView? {
val fileName = BookName.getFileName(rook.repoUri, rook.uri)
val repoRelativePath = BookName.getRepoRelativePath(rook.repoUri, rook.uri)

return loadBookFromRepo(rook.repoId, rook.repoType, rook.repoUri.toString(), fileName)
return loadBookFromRepo(rook.repoId, rook.repoType, rook.repoUri.toString(), repoRelativePath)
}

@Throws(IOException::class)
fun loadBookFromRepo(repoId: Long, repoType: RepoType, repoUrl: String, fileName: String): BookView? {
fun loadBookFromRepo(repoId: Long, repoType: RepoType, repoUrl: String, repoRelativePath: String): BookView? {
val book: BookView?

val repo = getRepoInstance(repoId, repoType, repoUrl)

val tmpFile = getTempBookFile()
try {
/* Download from repo. */
val vrook = repo.retrieveBook(fileName, tmpFile)
val vrook = repo.retrieveBook(repoRelativePath, tmpFile)

val bookName = BookName.fromFileName(fileName)
val bookName = BookName.fromRepoRelativePath(repoRelativePath)

/* Store from file to Shelf. */
book = loadBookFromFile(bookName.name, bookName.format, tmpFile, vrook)
Expand Down
Loading

0 comments on commit eb4b983

Please sign in to comment.