Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 139 additions & 0 deletions models/rss_feed.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
package main

import (
"encoding/xml"
"fmt"
"io"
"net/http"
"regexp"
"strings"
)
Comment on lines +1 to +10
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm whether other files in models/ declare a different package.
fd . models -t f -e go -x head -n 2 {}

Repository: cuappdev/chimes-backend

Length of output: 160


Fix package declaration: package main in models/ directory will not compile.

This file declares package main while sibling files in models/ declare package models. Go requires all files in the same directory to share the same package, so this will cause a compilation error.

Refactor by splitting into:

  • models/rss.go — type definitions and helper functions as package models (reusable by handlers and tests).
  • cmd/rss_scraper/main.go — the main entry point that fetches the feed and invokes the library.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@models/rss_feed.go` around lines 1 - 10, The file currently declares "package
main" which conflicts with other files in the models directory; change the
package declaration to "package models" and refactor by moving only library code
(type definitions and helper functions) into a new models file (e.g., rss.go)
under package models, and create a separate command entrypoint
(cmd/rss_scraper/main.go) with "package main" that imports models and implements
main() to fetch the feed and call the library functions; ensure exported
types/functions keep capitalized names used by the new main and update any
imports accordingly (look for the package declaration and functions that
parse/fetch RSS to split between models and the cmd main).


type RSS struct {
Channel Channel `xml:"channel"`
}

type Channel struct {
Items []Item `xml:"item"`
}

type Item struct {
Title string `xml:"title"`
Description string `xml:"description"`
PubDate string `xml:"pubDate"`
}

type Song struct {
Title string
Source string
Artist string
}

type TimeSlot struct {
Time string
Songs []Song
}

type ConcertDay struct {
Title string
TimeSlots []TimeSlot
}

// matches HTML tag
var tagPattern = regexp.MustCompile(`<[^>]+>`)

// removes all HTML tags from a string
func stripTags(s string) string {
return strings.TrimSpace(tagPattern.ReplaceAllString(s, ""))
}

var originPattern = regexp.MustCompile(`\(from "([^"]+)"\)`)

func parseSong(line string) Song {
song := Song{}

title, artist, found := strings.Cut(line, " / ")
if found {
song.Artist = strings.TrimSpace(artist)
}

match := originPattern.FindStringSubmatch(title)

if match != nil {
song.Source = match[1]
title = strings.TrimSpace(originPattern.ReplaceAllString(title, ""))
}
song.Title = title
return song

}
Comment on lines +52 to +69
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

parseSong can miss the Source when it lives in the artist segment.

strings.Cut(line, " / ") splits off the artist, but originPattern is then applied only to title. If the RSS line is shaped like Song Title / Artist Name (from "Movie") — which is common in the Chimes feed — the (from "...") ends up in artist and will be preserved verbatim in Song.Artist while Song.Source stays empty.

