Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add middleware option for slicer module and a UTF-8 aware slicer #23

Merged
merged 11 commits into from
Oct 25, 2024

Conversation

Ivor
Copy link
Contributor

@Ivor Ivor commented Oct 24, 2024

This pull request introduces a new slicing behavior for log entries, adds UTF-8 support, and updates the middleware to allow the use custom slicers through a configuration option. The most important changes include defining new slicing modules, updating the middleware to accept a custom slicer, and adding corresponding tests.

Slicing Behavior Enhancements:

Middleware Updates:

Testing Enhancements:

Highlights

🧵 #23 (comment)
🧵 #23 (comment)

lib/tesla/middleware/meta_logger.ex Show resolved Hide resolved
@@ -1,46 +1,9 @@
defmodule MetaLogger.Slicer do
@moduledoc """
Responsible for slicing log entries according to the given max length option.
A bebaviour for slicing long entries into a list of entries shorter than a passed `max_entry_length` value.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We add a behaviour to establish a contract for other custom slicing implementations.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A bebaviour for slicing long entries into a list of entries shorter than a passed `max_entry_length` value.
A behaviour for slicing long entries into a list of entries shorter than a passed `max_entry_length` value.

lib/meta_logger/slicer/default_impl.ex Show resolved Hide resolved
lib/meta_logger/slicer/utf8_impl.ex Show resolved Hide resolved
test/meta_logger/utf8_slicer_test.exs Outdated Show resolved Hide resolved
@Ivor Ivor marked this pull request as ready for review October 24, 2024 19:39
@Ivor Ivor requested review from prodis and nvieirafelipe October 24, 2024 19:39
lib/meta_logger/slicer/default_impl.ex Show resolved Hide resolved
test/meta_logger/slicer_test.exs Outdated Show resolved Hide resolved
lib/meta_logger/slicer/utf8_impl.ex Outdated Show resolved Hide resolved
test/meta_logger/utf8_slicer_test.exs Outdated Show resolved Hide resolved
@nvieirafelipe nvieirafelipe merged commit 1daaa11 into master Oct 25, 2024
3 checks passed
@nvieirafelipe nvieirafelipe deleted the add-middleware-option-for-slicer-module branch October 25, 2024 13:07
@@ -1,46 +1,9 @@
defmodule MetaLogger.Slicer do
@moduledoc """
Responsible for slicing log entries according to the given max length option.
A bebaviour for slicing long entries into a list of entries shorter than a passed `max_entry_length` value.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A bebaviour for slicing long entries into a list of entries shorter than a passed `max_entry_length` value.
A behaviour for slicing long entries into a list of entries shorter than a passed `max_entry_length` value.


"""
@impl MetaLogger.Slicer
@spec slice(binary(), MetaLogger.Slicer.max_entry_length()) :: [binary()]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This spec is not necessary because the specification in the callback.

when byte_size(entry) <= max_entry_length,
do: [entry]

def slice(entry, max_entry_length) do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to add a guard to ensure entry is binary.

Comment on lines +37 to +39
def slice(entry, max_entry_length) do
do_slice(entry, max_entry_length, [], [], 0)
end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +81 to +88
defp bank_partial_slice(slices, partial_slice) do
[reconstruct_current_slice_as_binary(partial_slice) | slices]
end

@spec reconstruct_current_slice_as_binary([iodata()]) :: binary()
defp reconstruct_current_slice_as_binary(current_slice) do
IO.iodata_to_binary(Enum.reverse(current_slice))
end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +21 to +22
test "when max entry length is smaller than the size of given entry, " <>
"returns a list with one entry",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ivor Ivor mentioned this pull request Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants