-
Notifications
You must be signed in to change notification settings - Fork 2
Allow custom RSS processing of non-standard feeds #26
base: master
Are you sure you want to change the base?
Conversation
src/main/java/edu/wisc/my/rssToJson/controller/RssToJsonController.java
Outdated
Show resolved
Hide resolved
HttpClient client = HttpClientBuilder.create().build(); | ||
HttpGet request = new HttpGet(endpointURL); | ||
request.setHeader(HttpHeaders.USER_AGENT, "rss-to-json service"); | ||
HttpResponse response = client.execute(request); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we're copying this, let's improve this a little bit, cause in some cases this can result in a memory leak. We'll need to add a finally
to this to close the response. If the connection errors after getting the entity, the connection won't be released and that'll be a memory leak.
See point 3 here: http://hc.apache.org/httpcomponents-client-ga/quickstart.html
But actually, point 4 shows an example of using the fluent API which would let us avoid this case in the first place.
src/main/java/edu/wisc/my/rssToJson/service/RsstoJsonServiceImpl.java
Outdated
Show resolved
Hide resolved
return getClassContext()[1].getPackage().getName(); | ||
} | ||
} | ||
private static String toTitleCase(String filterNameIn){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat, but it only handles a narrow case of one word keys? (like I don't think it would handle working-at-uw
)
Maybe want to pull in commons-text for this? https://commons.apache.org/proper/commons-text/apidocs/org/apache/commons/text/CaseUtils.html#toCamelCase(java.lang.String,boolean,char...)
protected final Logger logger = LoggerFactory.getLogger(getClass()); | ||
public static iFilter getXmlFilter(String filterName){ | ||
try{ | ||
String filterClass = XmlFilter.toTitleCase(filterName) + "Filter"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting loader here. I'm hesitant about passing user input (a piece of the path from a incoming request) directly into a function that loads arbitrary classes... But as the code is structured right now, the string wouldn't get to this line without already being a key in endpoint.properties
, so I guess it's just a potential sharp edge to keep in mind in future refactors.
A way I'd feel more comfortable with would be to somehow get Spring to handle the class loading, or if we absolutely had to do it manually, get all the possible Filter class handles at startup, and load into a map with approved keys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be feasible to ask the Spring ApplicationContext for a named bean, if we want to register and dereference filter singletons that way.
} | ||
|
||
public String getEndpointURL(String feed) { | ||
logger.error("GETTING THE ENDPOINT FOR " + feed); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of these logging levels are too high, it looks like these should be debug logs, not error
} | ||
}; | ||
String responseBody = httpclient.execute(httpget, responseHandler); | ||
logger.error("ONE POINT SIX " + responseBody); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe don't need these numbered logs after development
Using reflection, allow a filter class to be associated to an endpoint. We can then write custom code and force non-standard content into a consumable RSS feed.
For example, the Union's event calendar is not in a standard rss format.
https://union.wisc.edu/events-and-activities/event-calendar/feed/wud.xml
When hitting ...
{hostname}/rssTransform/wud/xml
the /xml at the end of the URL tells the microservice to seek out a filter class with the name "wud".