🔧 Suggested fix
 func parseSong(line string) Song {
 	song := Song{}
 
+	// Extract source first so it can't be split across title/artist.
+	if match := originPattern.FindStringSubmatch(line); match != nil {
+		song.Source = match[1]
+		line = strings.TrimSpace(originPattern.ReplaceAllString(line, ""))
+	}
+
 	title, artist, found := strings.Cut(line, " / ")
 	if found {
 		song.Artist = strings.TrimSpace(artist)
 	}
-
-	match := originPattern.FindStringSubmatch(title)
-
-	if match != nil {
-		song.Source = match[1]
-		title = strings.TrimSpace(originPattern.ReplaceAllString(title, ""))
-	}
-	song.Title = title
+	song.Title = strings.TrimSpace(title)
 	return song
-
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func parseSong(line string) Song {
song := Song{}
title, artist, found := strings.Cut(line, " / ")
if found {
song.Artist = strings.TrimSpace(artist)
}
match := originPattern.FindStringSubmatch(title)
if match != nil {
song.Source = match[1]
title = strings.TrimSpace(originPattern.ReplaceAllString(title, ""))
}
song.Title = title
return song
}
func parseSong(line string) Song {
song := Song{}
// Extract source first so it can't be split across title/artist.
if match := originPattern.FindStringSubmatch(line); match != nil {
song.Source = match[1]
line = strings.TrimSpace(originPattern.ReplaceAllString(line, ""))
}
title, artist, found := strings.Cut(line, " / ")
if found {
song.Artist = strings.TrimSpace(artist)
}
song.Title = strings.TrimSpace(title)
return song
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@models/rss_feed.go` around lines 52 - 69, parseSong currently only applies
originPattern to the title, so lines like `Title / Artist (from "X")` leave
Song.Source empty and the origin stuck in Song.Artist; update parseSong to check
originPattern against both title and artist: first attempt to extract source
from title (using originPattern.FindStringSubmatch and ReplaceAllString), and if
not found, try the same extraction on artist, removing the matched text from
artist when found; ensure you TrimSpace both title and artist before assigning
Song.Title and Song.Artist and prefer the source found in title over one found
in artist.


// converts HTML into TimeSlot structure
func parseDescription(desc string) []TimeSlot {
var slots []TimeSlot
var current *TimeSlot

var lines []string
for _, current := range strings.Split(desc, "<br>") {
for _, line := range strings.Split(current, "\n") {
lines = append(lines, line)
}
}

for _, line := range lines {
text := stripTags(line)
if text == "" {
continue
}

isHeader := false
switch text {
case "Morning", "Afternoon", "Evening":
isHeader = true
}
if isHeader {
slots = append(slots, TimeSlot{Time: text})
current = &slots[len(slots)-1]
} else if current != nil {
current.Songs = append(current.Songs, parseSong(text))
}
}
return slots
}

func main() {
resp, err := http.Get("https://apps.chimes.cornell.edu/music/rss.xml")
if err != nil {
panic(err)
}
defer resp.Body.Close()
data, err := io.ReadAll(resp.Body)
if err != nil {
panic(err)
}

var rss RSS
if err := xml.Unmarshal(data, &rss); err != nil {
panic(err)
}
Comment on lines +104 to +118
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Set an HTTP timeout and check the response status.

http.Get uses http.DefaultClient, which has no timeout — a slow/hung upstream will block this goroutine indefinitely. Also, a non-2xx response (e.g., 500, 404 HTML error page) will flow straight into xml.Unmarshal and panic with a confusing parse error. Finally, panicking on every transport/parse error is too aggressive for a library-shaped function; return errors so callers (including future HTTP handlers/cron jobs) can retry, log, or degrade gracefully.

🔧 Suggested fix
-func main() {
-	resp, err := http.Get("https://apps.chimes.cornell.edu/music/rss.xml")
-	if err != nil {
-		panic(err)
-	}
-	defer resp.Body.Close()
-	data, err := io.ReadAll(resp.Body)
-	if err != nil {
-		panic(err)
-	}
-
-	var rss RSS
-	if err := xml.Unmarshal(data, &rss); err != nil {
-		panic(err)
-	}
+func FetchConcerts(ctx context.Context, url string) ([]ConcertDay, error) {
+	client := &http.Client{Timeout: 15 * time.Second}
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
+	if err != nil {
+		return nil, err
+	}
+	resp, err := client.Do(req)
+	if err != nil {
+		return nil, fmt.Errorf("fetch rss: %w", err)
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode/100 != 2 {
+		return nil, fmt.Errorf("rss feed returned status %d", resp.StatusCode)
+	}
+	data, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, fmt.Errorf("read rss body: %w", err)
+	}
+	var rss RSS
+	if err := xml.Unmarshal(data, &rss); err != nil {
+		return nil, fmt.Errorf("parse rss xml: %w", err)
+	}
+	// ... build and return []ConcertDay
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@models/rss_feed.go` around lines 104 - 118, The current main function uses
http.Get (which uses http.DefaultClient with no timeout), panics on errors, and
feeds non-2xx responses straight to xml.Unmarshal for the RSS type; change this
to use an http.Client with a reasonable Timeout (e.g., 10s), replace http.Get
with client.Do(req), check resp.StatusCode and return a descriptive error for
any non-2xx status before reading the body, read and unmarshal into the RSS
struct but propagate errors (return error) instead of calling panic so callers
can handle retries/logging; update the function signature (e.g., fetchRSS or
NewRSSFromURL) to return (*RSS, error) and reference the main function, http.Get
usage, resp.StatusCode check, xml.Unmarshal, and RSS type when making these
changes.


var allConcerts []ConcertDay
for _, item := range rss.Channel.Items {
cleanHTML := strings.NewReplacer("&lt;", "<", "&gt;", ">", "&amp;", "&").Replace(item.Description)
day := ConcertDay{
Title: item.Title,
TimeSlots: parseDescription(cleanHTML),
}
allConcerts = append(allConcerts, day)
}

for _, day := range allConcerts {
fmt.Println("=====", day.Title, "=====")
for _, slot := range day.TimeSlots {
fmt.Println(slot.Time)
for _, song := range slot.Songs {
fmt.Printf("%q by %q from %q\n", song.Title, song.Artist, song.Source)
}
}
}
}
Loading