Add an RSS feed for https://metrics.torproject.org/news.html

added actualpoints::2.5 component::metrics/website owner::irl priority::medium resolution::fixed severity::normal status::closed type::enhancement labels

Yes, this is something we'd like to provide.

Couple of questions regarding a possible implementation:

What formats and versions should we provide? I'm not using RSS/Atom feeds myself, so I can't say what's most useful. Nor can I say what formats one would provide when starting to provide a feed today. So, would we want RSS or Atom or both or something else? And which version or versions would we want to provide?
Are there specific parts of news that we should include, and are there ways to provide data in ways that are easier (or unnecessarily harder) to process for feed clients?
Are there (implicit) conventions regarding feed URLs? For example, should we provide the RSS feed for https://metrics.torproject.org/news.html under something like https://metrics.torproject.org/news.rss, or are there no such conventions?
What do people usually use as library for creating feeds in Java? A quick search reveals ROME as possible candidate: "ROME is a Java framework for RSS and Atom feeds. It's open source and licensed under the Apache 2.0 license." But I have no idea if I'm overlooking an obviously better library.

Thanks for any input on these questions. Any (useful) answers on these will speed up the implementation process.

Trac:
Cc: N/A to irl

Moving into my queue.

Trac:
Owner: metrics-team to irl
Status: new to accepted

Done a little hacking on this, I think my plan looks like:

Add a formatAsFeedItem method to org.torproject.metrics.web.News
Point the resource /feed.xml at NewsServlet
Add a JSP named feed.jsp, which is used instead of news.jsp to render the feed version
Implement only ATOM (RFC4287), not RSS, as ATOM has far better support for internationalisation that may be used later, where internationalising RSS may hold back that work

This would be the way to do it to match the way the news page is currently built. I do wonder though if perhaps we should instead refactor the way the news page works:

Remove the formatAsTableRow method from org.torproject.metrics.web.News
Make getters for org.torproject.metrics.news.News public
Re-implement formatAsTableRow using JSTL in news.jsp
Point the resource /feed.xml at NewsServlet
Add a feed.jsp using JSTL

We definitely do not need to add ROME, or a similar library, at least for now. As an example to get an idea, the RSS 0.91 hacky JSP (where I made getDescription public so that it could be available):

<%@ page contentType="text/xml" %><?xml version="1.0"?>
<%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %>
<%@ taglib prefix="fn" uri="http://java.sun.com/jsp/jstl/functions" %>
<rss version="0.91">
<channel>
    <title>Tor Metrics - News</title>
    <link>https://metrics.torproject.org/</link>
    <c:forEach var="entry" items="${news}">
      <item>
        <title>
          <c:out value="${entry.description}"/>
        </title>
      </item>
    </c:forEach>
</channel>

karsten, iwakeh - Which approach do you think would be best?

karsten, iwakeh - Please see comment:4, I forgot to add metrics-team back to CC when taking the ticket.

Trac:
Cc: irl to metrics-team

Replying to irl:

Done a little hacking on this, I think my plan looks like:

Add a formatAsFeedItem method to org.torproject.metrics.web.News

Point the resource /feed.xml at NewsServlet

Add a JSP named feed.jsp, which is used instead of news.jsp to render the feed version

Implement only ATOM (RFC4287), not RSS, as ATOM has far better support for internationalisation that may be used later, where internationalising RSS may hold back that work

Sounds good!

This would be the way to do it to match the way the news page is currently built. I do wonder though if perhaps we should instead refactor the way the news page works:

Remove the formatAsTableRow method from org.torproject.metrics.web.News

Make getters for org.torproject.metrics.news.News public

Re-implement formatAsTableRow using JSTL in news.jsp

Point the resource /feed.xml at NewsServlet

Add a feed.jsp using JSTL

Hmm, I'm not sure how complex that JSTL variant of formatAsTableRow would become. Also keep in mind that we're using formatted news items in NewsServlet and GraphServlet.

We definitely do not need to add ROME, or a similar library, at least for now. As an example to get an idea, the RSS 0.91 hacky JSP (where I made getDescription public so that it could be available):

