From 2790064ad530238c56809d4da63edd1174127bae Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Thu, 6 Nov 2025 04:25:54 +0100 Subject: [PATCH] refactor: Standardize method names and introduce context propagation Removes the `Get` prefix from exporter methods (e.g., GetSupportedFormat -> SupportedFormat) to better align with Go conventions for simple accessors. Introduces `context.Context` propagation through the application, starting from `ProcessCourseFromURI` down to the HTTP request in the parser. This makes network operations cancellable and allows for setting deadlines, improving application robustness. Additionally, optimizes the HTML cleaner by pre-compiling regular expressions for a minor performance gain. --- .gitignore | 2 ++ internal/exporters/docx.go | 4 +-- internal/exporters/docx_test.go | 6 ++--- internal/exporters/factory.go | 4 +-- internal/exporters/factory_test.go | 38 ++++++++++++++--------------- internal/exporters/html.go | 4 +-- internal/exporters/html_test.go | 6 ++--- internal/exporters/markdown.go | 4 +-- internal/exporters/markdown_test.go | 6 ++--- internal/interfaces/exporter.go | 8 +++--- internal/interfaces/parser.go | 9 +++++-- internal/services/app.go | 11 +++++---- internal/services/app_test.go | 30 +++++++++++------------ internal/services/html_cleaner.go | 12 ++++++--- internal/services/parser.go | 11 +++++++-- main.go | 4 +-- 16 files changed, 90 insertions(+), 69 deletions(-) diff --git a/.gitignore b/.gitignore index 7466600..1aadfdb 100644 --- a/.gitignore +++ b/.gitignore @@ -73,3 +73,5 @@ main_coverage .task/ **/*.local.* + +.claude/ diff --git a/internal/exporters/docx.go b/internal/exporters/docx.go index a80a5ab..3e5285f 100644 --- a/internal/exporters/docx.go +++ b/internal/exporters/docx.go @@ -191,10 +191,10 @@ func (e *DocxExporter) exportSubItem(doc *docx.Docx, subItem *models.SubItem) { } } -// GetSupportedFormat returns the format name this exporter supports. +// SupportedFormat returns the format name this exporter supports. // // Returns: // - A string representing the supported format ("docx") -func (e *DocxExporter) GetSupportedFormat() string { +func (e *DocxExporter) SupportedFormat() string { return "docx" } diff --git a/internal/exporters/docx_test.go b/internal/exporters/docx_test.go index 46dd5ed..8c747da 100644 --- a/internal/exporters/docx_test.go +++ b/internal/exporters/docx_test.go @@ -30,13 +30,13 @@ func TestNewDocxExporter(t *testing.T) { } } -// TestDocxExporter_GetSupportedFormat tests the GetSupportedFormat method. -func TestDocxExporter_GetSupportedFormat(t *testing.T) { +// TestDocxExporter_SupportedFormat tests the SupportedFormat method. +func TestDocxExporter_SupportedFormat(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() exporter := NewDocxExporter(htmlCleaner) expected := "docx" - result := exporter.GetSupportedFormat() + result := exporter.SupportedFormat() if result != expected { t.Errorf("Expected format '%s', got '%s'", expected, result) diff --git a/internal/exporters/factory.go b/internal/exporters/factory.go index a05a8cb..c68ed79 100644 --- a/internal/exporters/factory.go +++ b/internal/exporters/factory.go @@ -55,11 +55,11 @@ func (f *Factory) CreateExporter(format string) (interfaces.Exporter, error) { } } -// GetSupportedFormats returns a list of all supported export formats. +// SupportedFormats returns a list of all supported export formats. // This includes both primary format names and their aliases. // // Returns: // - A string slice containing all supported format names -func (f *Factory) GetSupportedFormats() []string { +func (f *Factory) SupportedFormats() []string { return []string{"markdown", "md", "docx", "word", "html", "htm"} } diff --git a/internal/exporters/factory_test.go b/internal/exporters/factory_test.go index 464564b..34bec2b 100644 --- a/internal/exporters/factory_test.go +++ b/internal/exporters/factory_test.go @@ -125,7 +125,7 @@ func TestFactory_CreateExporter(t *testing.T) { } // Check supported format - supportedFormat := exporter.GetSupportedFormat() + supportedFormat := exporter.SupportedFormat() if supportedFormat != tc.expectedFormat { t.Errorf("Expected supported format '%s' for format '%s', got '%s'", tc.expectedFormat, tc.format, supportedFormat) } @@ -173,7 +173,7 @@ func TestFactory_CreateExporter_CaseInsensitive(t *testing.T) { t.Fatalf("CreateExporter returned nil for format '%s'", tc.format) } - supportedFormat := exporter.GetSupportedFormat() + supportedFormat := exporter.SupportedFormat() if supportedFormat != tc.expectedFormat { t.Errorf("Expected supported format '%s' for format '%s', got '%s'", tc.expectedFormat, tc.format, supportedFormat) } @@ -221,15 +221,15 @@ func TestFactory_CreateExporter_ErrorMessages(t *testing.T) { } } -// TestFactory_GetSupportedFormats tests the GetSupportedFormats method. -func TestFactory_GetSupportedFormats(t *testing.T) { +// TestFactory_SupportedFormats tests the SupportedFormats method. +func TestFactory_SupportedFormats(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() factory := NewFactory(htmlCleaner) - formats := factory.GetSupportedFormats() + formats := factory.SupportedFormats() if formats == nil { - t.Fatal("GetSupportedFormats() returned nil") + t.Fatal("SupportedFormats() returned nil") } expected := []string{"markdown", "md", "docx", "word", "html", "htm"} @@ -246,22 +246,22 @@ func TestFactory_GetSupportedFormats(t *testing.T) { for _, format := range formats { exporter, err := factory.CreateExporter(format) if err != nil { - t.Errorf("Format '%s' from GetSupportedFormats() should be creatable, got error: %v", format, err) + t.Errorf("Format '%s' from SupportedFormats() should be creatable, got error: %v", format, err) } if exporter == nil { - t.Errorf("Format '%s' from GetSupportedFormats() should create non-nil exporter", format) + t.Errorf("Format '%s' from SupportedFormats() should create non-nil exporter", format) } } } -// TestFactory_GetSupportedFormats_Immutable tests that the returned slice is safe to modify. -func TestFactory_GetSupportedFormats_Immutable(t *testing.T) { +// TestFactory_SupportedFormats_Immutable tests that the returned slice is safe to modify. +func TestFactory_SupportedFormats_Immutable(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() factory := NewFactory(htmlCleaner) // Get formats twice - formats1 := factory.GetSupportedFormats() - formats2 := factory.GetSupportedFormats() + formats1 := factory.SupportedFormats() + formats2 := factory.SupportedFormats() // Modify first slice if len(formats1) > 0 { @@ -270,13 +270,13 @@ func TestFactory_GetSupportedFormats_Immutable(t *testing.T) { // Check that second call returns unmodified data if len(formats2) > 0 && formats2[0] == "modified" { - t.Error("GetSupportedFormats() should return independent slices") + t.Error("SupportedFormats() should return independent slices") } // Verify original functionality still works - formats3 := factory.GetSupportedFormats() + formats3 := factory.SupportedFormats() if len(formats3) == 0 { - t.Error("GetSupportedFormats() should still return formats after modification") + t.Error("SupportedFormats() should still return formats after modification") } } @@ -436,7 +436,7 @@ func TestFactory_FormatNormalization(t *testing.T) { t.Fatalf("Failed to create exporter for '%s': %v", tc.input, err) } - format := exporter.GetSupportedFormat() + format := exporter.SupportedFormat() if format != tc.expected { t.Errorf("Expected format '%s' for input '%s', got '%s'", tc.expected, tc.input, format) } @@ -464,12 +464,12 @@ func BenchmarkFactory_CreateExporter_Docx(b *testing.B) { } } -// BenchmarkFactory_GetSupportedFormats benchmarks the GetSupportedFormats method. -func BenchmarkFactory_GetSupportedFormats(b *testing.B) { +// BenchmarkFactory_SupportedFormats benchmarks the SupportedFormats method. +func BenchmarkFactory_SupportedFormats(b *testing.B) { htmlCleaner := services.NewHTMLCleaner() factory := NewFactory(htmlCleaner) for b.Loop() { - _ = factory.GetSupportedFormats() + _ = factory.SupportedFormats() } } diff --git a/internal/exporters/html.go b/internal/exporters/html.go index b04e7d3..63cb628 100644 --- a/internal/exporters/html.go +++ b/internal/exporters/html.go @@ -113,12 +113,12 @@ func (e *HTMLExporter) Export(course *models.Course, outputPath string) error { return os.WriteFile(outputPath, buf.Bytes(), 0644) } -// GetSupportedFormat returns the format name this exporter supports +// SupportedFormat returns the format name this exporter supports // It indicates the file format that the HTMLExporter can generate. // // Returns: // - A string representing the supported format ("html") -func (e *HTMLExporter) GetSupportedFormat() string { +func (e *HTMLExporter) SupportedFormat() string { return "html" } diff --git a/internal/exporters/html_test.go b/internal/exporters/html_test.go index af4f547..d3de0de 100644 --- a/internal/exporters/html_test.go +++ b/internal/exporters/html_test.go @@ -32,13 +32,13 @@ func TestNewHTMLExporter(t *testing.T) { } } -// TestHTMLExporter_GetSupportedFormat tests the GetSupportedFormat method. -func TestHTMLExporter_GetSupportedFormat(t *testing.T) { +// TestHTMLExporter_SupportedFormat tests the SupportedFormat method. +func TestHTMLExporter_SupportedFormat(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() exporter := NewHTMLExporter(htmlCleaner) expected := "html" - result := exporter.GetSupportedFormat() + result := exporter.SupportedFormat() if result != expected { t.Errorf("Expected format '%s', got '%s'", expected, result) diff --git a/internal/exporters/markdown.go b/internal/exporters/markdown.go index 70c9dd0..fc40501 100644 --- a/internal/exporters/markdown.go +++ b/internal/exporters/markdown.go @@ -92,12 +92,12 @@ func (e *MarkdownExporter) Export(course *models.Course, outputPath string) erro return os.WriteFile(outputPath, buf.Bytes(), 0644) } -// GetSupportedFormat returns the format name this exporter supports +// SupportedFormat returns the format name this exporter supports // It indicates the file format that the MarkdownExporter can generate. // // Returns: // - A string representing the supported format ("markdown") -func (e *MarkdownExporter) GetSupportedFormat() string { +func (e *MarkdownExporter) SupportedFormat() string { return "markdown" } diff --git a/internal/exporters/markdown_test.go b/internal/exporters/markdown_test.go index 1273c3e..55ff485 100644 --- a/internal/exporters/markdown_test.go +++ b/internal/exporters/markdown_test.go @@ -32,13 +32,13 @@ func TestNewMarkdownExporter(t *testing.T) { } } -// TestMarkdownExporter_GetSupportedFormat tests the GetSupportedFormat method. -func TestMarkdownExporter_GetSupportedFormat(t *testing.T) { +// TestMarkdownExporter_SupportedFormat tests the SupportedFormat method. +func TestMarkdownExporter_SupportedFormat(t *testing.T) { htmlCleaner := services.NewHTMLCleaner() exporter := NewMarkdownExporter(htmlCleaner) expected := "markdown" - result := exporter.GetSupportedFormat() + result := exporter.SupportedFormat() if result != expected { t.Errorf("Expected format '%s', got '%s'", expected, result) diff --git a/internal/interfaces/exporter.go b/internal/interfaces/exporter.go index cd3eba7..671e48b 100644 --- a/internal/interfaces/exporter.go +++ b/internal/interfaces/exporter.go @@ -12,9 +12,9 @@ type Exporter interface { // specified output path. It returns an error if the export operation fails. Export(course *models.Course, outputPath string) error - // GetSupportedFormat returns the name of the format this exporter supports. + // SupportedFormat returns the name of the format this exporter supports. // This is used to identify which exporter to use for a given format. - GetSupportedFormat() string + SupportedFormat() string } // ExporterFactory creates exporters for different formats. @@ -25,7 +25,7 @@ type ExporterFactory interface { // It returns the appropriate exporter or an error if the format is not supported. CreateExporter(format string) (Exporter, error) - // GetSupportedFormats returns a list of all export formats supported by this factory. + // SupportedFormats returns a list of all export formats supported by this factory. // This is used to inform users of available export options. - GetSupportedFormats() []string + SupportedFormats() []string } diff --git a/internal/interfaces/parser.go b/internal/interfaces/parser.go index 815aff9..105e869 100644 --- a/internal/interfaces/parser.go +++ b/internal/interfaces/parser.go @@ -2,7 +2,11 @@ // It defines interfaces for parsing and exporting Articulate Rise courses. package interfaces -import "github.com/kjanat/articulate-parser/internal/models" +import ( + "context" + + "github.com/kjanat/articulate-parser/internal/models" +) // CourseParser defines the interface for loading course data. // It provides methods to fetch course content either from a remote URI @@ -10,8 +14,9 @@ import "github.com/kjanat/articulate-parser/internal/models" type CourseParser interface { // FetchCourse loads a course from a URI (typically an Articulate Rise share URL). // It retrieves the course data from the remote location and returns a parsed Course model. + // The context can be used for cancellation and timeout control. // Returns an error if the fetch operation fails or if the data cannot be parsed. - FetchCourse(uri string) (*models.Course, error) + FetchCourse(ctx context.Context, uri string) (*models.Course, error) // LoadCourseFromFile loads a course from a local file. // It reads and parses the course data from the specified file path. diff --git a/internal/services/app.go b/internal/services/app.go index 931e013..5e6c1de 100644 --- a/internal/services/app.go +++ b/internal/services/app.go @@ -3,6 +3,7 @@ package services import ( + "context" "fmt" "github.com/kjanat/articulate-parser/internal/interfaces" @@ -44,8 +45,8 @@ func (a *App) ProcessCourseFromFile(filePath, format, outputPath string) error { // ProcessCourseFromURI fetches a course from the provided URI and exports it to the specified format. // It takes the URI to fetch the course from, the desired export format, and the output file path. // Returns an error if fetching or exporting fails. -func (a *App) ProcessCourseFromURI(uri, format, outputPath string) error { - course, err := a.parser.FetchCourse(uri) +func (a *App) ProcessCourseFromURI(ctx context.Context, uri, format, outputPath string) error { + course, err := a.parser.FetchCourse(ctx, uri) if err != nil { return fmt.Errorf("failed to fetch course: %w", err) } @@ -69,8 +70,8 @@ func (a *App) exportCourse(course *models.Course, format, outputPath string) err return nil } -// GetSupportedFormats returns a list of all export formats supported by the application. +// SupportedFormats returns a list of all export formats supported by the application. // This information is provided by the ExporterFactory. -func (a *App) GetSupportedFormats() []string { - return a.exporterFactory.GetSupportedFormats() +func (a *App) SupportedFormats() []string { + return a.exporterFactory.SupportedFormats() } diff --git a/internal/services/app_test.go b/internal/services/app_test.go index 9e1a4a4..16ebe5b 100644 --- a/internal/services/app_test.go +++ b/internal/services/app_test.go @@ -31,8 +31,8 @@ func (m *MockCourseParser) LoadCourseFromFile(filePath string) (*models.Course, // MockExporter is a mock implementation of interfaces.Exporter for testing. type MockExporter struct { - mockExport func(course *models.Course, outputPath string) error - mockGetSupportedFormat func() string + mockExport func(course *models.Course, outputPath string) error + mockSupportedFormat func() string } func (m *MockExporter) Export(course *models.Course, outputPath string) error { @@ -42,17 +42,17 @@ func (m *MockExporter) Export(course *models.Course, outputPath string) error { return nil } -func (m *MockExporter) GetSupportedFormat() string { - if m.mockGetSupportedFormat != nil { - return m.mockGetSupportedFormat() +func (m *MockExporter) SupportedFormat() string { + if m.mockSupportedFormat != nil { + return m.mockSupportedFormat() } return "mock" } // MockExporterFactory is a mock implementation of interfaces.ExporterFactory for testing. type MockExporterFactory struct { - mockCreateExporter func(format string) (*MockExporter, error) - mockGetSupportedFormats func() []string + mockCreateExporter func(format string) (*MockExporter, error) + mockSupportedFormats func() []string } func (m *MockExporterFactory) CreateExporter(format string) (interfaces.Exporter, error) { @@ -63,9 +63,9 @@ func (m *MockExporterFactory) CreateExporter(format string) (interfaces.Exporter return &MockExporter{}, nil } -func (m *MockExporterFactory) GetSupportedFormats() []string { - if m.mockGetSupportedFormats != nil { - return m.mockGetSupportedFormats() +func (m *MockExporterFactory) SupportedFormats() []string { + if m.mockSupportedFormats != nil { + return m.mockSupportedFormats() } return []string{"mock"} } @@ -119,7 +119,7 @@ func TestNewApp(t *testing.T) { } // Test that the factory is set (we can't directly compare interface values) - formats := app.GetSupportedFormats() + formats := app.SupportedFormats() if len(formats) == 0 { t.Error("App exporterFactory was not set correctly - no supported formats") } @@ -306,19 +306,19 @@ func TestApp_ProcessCourseFromURI(t *testing.T) { } } -// TestApp_GetSupportedFormats tests the GetSupportedFormats method. -func TestApp_GetSupportedFormats(t *testing.T) { +// TestApp_SupportedFormats tests the SupportedFormats method. +func TestApp_SupportedFormats(t *testing.T) { expectedFormats := []string{"markdown", "docx", "pdf"} parser := &MockCourseParser{} factory := &MockExporterFactory{ - mockGetSupportedFormats: func() []string { + mockSupportedFormats: func() []string { return expectedFormats }, } app := NewApp(parser, factory) - formats := app.GetSupportedFormats() + formats := app.SupportedFormats() if len(formats) != len(expectedFormats) { t.Errorf("Expected %d formats, got %d", len(expectedFormats), len(formats)) diff --git a/internal/services/html_cleaner.go b/internal/services/html_cleaner.go index 3fd0c90..32942fb 100644 --- a/internal/services/html_cleaner.go +++ b/internal/services/html_cleaner.go @@ -7,6 +7,13 @@ import ( "strings" ) +var ( + // htmlTagRegex matches HTML tags for removal + htmlTagRegex = regexp.MustCompile(`<[^>]*>`) + // whitespaceRegex matches multiple whitespace characters for normalization + whitespaceRegex = regexp.MustCompile(`\s+`) +) + // HTMLCleaner provides utilities for converting HTML content to plain text. // It removes HTML tags while preserving their content and converts HTML entities // to their plain text equivalents. @@ -30,8 +37,7 @@ func NewHTMLCleaner() *HTMLCleaner { // - A plain text string with all HTML elements and entities removed/converted func (h *HTMLCleaner) CleanHTML(html string) string { // Remove HTML tags but preserve content - re := regexp.MustCompile(`<[^>]*>`) - cleaned := re.ReplaceAllString(html, "") + cleaned := htmlTagRegex.ReplaceAllString(html, "") // Replace common HTML entities with their character equivalents cleaned = strings.ReplaceAll(cleaned, " ", " ") @@ -46,7 +52,7 @@ func (h *HTMLCleaner) CleanHTML(html string) string { // Clean up extra whitespace by replacing multiple spaces, tabs, and newlines // with a single space, then trim any leading/trailing whitespace - cleaned = regexp.MustCompile(`\s+`).ReplaceAllString(cleaned, " ") + cleaned = whitespaceRegex.ReplaceAllString(cleaned, " ") cleaned = strings.TrimSpace(cleaned) return cleaned diff --git a/internal/services/parser.go b/internal/services/parser.go index 0823772..b6bd8e8 100644 --- a/internal/services/parser.go +++ b/internal/services/parser.go @@ -3,6 +3,7 @@ package services import ( + "context" "encoding/json" "fmt" "io" @@ -42,13 +43,14 @@ func NewArticulateParser() interfaces.CourseParser { // The course data is then unmarshalled into a Course model. // // Parameters: +// - ctx: Context for cancellation and timeout control // - uri: The Articulate Rise share URL (e.g., https://rise.articulate.com/share/SHARE_ID) // // Returns: // - A parsed Course model if successful // - An error if the fetch fails, if the share ID can't be extracted, // or if the response can't be parsed -func (p *ArticulateParser) FetchCourse(uri string) (*models.Course, error) { +func (p *ArticulateParser) FetchCourse(ctx context.Context, uri string) (*models.Course, error) { shareID, err := p.extractShareID(uri) if err != nil { return nil, err @@ -56,7 +58,12 @@ func (p *ArticulateParser) FetchCourse(uri string) (*models.Course, error) { apiURL := p.buildAPIURL(shareID) - resp, err := p.Client.Get(apiURL) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + resp, err := p.Client.Do(req) if err != nil { return nil, fmt.Errorf("failed to fetch course data: %w", err) } diff --git a/main.go b/main.go index 18e683b..9741c21 100644 --- a/main.go +++ b/main.go @@ -40,13 +40,13 @@ func run(args []string) int { // Check for help flag if len(args) > 1 && (args[1] == "--help" || args[1] == "-h" || args[1] == "help") { - printUsage(args[0], app.GetSupportedFormats()) + printUsage(args[0], app.SupportedFormats()) return 0 } // Check for required command-line arguments if len(args) < 4 { - printUsage(args[0], app.GetSupportedFormats()) + printUsage(args[0], app.SupportedFormats()) return 1 }