From defce7d2d3a702b3039a1c802711ac0d3d1ac10f Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Wed, 7 Jan 2026 15:42:00 +0100 Subject: [PATCH] refactor: extract base exporter, add sentinel errors, document unused fields --- internal/apperrors/errors.go | 28 ++++++++++++++++ internal/apperrors/errors_test.go | 50 +++++++++++++++++++++++++++++ internal/exporters/base.go | 20 ++++++++++++ internal/exporters/docx.go | 8 ++--- internal/exporters/factory.go | 3 +- internal/exporters/html.go | 12 +++---- internal/exporters/markdown.go | 8 ++--- internal/exporters/markdown_test.go | 26 +++++++-------- internal/models/course.go | 2 ++ internal/models/lesson.go | 9 ++++-- internal/services/app.go | 5 +-- internal/services/parser.go | 7 ++-- 12 files changed, 136 insertions(+), 42 deletions(-) create mode 100644 internal/apperrors/errors.go create mode 100644 internal/apperrors/errors_test.go create mode 100644 internal/exporters/base.go diff --git a/internal/apperrors/errors.go b/internal/apperrors/errors.go new file mode 100644 index 0000000..ff01da3 --- /dev/null +++ b/internal/apperrors/errors.go @@ -0,0 +1,28 @@ +// Package apperrors provides sentinel errors for the articulate-parser application. +package apperrors + +import "errors" + +// Export errors. +var ( + // ErrUnsupportedFormat is returned when an unknown export format is requested. + ErrUnsupportedFormat = errors.New("unsupported export format") +) + +// URI parsing errors. +var ( + // ErrInvalidURI is returned when a URI cannot be parsed. + ErrInvalidURI = errors.New("invalid URI") + + // ErrInvalidDomain is returned when the URI domain is not an Articulate Rise domain. + ErrInvalidDomain = errors.New("invalid domain for Articulate Rise URI") + + // ErrNoShareID is returned when a share ID cannot be extracted from the URI. + ErrNoShareID = errors.New("could not extract share ID from URI") +) + +// Operation errors. +var ( + // ErrOperationCanceled is returned when an operation is canceled via context. + ErrOperationCanceled = errors.New("operation canceled") +) diff --git a/internal/apperrors/errors_test.go b/internal/apperrors/errors_test.go new file mode 100644 index 0000000..846053f --- /dev/null +++ b/internal/apperrors/errors_test.go @@ -0,0 +1,50 @@ +package apperrors + +import ( + "errors" + "fmt" + "testing" +) + +func TestSentinelErrors_Unwrap(t *testing.T) { + tests := []struct { + name string + sentinel error + wrapped error + }{ + {"ErrUnsupportedFormat", ErrUnsupportedFormat, fmt.Errorf("%w: xyz", ErrUnsupportedFormat)}, + {"ErrInvalidURI", ErrInvalidURI, fmt.Errorf("%w: bad", ErrInvalidURI)}, + {"ErrInvalidDomain", ErrInvalidDomain, fmt.Errorf("%w: evil.com", ErrInvalidDomain)}, + {"ErrNoShareID", ErrNoShareID, fmt.Errorf("%w: no match", ErrNoShareID)}, + {"ErrOperationCanceled", ErrOperationCanceled, fmt.Errorf("%w: ctx done", ErrOperationCanceled)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if !errors.Is(tt.wrapped, tt.sentinel) { + t.Errorf("errors.Is(%v, %v) = false, want true", tt.wrapped, tt.sentinel) + } + }) + } +} + +func TestSentinelErrors_Message(t *testing.T) { + tests := []struct { + err error + expected string + }{ + {ErrUnsupportedFormat, "unsupported export format"}, + {ErrInvalidURI, "invalid URI"}, + {ErrInvalidDomain, "invalid domain for Articulate Rise URI"}, + {ErrNoShareID, "could not extract share ID from URI"}, + {ErrOperationCanceled, "operation canceled"}, + } + + for _, tt := range tests { + t.Run(tt.expected, func(t *testing.T) { + if tt.err.Error() != tt.expected { + t.Errorf("got %q, want %q", tt.err.Error(), tt.expected) + } + }) + } +} diff --git a/internal/exporters/base.go b/internal/exporters/base.go new file mode 100644 index 0000000..b5c57a7 --- /dev/null +++ b/internal/exporters/base.go @@ -0,0 +1,20 @@ +package exporters + +import "github.com/kjanat/articulate-parser/internal/interfaces" + +// baseExporter contains common fields shared by all exporter implementations. +// Embed this struct in concrete exporters to inherit HTMLCleaner and Logger. +type baseExporter struct { + // htmlCleaner is used to convert HTML content to plain text. + htmlCleaner interfaces.HTMLCleaner + // logger is used for logging warnings and errors. + logger interfaces.Logger +} + +// newBaseExporter creates a new baseExporter with the given dependencies. +func newBaseExporter(htmlCleaner interfaces.HTMLCleaner, logger interfaces.Logger) baseExporter { + return baseExporter{ + htmlCleaner: htmlCleaner, + logger: logger, + } +} diff --git a/internal/exporters/docx.go b/internal/exporters/docx.go index cd543e6..d9c8b6f 100644 --- a/internal/exporters/docx.go +++ b/internal/exporters/docx.go @@ -26,10 +26,7 @@ const ( // It converts Articulate Rise course data into a Microsoft Word document // using the go-docx package. type DocxExporter struct { - // htmlCleaner is used to convert HTML content to plain text - htmlCleaner interfaces.HTMLCleaner - // logger is used for logging warnings and errors - logger interfaces.Logger + baseExporter } // NewDocxExporter creates a new DocxExporter instance. @@ -43,8 +40,7 @@ type DocxExporter struct { // - An implementation of the Exporter interface for DOCX format func NewDocxExporter(htmlCleaner interfaces.HTMLCleaner, logger interfaces.Logger) interfaces.Exporter { return &DocxExporter{ - htmlCleaner: htmlCleaner, - logger: logger, + baseExporter: newBaseExporter(htmlCleaner, logger), } } diff --git a/internal/exporters/factory.go b/internal/exporters/factory.go index 5e4361c..eae6cb6 100644 --- a/internal/exporters/factory.go +++ b/internal/exporters/factory.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/kjanat/articulate-parser/internal/apperrors" "github.com/kjanat/articulate-parser/internal/interfaces" ) @@ -56,7 +57,7 @@ func (f *Factory) CreateExporter(format string) (interfaces.Exporter, error) { case FormatHTML, formatAliasHTML: return NewHTMLExporter(f.htmlCleaner, f.logger), nil default: - return nil, fmt.Errorf("unsupported export format: %s", format) + return nil, fmt.Errorf("%w: %s", apperrors.ErrUnsupportedFormat, format) } } diff --git a/internal/exporters/html.go b/internal/exporters/html.go index 92a1060..db28b2c 100644 --- a/internal/exporters/html.go +++ b/internal/exporters/html.go @@ -20,12 +20,9 @@ var htmlTemplate string // HTMLExporter implements the Exporter interface for HTML format. // It converts Articulate Rise course data into a structured HTML document using templates. type HTMLExporter struct { - // htmlCleaner is used to convert HTML content to plain text when needed - htmlCleaner interfaces.HTMLCleaner - // tmpl holds the parsed HTML template + baseExporter + // tmpl holds the parsed HTML template. tmpl *template.Template - // logger is used for logging warnings and errors - logger interfaces.Logger } // NewHTMLExporter creates a new HTMLExporter instance. @@ -52,9 +49,8 @@ func NewHTMLExporter(htmlCleaner interfaces.HTMLCleaner, logger interfaces.Logge tmpl := template.Must(template.New("html").Funcs(funcMap).Parse(htmlTemplate)) return &HTMLExporter{ - htmlCleaner: htmlCleaner, - tmpl: tmpl, - logger: logger, + baseExporter: newBaseExporter(htmlCleaner, logger), + tmpl: tmpl, } } diff --git a/internal/exporters/markdown.go b/internal/exporters/markdown.go index 29a2509..5a73854 100644 --- a/internal/exporters/markdown.go +++ b/internal/exporters/markdown.go @@ -16,10 +16,7 @@ import ( // MarkdownExporter implements the Exporter interface for Markdown format. // It converts Articulate Rise course data into a structured Markdown document. type MarkdownExporter struct { - // htmlCleaner is used to convert HTML content to plain text - htmlCleaner interfaces.HTMLCleaner - // logger is used for logging warnings and errors - logger interfaces.Logger + baseExporter } // NewMarkdownExporter creates a new MarkdownExporter instance. @@ -33,8 +30,7 @@ type MarkdownExporter struct { // - An implementation of the Exporter interface for Markdown format func NewMarkdownExporter(htmlCleaner interfaces.HTMLCleaner, logger interfaces.Logger) interfaces.Exporter { return &MarkdownExporter{ - htmlCleaner: htmlCleaner, - logger: logger, + baseExporter: newBaseExporter(htmlCleaner, logger), } } diff --git a/internal/exporters/markdown_test.go b/internal/exporters/markdown_test.go index 845a790..45219e1 100644 --- a/internal/exporters/markdown_test.go +++ b/internal/exporters/markdown_test.go @@ -124,7 +124,7 @@ func TestMarkdownExporter_Export_InvalidPath(t *testing.T) { // TestMarkdownExporter_ProcessTextItem tests the processTextItem method. func TestMarkdownExporter_ProcessTextItem(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} var buf bytes.Buffer item := models.Item{ @@ -153,7 +153,7 @@ func TestMarkdownExporter_ProcessTextItem(t *testing.T) { // TestMarkdownExporter_ProcessListItem tests the processListItem method. func TestMarkdownExporter_ProcessListItem(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} var buf bytes.Buffer item := models.Item{ @@ -178,7 +178,7 @@ func TestMarkdownExporter_ProcessListItem(t *testing.T) { // TestMarkdownExporter_ProcessMultimediaItem tests the processMultimediaItem method. func TestMarkdownExporter_ProcessMultimediaItem(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} var buf bytes.Buffer item := models.Item{ @@ -217,7 +217,7 @@ func TestMarkdownExporter_ProcessMultimediaItem(t *testing.T) { // TestMarkdownExporter_ProcessImageItem tests the processImageItem method. func TestMarkdownExporter_ProcessImageItem(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} var buf bytes.Buffer item := models.Item{ @@ -252,7 +252,7 @@ func TestMarkdownExporter_ProcessImageItem(t *testing.T) { // TestMarkdownExporter_ProcessKnowledgeCheckItem tests the processKnowledgeCheckItem method. func TestMarkdownExporter_ProcessKnowledgeCheckItem(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} var buf bytes.Buffer item := models.Item{ @@ -294,7 +294,7 @@ func TestMarkdownExporter_ProcessKnowledgeCheckItem(t *testing.T) { // TestMarkdownExporter_ProcessInteractiveItem tests the processInteractiveItem method. func TestMarkdownExporter_ProcessInteractiveItem(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} var buf bytes.Buffer item := models.Item{ @@ -319,7 +319,7 @@ func TestMarkdownExporter_ProcessInteractiveItem(t *testing.T) { // TestMarkdownExporter_ProcessDividerItem tests the processDividerItem method. func TestMarkdownExporter_ProcessDividerItem(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} var buf bytes.Buffer exporter.processDividerItem(&buf) @@ -335,7 +335,7 @@ func TestMarkdownExporter_ProcessDividerItem(t *testing.T) { // TestMarkdownExporter_ProcessUnknownItem tests the processUnknownItem method. func TestMarkdownExporter_ProcessUnknownItem(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} var buf bytes.Buffer item := models.Item{ @@ -366,7 +366,7 @@ func TestMarkdownExporter_ProcessUnknownItem(t *testing.T) { // TestMarkdownExporter_ProcessVideoMedia tests the processVideoMedia method. func TestMarkdownExporter_ProcessVideoMedia(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} var buf bytes.Buffer media := &models.Media{ @@ -391,7 +391,7 @@ func TestMarkdownExporter_ProcessVideoMedia(t *testing.T) { // TestMarkdownExporter_ProcessImageMedia tests the processImageMedia method. func TestMarkdownExporter_ProcessImageMedia(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} var buf bytes.Buffer media := &models.Media{ @@ -413,7 +413,7 @@ func TestMarkdownExporter_ProcessImageMedia(t *testing.T) { // TestMarkdownExporter_ProcessAnswers tests the processAnswers method. func TestMarkdownExporter_ProcessAnswers(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} var buf bytes.Buffer answers := []models.Answer{ @@ -443,7 +443,7 @@ func TestMarkdownExporter_ProcessAnswers(t *testing.T) { // TestMarkdownExporter_ProcessItemToMarkdown_AllTypes tests all item types. func TestMarkdownExporter_ProcessItemToMarkdown_AllTypes(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} tests := []struct { name string @@ -673,7 +673,7 @@ func BenchmarkMarkdownExporter_Export(b *testing.B) { // BenchmarkMarkdownExporter_ProcessTextItem benchmarks the processTextItem method. func BenchmarkMarkdownExporter_ProcessTextItem(b *testing.B) { htmlCleaner := services.NewHTMLCleaner() - exporter := &MarkdownExporter{htmlCleaner: htmlCleaner} + exporter := &MarkdownExporter{baseExporter: baseExporter{htmlCleaner: htmlCleaner}} item := models.Item{ Type: "text", diff --git a/internal/models/course.go b/internal/models/course.go index 152c236..4f754fa 100644 --- a/internal/models/course.go +++ b/internal/models/course.go @@ -47,6 +47,8 @@ type ExportSettings struct { // LabelSet contains customized labels used throughout the course. // This allows course creators to modify standard terminology. +// +// NOTE: Currently parsed but not used in export logic. type LabelSet struct { // ID is the unique identifier for this label set ID string `json:"id"` diff --git a/internal/models/lesson.go b/internal/models/lesson.go index 95866ee..1132dc7 100644 --- a/internal/models/lesson.go +++ b/internal/models/lesson.go @@ -15,7 +15,8 @@ type Lesson struct { Icon string `json:"icon"` // Items is an ordered array of content items within the lesson Items []Item `json:"items"` - // Position stores the ordering information for the lesson + // Position stores the ordering information for the lesson. + // NOTE: Currently parsed but not used in export logic. Position any `json:"position"` // Ready indicates whether the lesson is marked as complete Ready bool `json:"ready"` @@ -38,9 +39,11 @@ type Item struct { Variant string `json:"variant"` // Items contains the actual content elements (sub-items) of this item Items []SubItem `json:"items"` - // Settings contains configuration options specific to this item type + // Settings contains configuration options specific to this item type. + // NOTE: Currently parsed but not used in export logic. Settings any `json:"settings"` - // Data contains additional structured data for the item + // Data contains additional structured data for the item. + // NOTE: Currently parsed but not used in export logic. Data any `json:"data"` // Media contains any associated media for the item Media *Media `json:"media,omitempty"` diff --git a/internal/services/app.go b/internal/services/app.go index 06ec364..9014b8d 100644 --- a/internal/services/app.go +++ b/internal/services/app.go @@ -6,6 +6,7 @@ import ( "context" "fmt" + "github.com/kjanat/articulate-parser/internal/apperrors" "github.com/kjanat/articulate-parser/internal/interfaces" "github.com/kjanat/articulate-parser/internal/models" ) @@ -37,7 +38,7 @@ func NewApp(parser interfaces.CourseParser, exporterFactory interfaces.ExporterF func (a *App) ProcessCourseFromFile(ctx context.Context, filePath, format, outputPath string) error { // Check for cancellation before loading if err := ctx.Err(); err != nil { - return fmt.Errorf("operation canceled: %w", err) + return fmt.Errorf("%w: %w", apperrors.ErrOperationCanceled, err) } course, err := a.parser.LoadCourseFromFile(filePath) @@ -68,7 +69,7 @@ func (a *App) ProcessCourseFromURI(ctx context.Context, uri, format, outputPath func (a *App) exportCourse(ctx context.Context, course *models.Course, format, outputPath string) error { // Check for cancellation before export if err := ctx.Err(); err != nil { - return fmt.Errorf("operation canceled: %w", err) + return fmt.Errorf("%w: %w", apperrors.ErrOperationCanceled, err) } exporter, err := a.exporterFactory.CreateExporter(format) diff --git a/internal/services/parser.go b/internal/services/parser.go index b79eb0d..71ee861 100644 --- a/internal/services/parser.go +++ b/internal/services/parser.go @@ -10,6 +10,7 @@ import ( "os" "regexp" + "github.com/kjanat/articulate-parser/internal/apperrors" "github.com/kjanat/articulate-parser/internal/config" "github.com/kjanat/articulate-parser/internal/interfaces" "github.com/kjanat/articulate-parser/internal/models" @@ -127,17 +128,17 @@ func (p *ArticulateParser) extractShareID(uri string) (string, error) { // Parse the URL to validate the domain parsedURL, err := url.Parse(uri) if err != nil { - return "", fmt.Errorf("invalid URI: %s", uri) + return "", fmt.Errorf("%w: %s", apperrors.ErrInvalidURI, uri) } // Validate that it's an Articulate Rise domain if parsedURL.Host != riseHost { - return "", fmt.Errorf("invalid domain for Articulate Rise URI: %s", parsedURL.Host) + return "", fmt.Errorf("%w: %s", apperrors.ErrInvalidDomain, parsedURL.Host) } matches := shareIDRegex.FindStringSubmatch(uri) if len(matches) < 2 { - return "", fmt.Errorf("could not extract share ID from URI: %s", uri) + return "", fmt.Errorf("%w: %s", apperrors.ErrNoShareID, uri) } return matches[1], nil }