Skip to content
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

Feed name update not updating in RSS items #1075

Closed
proninyaroslav opened this issue Mar 9, 2022 · 2 comments · Fixed by #1076
Closed

Feed name update not updating in RSS items #1075

proninyaroslav opened this issue Mar 9, 2022 · 2 comments · Fixed by #1076

Comments

@proninyaroslav
Copy link
Contributor

proninyaroslav commented Mar 9, 2022

Problem:

  1. Update the feed name in the app or on the server side.
  2. Open the feed and look at its items. Each item has the name of the feed it belongs to.
  3. After the first feed name changing, most likely you will see that it has also changed in the items. But if you change the name again without close the app, then this will no longer affect the name in the items, it will remain the same.
Record_09032022_122302.mp4

I tried to research this issue. Next, I will give pieces of the problematic code.
So, the RssItem entity is responsible for information about visible items. This class contains a "to-one" relationship with the Feed entity.

When the UI code wants to get information about a feed that belongs to an item (for example, to display the name of the feed for that item), it calls this method:

/** To-one relationship, resolved on first access. */
public Feed getFeed() {
long __key = this.feedId;
if (feed__resolvedKey == null || !feed__resolvedKey.equals(__key)) {
if (daoSession == null) {
throw new DaoException("Entity is detached from DAO context");
}
FeedDao targetDao = daoSession.getFeedDao();
Feed feedNew = targetDao.load(__key);
synchronized (this) {
feed = feedNew;
feed__resolvedKey = __key;
}
}
return feed;
}

It just fetch the Feed entity from the DB and caches it. When we call this method again, it will fetch the Feed entity from its cache (and also saves the feed id to the feed__resolvedKey variable). Therein lies the problem.

When we open the list of items of the selected feed, or there is a synchronization, this DB ORM method is called:

public List<RssItem> getCurrentRssItemView(int page) {
String where_clause = ", " + CurrentRssItemViewDao.TABLENAME + " C "
+ " WHERE C." + CurrentRssItemViewDao.Properties.RssItemId.columnName + " = T."
+ RssItemDao.Properties.Id.columnName
+ " AND C._id > " + page * PageSize + " AND c._id <= " + ((page+1) * PageSize)
+ " ORDER BY C." + CurrentRssItemViewDao.Properties.Id.columnName;
return daoSession.getRssItemDao().queryRaw(where_clause);
}

It creates a raw DB query to get the RssItem entities for the selected feed (or any other fragment page like non-read or favourite items). I'm new to greenDAO but I tried to learn its behavior. Apparently, on the first query to get RssItem, it caches the entities. When a sync occurs, or we open the fragment with the items list, we get cached RssItem entities. To be more precise, we get entities with a cached Feed entity inside each. By calling the RssItem#getFeed() method, we get the old cached data, including the old feed name.

Also, RssItem has a setFeed() method which is used in RssItemDao to retrieve a deep copy of RssItem:


/** A raw-style query where you can pass any WHERE clause and arguments. */
public List<RssItem> queryDeep(String where, String... selectionArg) {
Cursor cursor = db.rawQuery(getSelectDeep() + where, selectionArg);
return loadDeepAllAndCloseCursor(cursor);
}

protected RssItem loadCurrentDeep(Cursor cursor, boolean lock) {
RssItem entity = loadCurrent(cursor, 0, lock);
int offset = getAllColumns().length;
Feed feed = loadCurrentOther(daoSession.getFeedDao(), cursor, offset);
if (feed != null) {
entity.setFeed(feed);
}
return entity;
}

So I verified that if I call RssItemDao#queryDeep() instead of RssItemDao#queryRaw() in the ORM, then I get the actual feed name, not the old data, because now the DAO is freshly sets Feed entities within RssItem entities.

It turns out that the problem is either in the caching of entities by the DAO itself, or the problem is in caching the "to-one" relationship with the Feed entity inside the RssItem entity. One way or another, this creates a problem when we don't get up-to-date data, for example, about the feed name.

Possible solutions:

I don't know how to do it right, but I see two ways out of this situation and would like to discuss it:

  1. Always use the RssItemDao#queryDeep() method, although this is likely to create overhead if you have many items.
  2. Consider a cache invalidation mechanism after synchronization. Perhaps greenDAO has the necessary settings and methods for this, or we need to implement it manually.

So far, I haven't submitted any fixes to the RP, as I don't know the possible subtleties and problems that these changes will lead to, discussion with the maintainers (@David-Development) is necessary.

@David-Development
Copy link
Member

@proninyaroslav Thank you for the thorough ticket. I wasn't really aware of that caching behavior.

I just went through the docs and saw that they have an option to clear the session's cache daoSession.clear();

Wondering if you could call that in the OwnCloudSyncAdapter once the sync is finished..? Not sure if it has other side-effects with the existing data visible in the app. Otherwise I agree, queryDeep seems like a good alternative. Not sure what the performance implications are but we are not constantly querying so it shouldn't be a huge issue.

@proninyaroslav
Copy link
Contributor Author

Ok, I'll try both options, although deepQuery is probably a cleaner way, there are no hidden or additional conditions like cleaning DAO, which you may not know or forget about when writing new code or maintaining old.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants