Thread that monitors a web API to get the latest announcement via JSON

Question

I'm working to make a thread that monitors a web API to get the latest announcement via JSON. I cannot test this currently, so I'm unsure if anything needs to be changed with this. I've read through other questions but everyone else doesn't seem to be using a loop to keep getting a response.

public void run(){
    try {
        URL url = new URL(announcementsURL);
        HttpURLConnection http = (HttpURLConnection) url.openConnection();
        http.setRequestMethod("GET");
        http.setRequestProperty("Connection", "keep-alive");
        http.setUseCaches(false);
        http.setAllowUserInteraction(false);
        http.setConnectTimeout(10);
        http.setReadTimeout(10);
        while (true){
            http.connect();
            int status = http.getResponseCode();
            if (status == 201){
                BufferedReader br = new BufferedReader(new InputStreamReader(http.getInputStream()));
                StringBuilder sb = new StringBuilder();
                String line;
                while ((line = br.readLine()) != null) {
                    sb.append(line+"\n");
                }
                br.close();
                String json = sb.toString();
                JSONParser parser = new JSONParser();
                JSONObject jsonResponse = (JSONObject) parser.parse(json);
                if (!(lastAnnouncement == (long) jsonResponse.get("time"))){
                    //String announcement = (String) jsonResponse.get("message");
                    //TODO What to do with announcement...
                }
            }
            http.getInputStream().close();
            http.disconnect();
        }

    } catch (IOException | ParseException e) {
        e.printStackTrace();
        this.interrupt();
        try {
            this.join();
        } catch (InterruptedException e1) {
            e1.printStackTrace();
        }
    }
}

Answer 1

Immediate observation: 201 CREATED is a really weird status code to expect here. The GET method is supposed to be both safe and idempotent. 201 is supposed to indicate that a new resource has been created in response to the request. Those don't go together.

Also, if you are polling for changes, you'll normally want to send a "conditional GET". The web server should be providing you with an ETag that you can use, and your client should be sending an If-None-Match header with the previously seen ETag, allowing the server to save some work and bandwidth by returning a 304 NOT MODIFIED

This implementation is also blasting the host with new connections. Check your requirements -- given that you are talking to the server across the network, there is clearly some acceptable latency. If the SLA really is so tight that you have to poll the host constantly, then you should look into Long Polling; if it isn't, then your loop should be spacing out the requests.

You didn't include anything but your run method, but this code fragment

    e.printStackTrace();
    this.interrupt();
    try {
        this.join();
    } catch (InterruptedException e1) {
        e1.printStackTrace();
    }

Stongly suggests that you've written a Thread. Don't do that. Let the standard libraries do the work of managing the threads. Instead, you should create a Runnable, and submit it to an ExecutorService to do the work.

A ScheduledExecutorService supports scheduledAtFixedDelay. You could use that to poll the server at a nice friendly interval, rather than writing a tight loop that tries to DOS the host all by its lonesome.

Your run() method is doing too many different things, and should be refactored. Single responsibility principle suggests that methods should either create, coordinate, or calculate. This code is doing all three -- you are coordinating the client server protocol, managing an http connection and an http input stream, creating a string buffer and a json parser, reading the response entity, parsing the entity, doing some unspecified work with the result.... Create different methods (possibly in different objects) to share the work.