Add support for OPML files with several nested outlines

reader/opml/opml.go

@@ -6,36 +6,21 @@ package opml // import "miniflux.app/reader/opml"
 
 import (
 	"encoding/xml"
+	"strings"
 )
 
 // Specs: http://opml.org/spec2.opml
 type opmlDocument struct {
-	XMLName  xml.Name      `xml:"opml"`
-	Version  string        `xml:"version,attr"`
-	Header   opmlHeader    `xml:"head"`
-	Outlines []opmlOutline `xml:"body>outline"`
+	XMLName  xml.Name              `xml:"opml"`
+	Version  string                `xml:"version,attr"`
+	Header   opmlHeader            `xml:"head"`
+	Outlines opmlOutlineCollection `xml:"body>outline"`
 }
 
 func NewOPMLDocument() *opmlDocument {
 	return &opmlDocument{}
 }
 
-func (o *opmlDocument) GetSubscriptionList() SubcriptionList {
-	var subscriptions SubcriptionList
-	for _, outline := range o.Outlines {
-		if len(outline.Outlines) > 0 {
-			for _, element := range outline.Outlines {
-				// outline.Text is only available in OPML v2.
-				subscriptions = element.Append(subscriptions, outline.Text)
-			}
-		} else {
-			subscriptions = outline.Append(subscriptions, "")
-		}
-	}
-
-	return subscriptions
-}
-
 type opmlHeader struct {
 	Title       string `xml:"title,omitempty"`
 	DateCreated string `xml:"dateCreated,omitempty"`
@@ -43,11 +28,15 @@ type opmlHeader struct {
 }
 
 type opmlOutline struct {
-	Title    string        `xml:"title,attr,omitempty"`
-	Text     string        `xml:"text,attr"`
-	FeedURL  string        `xml:"xmlUrl,attr,omitempty"`
-	SiteURL  string        `xml:"htmlUrl,attr,omitempty"`
-	Outlines []opmlOutline `xml:"outline,omitempty"`
+	Title    string                `xml:"title,attr,omitempty"`
+	Text     string                `xml:"text,attr"`
+	FeedURL  string                `xml:"xmlUrl,attr,omitempty"`
+	SiteURL  string                `xml:"htmlUrl,attr,omitempty"`
+	Outlines opmlOutlineCollection `xml:"outline,omitempty"`
+}
+
+func (o *opmlOutline) IsSubscription() bool {
+	return strings.TrimSpace(o.FeedURL) != ""
 }
 
 func (o *opmlOutline) GetTitle() string {
@@ -78,15 +67,8 @@ func (o *opmlOutline) GetSiteURL() string {
 	return o.FeedURL
 }
 
-func (o *opmlOutline) Append(subscriptions SubcriptionList, category string) SubcriptionList {
-	if o.FeedURL != "" {
-		subscriptions = append(subscriptions, &Subcription{
-			Title:        o.GetTitle(),
-			FeedURL:      o.FeedURL,
-			SiteURL:      o.GetSiteURL(),
-			CategoryName: category,
-		})
-	}
+type opmlOutlineCollection []opmlOutline
 
-	return subscriptions
+func (o opmlOutlineCollection) HasChildren() bool {
+	return len(o) > 0
 }

reader/opml/parser.go

@@ -25,5 +25,21 @@ func Parse(data io.Reader) (SubcriptionList, *errors.LocalizedError) {
 		return nil, errors.NewLocalizedError("Unable to parse OPML file: %q", err)
 	}
 
-	return opmlDocument.GetSubscriptionList(), nil
+	return getSubscriptionsFromOutlines(opmlDocument.Outlines, ""), nil
+}
+
+func getSubscriptionsFromOutlines(outlines opmlOutlineCollection, category string) (subscriptions SubcriptionList) {
+	for _, outline := range outlines {
+		if outline.IsSubscription() {
+			subscriptions = append(subscriptions, &Subcription{
+				Title:        outline.GetTitle(),
+				FeedURL:      outline.FeedURL,
+				SiteURL:      outline.GetSiteURL(),
+				CategoryName: category,
+			})
+		} else if outline.Outlines.HasChildren() {
+			subscriptions = append(subscriptions, getSubscriptionsFromOutlines(outline.Outlines, outline.Text)...)
+		}
+	}
+	return subscriptions
 }

reader/opml/parser_test.go

@@ -38,15 +38,15 @@ func TestParseOpmlWithoutCategories(t *testing.T) {
 
 	subscriptions, err := Parse(bytes.NewBufferString(data))
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
 	}
 
 	if len(subscriptions) != 13 {
-		t.Errorf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 13)
+		t.Fatalf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 13)
 	}
 
 	if !subscriptions[0].Equals(expected[0]) {
-		t.Errorf(`Subscription are different: "%v" vs "%v"`, subscriptions[0], expected[0])
+		t.Errorf(`Subscription is different: "%v" vs "%v"`, subscriptions[0], expected[0])
 	}
 }
 
@@ -75,16 +75,16 @@ func TestParseOpmlWithCategories(t *testing.T) {
 
 	subscriptions, err := Parse(bytes.NewBufferString(data))
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
 	}
 
 	if len(subscriptions) != 3 {
-		t.Errorf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 3)
+		t.Fatalf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 3)
 	}
 
 	for i := 0; i < len(subscriptions); i++ {
 		if !subscriptions[i].Equals(expected[i]) {
-			t.Errorf(`Subscription are different: "%v" vs "%v"`, subscriptions[i], expected[i])
+			t.Errorf(`Subscription is different: "%v" vs "%v"`, subscriptions[i], expected[i])
 		}
 	}
 }
