Skip to content

[feat] Support scoped storage when sharing images #148 #149

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.

Sign up for GitHub

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

Merged
tuancoltech merged 3 commits into from
Feb 28, 2025
Merged

[feat] Support scoped storage when sharing images #148 #149

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2128c5d
[feat] Support scoped storage when sharing images: https://github.com…
tuancoltech Feb 25, 2025
d19c17b
Add null check for image/imageFile when obtaining from DB/getImageFil…
tuancoltech Feb 25, 2025
9bab2b9
Merge branch 'main' into feat/support_scoped_storage_images_sharing
jo-elimu Feb 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ apply plugin: 'com.android.application'
apply plugin: 'org.ajoberstar.grgit'

android {
compileSdk 34
compileSdk 35
namespace 'ai.elimu.content_provider'

defaultConfig {
applicationId "ai.elimu.content_provider"
minSdkVersion 24
targetSdkVersion 34
targetSdkVersion 35
versionCode 1002029
versionName "1.2.29-SNAPSHOT"
setProperty("archivesBaseName", "${applicationId}-${versionCode}")
Expand Down
7 changes: 6 additions & 1 deletion app/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,12 @@
android:name=".provider.ImageContentProvider"
android:authorities="${applicationId}.provider.image_provider"
android:enabled="true"
android:exported="true" />
android:exported="true"
android:grantUriPermissions="true">
<meta-data
android:name="android.support.FILE_PROVIDER_PATHS"
android:resource="@xml/image_file_path"/>
</provider>
<provider
android:name=".provider.AudioContentProvider"
android:authorities="${applicationId}.provider.audio_provider"
Expand Down
43 changes: 43 additions & 0 deletions app/src/main/java/ai/elimu/content_provider/provider/ImageContentProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@
import android.content.UriMatcher;
import android.database.Cursor;
import android.net.Uri;
import android.os.ParcelFileDescriptor;
import android.util.Log;

import java.io.File;
import java.io.FileNotFoundException;
import java.util.List;

import ai.elimu.content_provider.BuildConfig;
import ai.elimu.content_provider.room.dao.ImageDao;
import ai.elimu.content_provider.room.db.RoomDb;
import ai.elimu.content_provider.room.entity.Image;
import ai.elimu.content_provider.util.FileHelper;

public class ImageContentProvider extends ContentProvider {

Expand All @@ -24,6 +29,7 @@ public class ImageContentProvider extends ContentProvider {
private static final int CODE_IMAGES_BY_WORD_LABEL_ID = 3;

private static final UriMatcher MATCHER = new UriMatcher(UriMatcher.NO_MATCH);
private String TAG = ImageContentProvider.class.getName();

static {
MATCHER.addURI(AUTHORITY, TABLE_IMAGES, CODE_IMAGES);
Expand Down Expand Up @@ -150,4 +156,41 @@ public int delete(Uri uri, String selection, String[] selectionArgs) {

throw new UnsupportedOperationException("Not yet implemented");
}

@Override
public ParcelFileDescriptor openFile(Uri uri, String mode) throws FileNotFoundException {
List<String> segments = uri.getPathSegments();
if (segments.size() < 2) {
throw new FileNotFoundException("Invalid URI: " + uri);
}
String fileId = segments.get(1);

RoomDb roomDb = RoomDb.getDatabase(getContext());
ImageDao imageDao = roomDb.imageDao();

long imageId;
try {
imageId = Long.parseLong(fileId);
} catch (NumberFormatException e) {
Log.e(TAG, "Failed to parse image ID: " + fileId, e);
throw new FileNotFoundException("Invalid image ID format: " + fileId);
}

Image image = imageDao.load(imageId);

if (image == null) {
throw new FileNotFoundException("File not found with id: " + imageId);
}

File imageFile = FileHelper.getImageFile(image, getContext());
if (imageFile == null) {
throw new FileNotFoundException("imageFile not found with id: " + imageId);
}
if (!imageFile.exists()) {
Log.e(TAG, "imageFile doesn't exist: " + imageFile.getAbsolutePath());
throw new FileNotFoundException("File not found: " + imageFile.getAbsolutePath());
}
return ParcelFileDescriptor.open(imageFile, ParcelFileDescriptor.MODE_READ_ONLY);
}
Comment on lines +160 to +194
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add null check for Image object to prevent potential NullPointerException.

If the image is not found in the database, a NullPointerException will occur when calling FileHelper.getImageFile(image, getContext()). Add a null check before accessing the image.

    Image image = imageDao.load(Long.parseLong(fileId));

+    if (image == null) {
+        Log.e(TAG, "Image not found with ID: " + fileId);
+        throw new FileNotFoundException("Image not found with ID: " + fileId);
+    }
    
    File imageFile = FileHelper.getImageFile(image, getContext());
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Override
public ParcelFileDescriptor openFile(Uri uri, String mode) throws FileNotFoundException {
List<String> segments = uri.getPathSegments();
if (segments.size() < 2) {
throw new FileNotFoundException("Invalid URI: " + uri);
}
String fileId = segments.get(1);
RoomDb roomDb = RoomDb.getDatabase(getContext());
ImageDao imageDao = roomDb.imageDao();
Image image = imageDao.load(Long.parseLong(fileId));
File imageFile = FileHelper.getImageFile(image, getContext());
if (imageFile == null) {
throw new FileNotFoundException("File not found!");
}
if (!imageFile.exists()) {
Log.e(TAG, "imageFile doesn't exist: " + imageFile.getAbsolutePath());
throw new FileNotFoundException("File not found: " + imageFile.getAbsolutePath());
}
return ParcelFileDescriptor.open(imageFile, ParcelFileDescriptor.MODE_READ_ONLY);
}
@Override
public ParcelFileDescriptor openFile(Uri uri, String mode) throws FileNotFoundException {
List<String> segments = uri.getPathSegments();
if (segments.size() < 2) {
throw new FileNotFoundException("Invalid URI: " + uri);
}
String fileId = segments.get(1);
RoomDb roomDb = RoomDb.getDatabase(getContext());
ImageDao imageDao = roomDb.imageDao();
Image image = imageDao.load(Long.parseLong(fileId));
if (image == null) {
Log.e(TAG, "Image not found with ID: " + fileId);
throw new FileNotFoundException("Image not found with ID: " + fileId);
}
File imageFile = FileHelper.getImageFile(image, getContext());
if (imageFile == null) {
throw new FileNotFoundException("File not found!");
}
if (!imageFile.exists()) {
Log.e(TAG, "imageFile doesn't exist: " + imageFile.getAbsolutePath());
throw new FileNotFoundException("File not found: " + imageFile.getAbsolutePath());
}
return ParcelFileDescriptor.open(imageFile, ParcelFileDescriptor.MODE_READ_ONLY);
}

Copy link
Copy Markdown
Member Author

@tuancoltech tuancoltech Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai Isn't the null check already there?

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tuancoltech There isn't a null check for the Image object itself after loading it from the database. The current code has two checks:

  1. if (imageFile == null) - This checks if FileHelper.getImageFile() returned null
  2. if (!imageFile.exists()) - This checks if the file actually exists on disk

However, there's no check between retrieving the image from the database and passing it to the FileHelper. If an image with the requested ID doesn't exist in the database, imageDao.load() would return null, and passing that null to FileHelper.getImageFile() could potentially cause a NullPointerException.

Adding the suggested null check would handle this case explicitly with a more descriptive error message.

Copy link
Copy Markdown
Member Author

@tuancoltech tuancoltech Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai This was fixed!

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tuancoltech That's great! I can see you've not only added the null check for the Image object but also improved the error handling with more informative messages and a try-catch block for parsing the ID. Nice work on making the code more robust!

If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!


}
19 changes: 17 additions & 2 deletions app/src/main/java/ai/elimu/content_provider/util/FileHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
import android.content.Context;
import android.os.Environment;

import androidx.annotation.RestrictTo;

import java.io.File;

import ai.elimu.content_provider.room.entity.Image;
import ai.elimu.model.v2.gson.content.AudioGson;
import ai.elimu.model.v2.gson.content.ImageGson;
import ai.elimu.model.v2.gson.content.VideoGson;
Expand All @@ -19,8 +22,20 @@ public static File getImageFile(ImageGson imageGson, Context context) {
return null;
}
File imagesDirectory = context.getExternalFilesDir(Environment.DIRECTORY_PICTURES);
File file = new File(imagesDirectory, imageGson.getId() + "_r" + imageGson.getRevisionNumber() + "." + imageGson.getImageFormat().toString().toLowerCase());
return file;
return new File(imagesDirectory, imageGson.getId()
+ "_r" + imageGson.getRevisionNumber() + "."
+ imageGson.getImageFormat().toString().toLowerCase());
}

@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP_PREFIX)
public static File getImageFile(Image imageGson, Context context) {
if ((imageGson.getId() == null) || (imageGson.getRevisionNumber() == null)) {
return null;
}
File imagesDirectory = context.getExternalFilesDir(Environment.DIRECTORY_PICTURES);
return new File(imagesDirectory, imageGson.getId()
+ "_r" + imageGson.getRevisionNumber() + "."
+ imageGson.getImageFormat().toString().toLowerCase());
}

public static File getAudioFile(AudioGson audioGson, Context context) {
Expand Down
4 changes: 4 additions & 0 deletions app/src/main/res/xml/image_file_path.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<paths>
<external-files-path name="pictures" path="Pictures/" />
</paths>