{{{ <%@ page contentType="text/xml" %> <%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %> <%@ taglib prefix="fn" uri="http://java.sun.com/jsp/jstl/functions" %> Tor Metrics - News https://metrics.torproject.org/ <c:forEach var="entry" items="${news}"> <c:out value="${entry.description}"/> </c:forEach> }}}

Great that we don't need to add a library.

karsten, iwakeh - Which approach do you think would be best?

I'd say let's start with the first plan. Thanks for researching these alternatives!

Ah ok, I hadn't seen it used in GraphServlet also. Will go for the first plan.

I have implemented the first plan in my [branch]. There are a couple of special cases that I'd like to not have to deal with though.

Currently the news.json assumes that the output will be HTML and that it's OK to have HTML inline, but ATOM is XML and has a different root namespace.

Can you point me at the tool you're using to import changes from the wiki? I wonder if I can do a little sanitising of the data there that would greatly simplify the output of the ATOM and remove the special cases.

Trac:
UpdateNews.java

Replying to irl:

Can you point me at the tool you're using to import changes from the wiki? I wonder if I can do a little sanitising of the data there that would greatly simplify the output of the ATOM and remove the special cases.

Please find it attached. I'm using jq "." news-raw.json > news.json to produce the final output.

If you feel like cleaning up this code a bit and adding it to the tree, great. But feel free to send me a modified version back, and we'll add it to the repository when we have more time.

The first approach is making me write code that makes me very sad. It hides presentation steps as data pre-processing steps.

I'd like to take the second approach instead, with the reusable portion (each row of the table) inserted as a include in the JSPs for GraphServlet and NewsServlet.

This has benefits later for localisation, for the styleguide updates, and it would be really easy to add an iCal feed in addition to the RSS/ATOM.

I'd also like to change the format of news.json to not presume that output is going to only be HTML. Does anything else consume news.json other than GraphServlet and NewsServlet?

Alright, please go ahead. (Nothing else consumes news.json than the two classes you mention.)

Cool. (:

Please review my branch https://gitweb.torproject.org/user/irl/metrics-web.git/log/?h=task/23854. It has been rebased onto master just now, and the feed tested in Firefox/Thunderbird. Don't forget to make sure I haven't broken NewsServlet or GraphServlet's existing functionality (I don't think I have).

Note that for the JSTL rendering the table rows in the news and graph JSPs, this is a static include to have it happen at translation time.

Trac:
Status: accepted to needs_review

Alright, I did a first review by reading commits without trying them out yet. Here's what I found:

You should read about "Java diamond operator". In several places you're undoing changes that we made quite recently when switching to Java 7. For example, we prefer List<String> aList = new ArrayList<>(); over `List aList = new ArrayList();'.
Please add a space after the closing bracket in return (String[]) this.protocols.toArray() and related places.
I think your code prints a news entry with start == end as "start to end", whereas the current code would print it as just "start". Can you change that back?
You picked non-standard Java variable names like short_desc and fixed them in a later commit. Can you rather make such fixes to your own commits as "fixup" or "squash" commits and either squash them yourself or let me do that as part of the merge process?

Trac:
Status: needs_review to needs_revision

Replying to karsten:

You should read about "Java diamond operator". In several places you're undoing changes that we made quite recently when switching to Java 7. For example, we prefer List<String> aList = new ArrayList<>(); over `List aList = new ArrayList();'.

Fixed.

That's going to be a hard habit to break. Is there something we can add to checkstyle to catch this?

Please add a space after the closing bracket in return (String[]) this.protocols.toArray() and related places.

Fixed.

I think your code prints a news entry with start == end as "start to end", whereas the current code would print it as just "start". Can you change that back?

Fixed.

You picked non-standard Java variable names like short_desc and fixed them in a later commit. Can you rather make such fixes to your own commits as "fixup" or "squash" commits and either squash them yourself or let me do that as part of the merge process?

Fixed.

Trac:
Status: needs_revision to needs_review

Trac:
Actualpoints: N/A to 2.5

Looks good! Merged and deployed! Yay. :)

Regarding checkstyle and the diamond operator, I don't know whether we can put in a check specifically for that.

Closing. Thanks!

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

closed

added 20h of time spent

mentioned in issue #25571 (moved)

Add an RSS feed for https://metrics.torproject.org/news.html

Child items 0

Activity