3

Since the ValueEventListener gets triggered every time the data in the Firebase Database updates, it is also called when a new upload is done while the RecyclerView is showing. The way I have it now causes duplicate entries when this happens, because the Upload objects are already in the mUploads List. And when an upload is done, the whole database gets queried again and all items added to the already existing ArrayList I don't know how to solve this. Should I create a new Arraylist every time the ValueEventListener is triggered or do I need a completely different callback? Also the ValueEventListener interfers with my mAdapter.notifyItemRemoved call because the ValueEventListener gets triggered as soon as I delete something.

public class ImagesActivity extends AppCompatActivity implements ImageAdapter.OnItemClickListener {
private RecyclerView mRecyclerView;
private ImageAdapter mAdapter;

private ProgressBar mProgressCircle;

private DatabaseReference mDatabaseRef;
private FirebaseStorage mStorage;

private List<Upload> mUploads;

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_show_images);

    mUploads = new ArrayList<>();

    mRecyclerView = findViewById(R.id.recycler_view);
    mRecyclerView.setHasFixedSize(true);
    mRecyclerView.setLayoutManager(new LinearLayoutManager(this));

    mProgressCircle = findViewById(R.id.progress_circle);

    mDatabaseRef = FirebaseDatabase.getInstance().getReference("uploads");
    mStorage = FirebaseStorage.getInstance();

    mDatabaseRef.addValueEventListener(new ValueEventListener() {
        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {

            for (DataSnapshot postSnapshot : dataSnapshot.getChildren()) {
                Upload upload = postSnapshot.getValue(Upload.class);
                upload.setKey(postSnapshot.getKey());

                mUploads.add(upload);
            }

            mAdapter = new ImageAdapter(ImagesActivity.this, mUploads);

            mRecyclerView.setAdapter(mAdapter);

            mAdapter.setOnItemClickListener(ImagesActivity.this);

            mProgressCircle.setVisibility(View.INVISIBLE);
        }

        @Override
        public void onCancelled(DatabaseError databaseError) {
            Toast.makeText(ImagesActivity.this, databaseError.getMessage(), Toast.LENGTH_LONG).show();
            mProgressCircle.setVisibility(View.INVISIBLE);
        }
    });
}

@Override
public void onDeleteClick(final int position) {
    Upload selectedItem = mUploads.get(position);
    final String selectedKey = selectedItem.getKey();

    StorageReference imageRef = mStorage.getReferenceFromUrl(selectedItem.getImageUrl());
    imageRef.delete().addOnSuccessListener(new OnSuccessListener<Void>() {
        @Override
        public void onSuccess(Void aVoid) {
            mDatabaseRef.child(selectedKey).removeValue();
            mUploads.remove(position);
            mAdapter.notifyItemRemoved(position);
        }
    });

}
}
Florian Walther
  • 6,237
  • 5
  • 46
  • 104

5 Answers5

5

