From c26787f47667f69a2d3e85db01fe20b9bc70bc9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Guillot?= Date: Mon, 20 Nov 2017 19:11:06 -0800 Subject: [PATCH] Improve OPML package to be more idiomatic --- reader/opml/handler.go | 39 +++++++++++++++++------------- reader/opml/opml.go | 18 +++++++------- reader/opml/parser.go | 6 ++--- reader/opml/serializer.go | 12 ++++----- reader/opml/subscription.go | 6 ++++- server/routes.go | 5 ++-- server/ui/controller/controller.go | 4 +-- 7 files changed, 50 insertions(+), 40 deletions(-) diff --git a/reader/opml/handler.go b/reader/opml/handler.go index 6150d919..8ba27368 100644 --- a/reader/opml/handler.go +++ b/reader/opml/handler.go @@ -7,21 +7,24 @@ package opml import ( "errors" "fmt" - "github.com/miniflux/miniflux2/model" - "github.com/miniflux/miniflux2/storage" "io" "log" + + "github.com/miniflux/miniflux2/model" + "github.com/miniflux/miniflux2/storage" ) -type OpmlHandler struct { +// Handler handles the logic for OPML import/export. +type Handler struct { store *storage.Storage } -func (o *OpmlHandler) Export(userID int64) (string, error) { - feeds, err := o.store.GetFeeds(userID) +// Export exports user feeds to OPML. +func (h *Handler) Export(userID int64) (string, error) { + feeds, err := h.store.GetFeeds(userID) if err != nil { log.Println(err) - return "", errors.New("Unable to fetch feeds.") + return "", errors.New("unable to fetch feeds") } var subscriptions SubcriptionList @@ -37,27 +40,28 @@ func (o *OpmlHandler) Export(userID int64) (string, error) { return Serialize(subscriptions), nil } -func (o *OpmlHandler) Import(userID int64, data io.Reader) (err error) { +// Import parses and create feeds from an OPML import. +func (h *Handler) Import(userID int64, data io.Reader) (err error) { subscriptions, err := Parse(data) if err != nil { return err } for _, subscription := range subscriptions { - if !o.store.FeedURLExists(userID, subscription.FeedURL) { + if !h.store.FeedURLExists(userID, subscription.FeedURL) { var category *model.Category if subscription.CategoryName == "" { - category, err = o.store.GetFirstCategory(userID) + category, err = h.store.GetFirstCategory(userID) if err != nil { log.Println(err) - return errors.New("Unable to find first category.") + return errors.New("unable to find first category") } } else { - category, err = o.store.GetCategoryByTitle(userID, subscription.CategoryName) + category, err = h.store.GetCategoryByTitle(userID, subscription.CategoryName) if err != nil { log.Println(err) - return errors.New("Unable to search category by title.") + return errors.New("unable to search category by title") } if category == nil { @@ -66,10 +70,10 @@ func (o *OpmlHandler) Import(userID int64, data io.Reader) (err error) { Title: subscription.CategoryName, } - err := o.store.CreateCategory(category) + err := h.store.CreateCategory(category) if err != nil { log.Println(err) - return fmt.Errorf(`Unable to create this category: "%s".`, subscription.CategoryName) + return fmt.Errorf(`unable to create this category: "%s"`, subscription.CategoryName) } } } @@ -82,13 +86,14 @@ func (o *OpmlHandler) Import(userID int64, data io.Reader) (err error) { Category: category, } - o.store.CreateFeed(feed) + h.store.CreateFeed(feed) } } return nil } -func NewOpmlHandler(store *storage.Storage) *OpmlHandler { - return &OpmlHandler{store: store} +// NewHandler creates a new handler for OPML files. +func NewHandler(store *storage.Storage) *Handler { + return &Handler{store: store} } diff --git a/reader/opml/opml.go b/reader/opml/opml.go index d5278a75..ab818125 100644 --- a/reader/opml/opml.go +++ b/reader/opml/opml.go @@ -6,21 +6,21 @@ package opml import "encoding/xml" -type Opml struct { +type opml struct { XMLName xml.Name `xml:"opml"` Version string `xml:"version,attr"` - Outlines []Outline `xml:"body>outline"` + Outlines []outline `xml:"body>outline"` } -type Outline struct { +type outline 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 []Outline `xml:"outline,omitempty"` + Outlines []outline `xml:"outline,omitempty"` } -func (o *Outline) GetTitle() string { +func (o *outline) GetTitle() string { if o.Title != "" { return o.Title } @@ -40,7 +40,7 @@ func (o *Outline) GetTitle() string { return "" } -func (o *Outline) GetSiteURL() string { +func (o *outline) GetSiteURL() string { if o.SiteURL != "" { return o.SiteURL } @@ -48,11 +48,11 @@ func (o *Outline) GetSiteURL() string { return o.FeedURL } -func (o *Outline) IsCategory() bool { +func (o *outline) IsCategory() bool { return o.Text != "" && o.SiteURL == "" && o.FeedURL == "" } -func (o *Outline) Append(subscriptions SubcriptionList, category string) SubcriptionList { +func (o *outline) Append(subscriptions SubcriptionList, category string) SubcriptionList { if o.FeedURL != "" { subscriptions = append(subscriptions, &Subcription{ Title: o.GetTitle(), @@ -65,7 +65,7 @@ func (o *Outline) Append(subscriptions SubcriptionList, category string) Subcrip return subscriptions } -func (o *Opml) Transform() SubcriptionList { +func (o *opml) Transform() SubcriptionList { var subscriptions SubcriptionList for _, outline := range o.Outlines { diff --git a/reader/opml/parser.go b/reader/opml/parser.go index 02a6dbae..1b2e1e7f 100644 --- a/reader/opml/parser.go +++ b/reader/opml/parser.go @@ -14,14 +14,14 @@ import ( // Parse reads an OPML file and returns a SubcriptionList. func Parse(data io.Reader) (SubcriptionList, error) { - opml := new(Opml) + feeds := new(opml) decoder := xml.NewDecoder(data) decoder.CharsetReader = charset.NewReaderLabel - err := decoder.Decode(opml) + err := decoder.Decode(feeds) if err != nil { return nil, errors.NewLocalizedError("Unable to parse OPML file: %v.", err) } - return opml.Transform(), nil + return feeds.Transform(), nil } diff --git a/reader/opml/serializer.go b/reader/opml/serializer.go index 3ca859af..5ba494e4 100644 --- a/reader/opml/serializer.go +++ b/reader/opml/serializer.go @@ -17,13 +17,13 @@ func Serialize(subscriptions SubcriptionList) string { writer := bufio.NewWriter(&b) writer.WriteString(xml.Header) - opml := new(Opml) - opml.Version = "2.0" + feeds := new(opml) + feeds.Version = "2.0" for categoryName, subs := range groupSubscriptionsByFeed(subscriptions) { - outline := Outline{Text: categoryName} + category := outline{Text: categoryName} for _, subscription := range subs { - outline.Outlines = append(outline.Outlines, Outline{ + category.Outlines = append(category.Outlines, outline{ Title: subscription.Title, Text: subscription.Title, FeedURL: subscription.FeedURL, @@ -31,12 +31,12 @@ func Serialize(subscriptions SubcriptionList) string { }) } - opml.Outlines = append(opml.Outlines, outline) + feeds.Outlines = append(feeds.Outlines, category) } encoder := xml.NewEncoder(writer) encoder.Indent(" ", " ") - if err := encoder.Encode(opml); err != nil { + if err := encoder.Encode(feeds); err != nil { log.Println(err) return "" } diff --git a/reader/opml/subscription.go b/reader/opml/subscription.go index b968bb08..45f2597d 100644 --- a/reader/opml/subscription.go +++ b/reader/opml/subscription.go @@ -4,6 +4,7 @@ package opml +// Subcription represents a feed that will be imported or exported. type Subcription struct { Title string SiteURL string @@ -11,8 +12,11 @@ type Subcription struct { CategoryName string } +// Equals compare two subscriptions. func (s Subcription) Equals(subscription *Subcription) bool { - return s.Title == subscription.Title && s.SiteURL == subscription.SiteURL && s.FeedURL == subscription.FeedURL && s.CategoryName == subscription.CategoryName + return s.Title == subscription.Title && s.SiteURL == subscription.SiteURL && + s.FeedURL == subscription.FeedURL && s.CategoryName == subscription.CategoryName } +// SubcriptionList is a list of subscriptions. type SubcriptionList []*Subcription diff --git a/server/routes.go b/server/routes.go index 0c5ec65f..4f6b1d48 100644 --- a/server/routes.go +++ b/server/routes.go @@ -5,6 +5,8 @@ package server import ( + "net/http" + "github.com/miniflux/miniflux2/locale" "github.com/miniflux/miniflux2/reader/feed" "github.com/miniflux/miniflux2/reader/opml" @@ -14,7 +16,6 @@ import ( "github.com/miniflux/miniflux2/server/template" ui_controller "github.com/miniflux/miniflux2/server/ui/controller" "github.com/miniflux/miniflux2/storage" - "net/http" "github.com/gorilla/mux" ) @@ -25,7 +26,7 @@ func getRoutes(store *storage.Storage, feedHandler *feed.Handler) *mux.Router { templateEngine := template.NewTemplateEngine(router, translator) apiController := api_controller.NewController(store, feedHandler) - uiController := ui_controller.NewController(store, feedHandler, opml.NewOpmlHandler(store)) + uiController := ui_controller.NewController(store, feedHandler, opml.NewHandler(store)) apiHandler := core.NewHandler(store, router, templateEngine, translator, middleware.NewMiddlewareChain( middleware.NewBasicAuthMiddleware(store).Handler, diff --git a/server/ui/controller/controller.go b/server/ui/controller/controller.go index aad32582..535fbba4 100644 --- a/server/ui/controller/controller.go +++ b/server/ui/controller/controller.go @@ -25,7 +25,7 @@ func (t tplParams) Merge(d tplParams) tplParams { type Controller struct { store *storage.Storage feedHandler *feed.Handler - opmlHandler *opml.OpmlHandler + opmlHandler *opml.Handler } func (c *Controller) getCommonTemplateArgs(ctx *core.Context) (tplParams, error) { @@ -47,7 +47,7 @@ func (c *Controller) getCommonTemplateArgs(ctx *core.Context) (tplParams, error) return params, nil } -func NewController(store *storage.Storage, feedHandler *feed.Handler, opmlHandler *opml.OpmlHandler) *Controller { +func NewController(store *storage.Storage, feedHandler *feed.Handler, opmlHandler *opml.Handler) *Controller { return &Controller{ store: store, feedHandler: feedHandler,