@@ -108,16 +108,16 @@ func TestParseOpmlWithEmptyTitleAndEmptySiteURL(t *testing.T) {
 
 	subscriptions, err := Parse(bytes.NewBufferString(data))
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
 	}
 
 	if len(subscriptions) != 2 {
-		t.Errorf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 2)
+		t.Fatalf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 2)
 	}
 
 	for i := 0; i < len(subscriptions); i++ {
 		if !subscriptions[i].Equals(expected[i]) {
-			t.Errorf(`Subscription are different: "%v" vs "%v"`, subscriptions[i], expected[i])
+			t.Errorf(`Subscription is different: "%v" vs "%v"`, subscriptions[i], expected[i])
 		}
 	}
 }
@@ -146,16 +146,16 @@ func TestParseOpmlVersion1(t *testing.T) {
 
 	subscriptions, err := Parse(bytes.NewBufferString(data))
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
 	}
 
 	if len(subscriptions) != 2 {
-		t.Errorf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 2)
+		t.Fatalf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 2)
 	}
 
 	for i := 0; i < len(subscriptions); i++ {
 		if !subscriptions[i].Equals(expected[i]) {
-			t.Errorf(`Subscription are different: "%v" vs "%v"`, subscriptions[i], expected[i])
+			t.Errorf(`Subscription is different: "%v" vs "%v"`, subscriptions[i], expected[i])
 		}
 	}
 }
@@ -180,16 +180,58 @@ func TestParseOpmlVersion1WithoutOuterOutline(t *testing.T) {
 
 	subscriptions, err := Parse(bytes.NewBufferString(data))
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
 	}
 
 	if len(subscriptions) != 2 {
-		t.Errorf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 2)
+		t.Fatalf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 2)
 	}
 
 	for i := 0; i < len(subscriptions); i++ {
 		if !subscriptions[i].Equals(expected[i]) {
-			t.Errorf(`Subscription are different: "%v" vs "%v"`, subscriptions[i], expected[i])
+			t.Errorf(`Subscription is different: "%v" vs "%v"`, subscriptions[i], expected[i])
+		}
+	}
+}
+
+func TestParseOpmlVersion1WithSeveralNestedOutlines(t *testing.T) {
+	data := `<?xml version="1.0"?>
+	<opml xmlns:rssowl="http://www.rssowl.org" version="1.1">
+		<head>
+			<title>RSSOwl Subscriptions</title>
+			<dateCreated>星期二, 26 四月 2022 00:12:04 CST</dateCreated>
+		</head>
+		<body>
+			<outline text="My Feeds" rssowl:isSet="true" rssowl:id="7">
+				<outline text="Some Category" rssowl:isSet="false" rssowl:id="55">
+					<outline type="rss" title="Feed 1" xmlUrl="http://example.org/feed1/" htmlUrl="http://example.org/1"></outline>
+					<outline type="rss" title="Feed 2" xmlUrl="http://example.org/feed2/" htmlUrl="http://example.org/2"></outline>
+				</outline>
+				<outline text="Another Category" rssowl:isSet="false" rssowl:id="87">
+					<outline type="rss" title="Feed 3" xmlUrl="http://example.org/feed3/" htmlUrl="http://example.org/3"></outline>
+				</outline>
+			</outline>
+		</body>
+	</opml>
+	`
+
+	var expected SubcriptionList
+	expected = append(expected, &Subcription{Title: "Feed 1", FeedURL: "http://example.org/feed1/", SiteURL: "http://example.org/1", CategoryName: "Some Category"})
+	expected = append(expected, &Subcription{Title: "Feed 2", FeedURL: "http://example.org/feed2/", SiteURL: "http://example.org/2", CategoryName: "Some Category"})
+	expected = append(expected, &Subcription{Title: "Feed 3", FeedURL: "http://example.org/feed3/", SiteURL: "http://example.org/3", CategoryName: "Another Category"})
+
+	subscriptions, err := Parse(bytes.NewBufferString(data))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(subscriptions) != 3 {
+		t.Fatalf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 3)
+	}
+
+	for i := 0; i < len(subscriptions); i++ {
+		if !subscriptions[i].Equals(expected[i]) {
+			t.Errorf(`Subscription is different: "%v" vs "%v"`, subscriptions[i], expected[i])
 		}
 	}
 }
@@ -213,16 +255,16 @@ func TestParseOpmlWithInvalidCharacterEntity(t *testing.T) {
 
 	subscriptions, err := Parse(bytes.NewBufferString(data))
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
 	}
 
 	if len(subscriptions) != 1 {
-		t.Errorf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 1)
+		t.Fatalf("Wrong number of subscriptions: %d instead of %d", len(subscriptions), 1)
 	}
 
 	for i := 0; i < len(subscriptions); i++ {
 		if !subscriptions[i].Equals(expected[i]) {
-			t.Errorf(`Subscription are different: "%v" vs "%v"`, subscriptions[i], expected[i])
+			t.Errorf(`Subscription is different: "%v" vs "%v"`, subscriptions[i], expected[i])
 		}
 	}
 }