Article voting schema

System I'm building has Users and Articles. Users can upvote/downvote Articles and cancel their votes if they want to. Each vote changes article's score. Score equals to sum of all vote values together.

So I created three models: User, Article, ArticleVote. Even though the most natural way for me would be to be able to do user.upvote(article), for separation of concerns reasons I've put it in ArticleVote model:

class ArticleVote < ActiveRecord::Base
  belongs_to :article
  belongs_to :user

  before_destroy :before_destroy

  validates :value, inclusion: {in: [1, -1]}

  def self.upvote(user, article)
    cast_vote(user, article, 1)
  end

  def self.downvote(user, article)
    cast_vote(user, article, -1)
  end

private

  def self.cast_vote(user, article, value)
    vote = ArticleVote.where(user_id: user.id, article_id: article.id).first_or_initialize
    vote.value = value
    vote.save!
    article.score += value
    article.save!
  end

  def before_destroy
    article.score -= value
    article.save
  end
end

So now I call upvote via ArticleVote.upvote(user, article) which doesn't look too bad, but because of Rails' behavior, this doesn't work very good.

If I want to cancel an upvote for example:

article = Article.find(528953)
# article.score = 5 at this point (for example)
vote = ArticleVote.where(user: current_user, article: article).first.destroy
# article.score = 5 !!!!
article.reload
# article.score = 4

So even though I already fetched an Article from database, and used instance of it to find ArticleVote I have to fetch it again to get the actual score value. I would expect ActiveRecord to automatically set vote.article to reference my article, so when I modify it's state, I wouldn't have to fetch it once again.

Is there a way I can avoid this unnecessary article.reload call and still have valid article.score value?

(Using Rails version 4)

Rails just isn't taking chances, so when you call vote.article it's performing a new DB query, and thus creating a new instance rather than grabbing a cache. And that's a good thing! You might run into trouble otherwise, since you call save. If it was the same Article instance everywhere, you'd suddenly be calling save on it from a non-obvious point in the code.

I.e. pretend that Rails does reuse the same Article instance, and that instance has other unsaved changes. Perhaps these changes aren't valid, and in that case, the save call in ArticleVote would fail although you're only trying to change the article's score. It would also update the article's updated_at timestamp, although that may not be relevant; an article receiving or losing a vote doesn't really mean that the article itself was updated.

So, for one I'd recommend using article.update_column(:score, article.score - value) in you before_destroy method. And, yes, you'll still need to call reload elsewhere.

However, the underlying issue is that you're modifying an article "from the outside". I'd recommend a different approach altogether, with no articles.score column in the database, and using Rails' caching API instead:

class Article
  has_many :votes, class_name: "ArticleVotes"

  # score is not part of the DB schema;
  # it's computed and cached as necessary
  def score
    # get or update the cached score
    Rails.cache.fetch(score_cache_key) { votes.sum(:value) }
  end

  def flush_score_cache
    Rails.cache.delete(score_cache_key)
  end

  # let's not use or override the built-in
  # #cache_key method for this, since that
  # relies on the updated_at timestamp.
  def score_cache_key
    "article/#{id}/score" # or something similar with the ID in it
  end
end

(In the above, you may want to handle the possibility of the record being new, and thus having a nil id.)

And in ArticleVote:

class ArticleVote
  after_commit :flush_article_score

private

  def flush_article_score
    article.flush_score_cache
  end
end

Now whenever a vote is added, removed or updated, the article's score cache will be busted, forcing a article.score to do a proper recount if/when it's called.

This also has the advantage that the article's score remains consistent with the actual votes: It's computed entirely from what's in the database. Alternatively, you can simply increment/decrement the cached value instead of flushing it, and still avoid having a database column.

Of course, there are some drawbacks too. Since it is a cache the usual disclaimers apply (you have to be diligent about busting it when necessary, or you'll hit a stale cache, etc..). And if you've got heavy voting traffic you might end up busting the cache so often it just always does the SUM() query. In that case, you could still go with your current solution of having a database column that you increment and decrement, but I might still want to stick that in a different table to avoid the aforementioned issues with implicitly calling save on an article (or, like now, overwriting an updated score with a older one if you forget to reload). You could also consider a more "raw SQL" approach of actually incrementing or decrementing rather than writing the value directly, i.e. UPDATE ... SET score=score + 1 WHERE .... That would avoid race conditions. Optimally, you might do a bit of both: A database column holding the canonical score of an article on disk, and a memory cache you use most of the time for reads, so you only have to hit the database for writes.

StackExchange Code Review Q#45781, answer score: 5