From 37b4746c814649e68f1a7b843816a346dfc6114d Mon Sep 17 00:00:00 2001 From: Luna Date: Sat, 7 Oct 2023 18:05:25 -0300 Subject: [PATCH] refactor to out parameters on createTagName before, we used a thread local variable that was global across the entire Context, and it only held the "last error". while it is thread safe, it is very inflexible. instead, use out parameters on the functions that need it, like createTagName. --- src/include_main.zig | 4 +- src/janitor_main.zig | 12 +++--- src/main.zig | 91 +++++++++++++++++++++++++++++++++++--------- src/metrics_main.zig | 16 ++++---- src/tags_main.zig | 22 +++++------ 5 files changed, 100 insertions(+), 45 deletions(-) diff --git a/src/include_main.zig b/src/include_main.zig index 2c8b22f..b7591a3 100644 --- a/src/include_main.zig +++ b/src/include_main.zig @@ -892,7 +892,7 @@ fn addTagList( if (maybe_tag) |tag| { try file.addTag(tag.core, .{}); } else { - var tag = try ctx.createNamedTag(named_tag_text, "en", null); + var tag = try ctx.createNamedTag(named_tag_text, "en", null, .{}); try file.addTag(tag.core, .{}); } } @@ -1045,7 +1045,7 @@ pub fn main() anyerror!void { logger.err("strict mode is on. '{s}' is an unknown tag", .{named_tag_text}); } else { // TODO support ISO 639-1 for language codes - var new_tag = try ctx.createNamedTag(named_tag_text, "en", null); + var new_tag = try ctx.createNamedTag(named_tag_text, "en", null, .{}); logger.debug( "(created!) tag '{s}' with core {s}", .{ named_tag_text, new_tag.core }, diff --git a/src/janitor_main.zig b/src/janitor_main.zig index 0a75980..31635dc 100644 --- a/src/janitor_main.zig +++ b/src/janitor_main.zig @@ -214,7 +214,7 @@ pub fn janitorCheckTagNameRegex( defer ctx.allocator.free(row.tag_text); logger.debug("verify tag: {s}", .{row.tag_text}); - ctx.verifyTagName(row.tag_text) catch |err| { + ctx.verifyTagName(row.tag_text, .{}) catch |err| { logger.warn("tag name '{s}' does not match regex ({s})", .{ row.tag_text, @errorName(err) }); counters.invalid_tag_name.total += 1; counters.invalid_tag_name.unrepairable += 1; @@ -541,7 +541,7 @@ test "janitor functionality" { var indexed_file = try ctx.createFileFromDir(tmp.dir, "test_file", .{}); defer indexed_file.deinit(); - var tag = try ctx.createNamedTag("test_tag", "en", null); + var tag = try ctx.createNamedTag("test_tag", "en", null, .{}); try indexed_file.addTag(tag.core, .{}); var counters: ErrorCounters = .{}; @@ -558,10 +558,10 @@ test "tag name regex retroactive checker" { var ctx = try manage_main.makeTestContext(); defer ctx.deinit(); - _ = try ctx.createNamedTag("correct_tag", "en", null); - _ = try ctx.createNamedTag("incorrect tag", "en", null); - _ = try ctx.createNamedTag("abceddef", "en", null); - _ = try ctx.createNamedTag("tag2", "en", null); + _ = try ctx.createNamedTag("correct_tag", "en", null, .{}); + _ = try ctx.createNamedTag("incorrect tag", "en", null, .{}); + _ = try ctx.createNamedTag("abceddef", "en", null, .{}); + _ = try ctx.createNamedTag("tag2", "en", null, .{}); // TODO why doesnt a constant string on the stack work on this query // TODO API for changing library config diff --git a/src/main.zig b/src/main.zig index 8640c84..32da054 100644 --- a/src/main.zig +++ b/src/main.zig @@ -959,7 +959,7 @@ pub const Context = struct { self.resetConfig(); } - pub fn verifyTagName(self: *Self, text: []const u8) !void { + pub fn verifyTagName(self: *Self, text: []const u8, options: CreateNamedTagOptions) !void { try self.wantConfigFields(.{ .tag_name_regex = true }); if (self.library_config.tag_name_regex) |regex| { const maybe_capture = try regex.matches(text, .{}); @@ -969,14 +969,14 @@ pub const Context = struct { const is_at_end = capture.end == text.len; if (!(is_at_start and is_at_end)) { - self.setLastError(.{ .tag_name_regex = .{ + options.setError(.{ .invalid_tag_name = .{ .full_regex = self.library_config.tag_name_regex_string.?, .matched_result = text[capture.start..capture.end], } }); return error.InvalidTagName; } } else { - self.setLastError(.{ .tag_name_regex = .{ + options.setError(.{ .invalid_tag_name = .{ .full_regex = self.library_config.tag_name_regex_string.?, .matched_result = null, } }); @@ -985,13 +985,68 @@ pub const Context = struct { } } + pub const CreateNamedTagError = union(enum) { + none: void, + invalid_tag_name: struct { + full_regex: []const u8, + matched_result: ?[]const u8 = null, + }, + + pub fn format( + self: @This(), + comptime f: []const u8, + options: std.fmt.FormatOptions, + writer: anytype, + ) !void { + _ = f; + _ = options; + return switch (self) { + .invalid_tag_name => |data| if (data.matched_result) |matched| + std.fmt.format( + writer, + "regex {s} does not match to given tag name, only '{?s}'", + .{ data.full_regex, matched }, + ) + else + std.fmt.format( + writer, + "regex {s} does not match to given tag name", + .{data.full_regex}, + ), + .none => {}, + }; + } + }; + + pub const CreateNamedTagOptions = struct { + error_output: ?*CreateNamedTagError = null, + + pub fn logError(self: @This()) void { + if (self.error_output) |error_output_ptr| { + switch (error_output_ptr.*) { + .none => {}, + else => logger.err("an error happened: {}", .{error_output_ptr.*}), + } + } + } + + pub fn setError(self: @This(), error_data: CreateNamedTagError) void { + if (self.error_output) |error_output_ptr| { + error_output_ptr.* = error_data; + } else { + logger.err("an error happened: {}", .{error_data}); + } + } + }; + pub fn createNamedTag( self: *Self, text: []const u8, language: []const u8, maybe_core: ?Hash, + options: CreateNamedTagOptions, ) !Tag { - try self.verifyTagName(text); + try self.verifyTagName(text, options); var core_hash: Hash = undefined; if (maybe_core) |existing_core_hash| { @@ -2187,7 +2242,7 @@ test "tag creation" { var ctx = try makeTestContext(); defer ctx.deinit(); - var tag = try ctx.createNamedTag("test_tag", "en", null); + var tag = try ctx.createNamedTag("test_tag", "en", null, .{}); var fetched_tag = (try ctx.fetchNamedTag("test_tag", "en")).?; try std.testing.expectEqualStrings("test_tag", tag.kind.Named.text); @@ -2198,7 +2253,7 @@ test "tag creation" { try std.testing.expectEqual(tag.core.id, fetched_tag.core.id); try std.testing.expectEqualStrings(tag.core.hash_data[0..], fetched_tag.core.hash_data[0..]); - var same_core_tag = try ctx.createNamedTag("another_test_tag", "en", tag.core); + var same_core_tag = try ctx.createNamedTag("another_test_tag", "en", tag.core, .{}); var fetched_same_core_tag = (try ctx.fetchNamedTag("another_test_tag", "en")).?; try std.testing.expectEqualStrings(tag.core.hash_data[0..], same_core_tag.core.hash_data[0..]); try std.testing.expectEqualStrings(fetched_tag.core.hash_data[0..], fetched_same_core_tag.core.hash_data[0..]); @@ -2279,7 +2334,7 @@ test "file and tags" { var indexed_file = try ctx.createFileFromDir(tmp.dir, "test_file", .{}); defer indexed_file.deinit(); - var tag = try ctx.createNamedTag("test_tag", "en", null); + var tag = try ctx.createNamedTag("test_tag", "en", null, .{}); // add tag try indexed_file.addTag(tag.core, .{}); @@ -2310,7 +2365,7 @@ test "in memory database" { var ctx = try makeTestContextRealFile(); defer ctx.deinit(); - var tag1 = try ctx.createNamedTag("test_tag", "en", null); + var tag1 = try ctx.createNamedTag("test_tag", "en", null, .{}); _ = tag1; try ctx.turnIntoMemoryDb(); @@ -2318,7 +2373,7 @@ test "in memory database" { var tag1_inmem = try ctx.fetchNamedTag("test_tag", "en"); try std.testing.expect(tag1_inmem != null); - var tag2 = try ctx.createNamedTag("test_tag2", "en", null); + var tag2 = try ctx.createNamedTag("test_tag2", "en", null, .{}); _ = tag2; var tag2_inmem = try ctx.fetchNamedTag("test_tag2", "en"); @@ -2343,13 +2398,13 @@ test "tag parenting" { var indexed_file = try ctx.createFileFromDir(tmp.dir, "test_file", .{}); defer indexed_file.deinit(); - var child_tag = try ctx.createNamedTag("child_test_tag", "en", null); + var child_tag = try ctx.createNamedTag("child_test_tag", "en", null, .{}); try indexed_file.addTag(child_tag.core, .{}); // only add this through inferrence - var parent_tag = try ctx.createNamedTag("parent_test_tag", "en", null); - var parent_tag2 = try ctx.createNamedTag("parent_test_tag2", "en", null); - var parent_tag3 = try ctx.createNamedTag("parent_test_tag3", "en", null); + var parent_tag = try ctx.createNamedTag("parent_test_tag", "en", null, .{}); + var parent_tag2 = try ctx.createNamedTag("parent_test_tag2", "en", null, .{}); + var parent_tag3 = try ctx.createNamedTag("parent_test_tag3", "en", null, .{}); const tag_tree_entry_id = try ctx.createTagParent(child_tag, parent_tag); const tag_tree_entry2_id = try ctx.createTagParent(child_tag, parent_tag2); const tag_tree_entry3_id = try ctx.createTagParent(parent_tag2, parent_tag3); @@ -2415,7 +2470,7 @@ test "file pools" { var indexed_file3 = try ctx.createFileFromDir(tmp.dir, "test_file3", .{}); defer indexed_file3.deinit(); - var child_tag = try ctx.createNamedTag("child_test_tag", "en", null); + var child_tag = try ctx.createNamedTag("child_test_tag", "en", null, .{}); try indexed_file1.addTag(child_tag.core, .{}); try indexed_file2.addTag(child_tag.core, .{}); try indexed_file3.addTag(child_tag.core, .{}); @@ -2530,8 +2585,8 @@ test "tag sources" { var source2 = try ctx.createTagSource("my test tag source 2", .{}); _ = source2; - var tag1 = try ctx.createNamedTag("child_test_tag", "en", null); - var tag2 = try ctx.createNamedTag("child_test_tag2", "en", null); + var tag1 = try ctx.createNamedTag("child_test_tag", "en", null, .{}); + var tag2 = try ctx.createNamedTag("child_test_tag2", "en", null, .{}); try indexed_file1.addTag(tag1.core, .{ .source = source }); try indexed_file1.addTag(tag2.core, .{ .source = null }); @@ -2557,9 +2612,9 @@ test "tag name regex" { defer std.testing.allocator.free(TEST_TAG_REGEX); try ctx.updateLibraryConfig(.{ .tag_name_regex = TEST_TAG_REGEX }); - try std.testing.expectError(error.InvalidTagName, ctx.createNamedTag("my test tag", "en", null)); + try std.testing.expectError(error.InvalidTagName, ctx.createNamedTag("my test tag", "en", null, .{})); try std.testing.expect(ctx.getLastError() != null); - _ = try ctx.createNamedTag("correct_tag_source", "en", null); + _ = try ctx.createNamedTag("correct_tag_source", "en", null, .{}); } test "everyone else" { diff --git a/src/metrics_main.zig b/src/metrics_main.zig index a37d6df..e077807 100644 --- a/src/metrics_main.zig +++ b/src/metrics_main.zig @@ -243,12 +243,12 @@ test "metrics (tags)" { var ctx = try manage_main.makeTestContext(); defer ctx.deinit(); - var tag1 = try ctx.createNamedTag("test_tag1", "en", null); - var tag2 = try ctx.createNamedTag("test_tag2", "en", null); + var tag1 = try ctx.createNamedTag("test_tag1", "en", null, .{}); + var tag2 = try ctx.createNamedTag("test_tag2", "en", null, .{}); _ = tag2; - var tag3 = try ctx.createNamedTag("test_tag3", "en", null); + var tag3 = try ctx.createNamedTag("test_tag3", "en", null, .{}); _ = tag3; - var tag_named1 = try ctx.createNamedTag("test_tag1_samecore", "en", tag1.core); + var tag_named1 = try ctx.createNamedTag("test_tag1_samecore", "en", tag1.core, .{}); _ = tag_named1; // run metrics code @@ -268,10 +268,10 @@ test "metrics (tags and files)" { // setup tags - var tag1 = try ctx.createNamedTag("test_tag1", "en", null); - var tag2 = try ctx.createNamedTag("test_tag2", "en", null); - var tag3 = try ctx.createNamedTag("test_tag3", "en", null); - var tag_named1 = try ctx.createNamedTag("test_tag1_samecore", "en", tag1.core); + var tag1 = try ctx.createNamedTag("test_tag1", "en", null, .{}); + var tag2 = try ctx.createNamedTag("test_tag2", "en", null, .{}); + var tag3 = try ctx.createNamedTag("test_tag3", "en", null, .{}); + var tag_named1 = try ctx.createNamedTag("test_tag1_samecore", "en", tag1.core, .{}); // setup files diff --git a/src/tags_main.zig b/src/tags_main.zig index 94a5262..5adfce9 100644 --- a/src/tags_main.zig +++ b/src/tags_main.zig @@ -212,13 +212,13 @@ const CreateAction = struct { logger.info("deleted {d} tag names", .{deleted_tag_names}); // and create the proper alias (can only be done after deletion) - const aliased_tag = try self.ctx.createNamedTag(self.config.tag.?, "en", tag_to_be_aliased_to); + const aliased_tag = try self.ctx.createNamedTag(self.config.tag.?, "en", tag_to_be_aliased_to, .{}); logger.info("full tag info: {}", .{aliased_tag}); return; } - const tag = try self.ctx.createNamedTag(self.config.tag.?, "en", maybe_core); + const tag = try self.ctx.createNamedTag(self.config.tag.?, "en", maybe_core, .{}); try stdout.print( "created tag with core '{s}' name '{s}'\n", @@ -248,8 +248,8 @@ test "create action (aliasing)" { var ctx = try manage_main.makeTestContext(); defer ctx.deinit(); - var tag1 = try ctx.createNamedTag("test tag1", "en", null); - var tag2_before_alias = try ctx.createNamedTag("test tag2", "en", null); + var tag1 = try ctx.createNamedTag("test tag1", "en", null, .{}); + var tag2_before_alias = try ctx.createNamedTag("test tag2", "en", null, .{}); try std.testing.expect(!std.meta.eql(tag2_before_alias.core.id, tag1.core.id)); @@ -496,10 +496,10 @@ test "remove action" { var ctx = try manage_main.makeTestContext(); defer ctx.deinit(); - var tag = try ctx.createNamedTag("test tag", "en", null); - var tag2 = try ctx.createNamedTag("test tag2", "en", tag.core); + var tag = try ctx.createNamedTag("test tag", "en", null, .{}); + var tag2 = try ctx.createNamedTag("test tag2", "en", tag.core, .{}); _ = tag2; - var tag3 = try ctx.createNamedTag("test tag3", "en", null); + var tag3 = try ctx.createNamedTag("test tag3", "en", null, .{}); var tmp = std.testing.tmpDir(.{}); defer tmp.cleanup(); @@ -983,15 +983,15 @@ fn parentTestSetup( ctx: *Context, indexed_file: *Context.File, ) !ParentTestSetupResult { - var child_tag = try ctx.createNamedTag("child_test_tag", "en", null); + var child_tag = try ctx.createNamedTag("child_test_tag", "en", null, .{}); try indexed_file.addTag(child_tag.core, .{}); // only add this through inferrence // child_tag -> parent_tag, parent_tag2 // parent_tag2 -> parent_tag3 - var parent_tag = try ctx.createNamedTag("parent_test_tag", "en", null); - var parent_tag2 = try ctx.createNamedTag("parent_test_tag2", "en", null); - var parent_tag3 = try ctx.createNamedTag("parent_test_tag3", "en", null); + var parent_tag = try ctx.createNamedTag("parent_test_tag", "en", null, .{}); + var parent_tag2 = try ctx.createNamedTag("parent_test_tag2", "en", null, .{}); + var parent_tag3 = try ctx.createNamedTag("parent_test_tag3", "en", null, .{}); const tag_tree_entry_id = try ctx.createTagParent(child_tag, parent_tag); const tag_tree_entry2_id = try ctx.createTagParent(child_tag, parent_tag2); const tag_tree_entry3_id = try ctx.createTagParent(parent_tag2, parent_tag3);