clear mUploads before adding data in addValueEventListener like below

 mDatabaseRef.addValueEventListener(new ValueEventListener() {
        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {
            mUploads.clear(); //change here

            for (DataSnapshot postSnapshot : dataSnapshot.getChildren()) {
                Upload upload = postSnapshot.getValue(Upload.class);
                upload.setKey(postSnapshot.getKey());

                mUploads.add(upload);
            }

            mAdapter = new ImageAdapter(ImagesActivity.this, mUploads);

            mRecyclerView.setAdapter(mAdapter);

            mAdapter.setOnItemClickListener(ImagesActivity.this);

            mProgressCircle.setVisibility(View.INVISIBLE);
        }
Omkar
  • 3,040
  • 1
  • 22
  • 42
  • Thank you, I will do it like this! Would you say this approach overall is acceptable or would it be better to not query the whole database everytime changes are made? – Florian Walther Jan 31 '18 at 13:08
3

Instead of using a ValueEventListener consider attaching a ChildEventListener. This type of listener gets triggered for the individual child nodes that are added, changed, or removed.

When you initially attach a ChildEventListener its onChildAdded will be called for each child that the listener matches. Then when you add a child node to the database later, the onChildAdded will be called again with only the existing child. This makes it super easy to update the UI, just add the new child to the adapter.

A ChildEventListener also has an onChildChanged event, which gets triggered when a specific child is changed. Here too, the method gets triggered with only the existing child, once again making it easy to update just that child in the adapter.

See listen for child events in the Firebase documentation for more on this type of listener.

Frank van Puffelen
  • 565,676
  • 79
  • 828
  • 807
  • better solution – Ali Faris Jan 31 '18 at 14:07
  • Thank you, sounds like this is exactly what I need! What do you think about my setKey/getkey approach to save the key as a reference to the corresponding Firebase node? I am a beginner and wonder if this is the correct way to temporarily store the key. I don't have the key in the constructor of my Upload class. – Florian Walther Jan 31 '18 at 14:08
  • That approach sounds common. Just be sure to mark the key as `JsonIgnore`, so that it doesn't get written to the database. See https://stackoverflow.com/questions/38860224/obtaining-the-reference-and-key-in-custom-object-firebase-android – Frank van Puffelen Jan 31 '18 at 14:32
  • I found another answer of yours where you use the @Exclude anotation. What's the difference between them? Can I use either of them? – Florian Walther Jan 31 '18 at 21:20
2

Try this

clear arraylist mUploads.clear() before addValueEventListener like below

mUploads.clear();
Ratilal Chopda
  • 4,162
  • 4
  • 18
  • 31
  • Thank you! Would you say I should set the adapter outside of the ValueEventListener and then just call notifyDataSetChanged instead? – Florian Walther Jan 31 '18 at 13:54
1

You can use addListenerForSingleValueEvent() and this will read the data once, if you want to refresh the data, you can call it again, for more info Firebase - read data once

Pavel Poley
  • 5,307
  • 4
  • 35
  • 66
  • This really looks great. The only problem is, that this wont get triggered when a new upload was made. So when I upload something in activity 1 and then quickly go into the RecyclerView activity, the upload wont trigger an update of the adapter. Do I have to add an additional ChildEvenListener for this? – Florian Walther Jan 31 '18 at 13:21
  • If you know when the data might be changed addListenerForSingleValueEvent() again and clear the list before, if you need always listen to Firebase database changes addValueEventListener() and clear the list or check for every item if the item exist, if not add the item to the list – Pavel Poley Jan 31 '18 at 13:32
  • Thank you this makes a lot of sense! – Florian Walther Jan 31 '18 at 13:34
1

I suggest that you use addListenerForSingleValueEvent so that the data will be fetched once and at the end add addChildEventListener and override onChildAdded so new add items will be fetched also , I think this better than clearing the list and re-inflating the recyclerview

checkout the code

mDatabaseRef.addListenerForSingleValueEvent(new ValueEventListener() {
    @Override
    public void onDataChange(DataSnapshot dataSnapshot) {

        for (DataSnapshot postSnapshot : dataSnapshot.getChildren()) {
            Upload upload = postSnapshot.getValue(Upload.class);
            upload.setKey(postSnapshot.getKey());

            mUploads.add(upload);
        }

        mAdapter = new ImageAdapter(ImagesActivity.this, mUploads);
        mRecyclerView.setAdapter(mAdapter);
        mAdapter.setOnItemClickListener(ImagesActivity.this);
        mProgressCircle.setVisibility(View.INVISIBLE);

        mDatabaseRef.addChildEventListener(new ChildEventListener(){
            @Override
            public void onChildAdded(DataSnapshot dataSnapshot, String previousChildName) {
                Upload upload = postSnapshot.getValue(Upload.class);
                upload.setKey(postSnapshot.getKey());
                mUploads.add(upload);
            }

        })
    }

    @Override
    public void onCancelled(DatabaseError databaseError) {
        Toast.makeText(ImagesActivity.this, databaseError.getMessage(), Toast.LENGTH_LONG).show();
        mProgressCircle.setVisibility(View.INVISIBLE);
    }
});
Ali Faris
  • 17,754
  • 10
  • 45
  • 70
  • Ok thank you! Do you think I also should create and set the adapter outside of the ValueEventListener and then simply call notifyDataSetChanged? – Florian Walther Jan 31 '18 at 14:27