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

Adds response header handling and unit tests #12

Merged
merged 18 commits into from
Apr 20, 2024

Conversation

tanczosm
Copy link
Collaborator

@tanczosm tanczosm commented Jan 30, 2024

Adds response header handling and unit tests

This commit adds a new file TriggerStyle.cs which contains the TriggerStyle enum. The enum defines three trigger styles for events in an HTMX response.

In addition, this commit modifies the HtmxResponse.cs file to include several new methods:

  • Location(string url): Sets the location header for client-side redirect.
  • PushUrl(string url): Pushes a new URL onto the history stack.
  • Redirect(string url): Sets the redirect header for client-side redirect.
  • Refresh(): Enables a client-side full refresh of the page.
  • ReplaceUrl(string url): Replaces the current URL in the location bar.
  • Reswap(SwapStyle swapStyle): Allows specifying how the response will be swapped.
  • Retarget(string selector): Updates the target of content update to a different element on the page.
  • Reselect(string selector): Allows choosing which part of the response is used to be swapped in.
  • Trigger(string eventName, object? detail = null, TriggerTiming timing = TriggerTiming.Default): Triggers client-side events with optional event details and timing.

The commit also includes unit tests for each of these methods to ensure their functionality. The tests cover scenarios such as adding different headers with different values and checking if the headers are correctly set in the response.

Additionally, a MockHeaderDictionary class is added to provide a mock implementation of the IHeaderDictionary interface for testing purposes. This allows us to simulate HTTP headers in our unit tests without relying on actual HTTP requests or responses.

This commit adds a new file `TriggerStyle.cs` which contains the `TriggerStyle` enum. The enum defines three trigger styles for events in an HTMX response.

In addition, this commit modifies the `HtmxResponse.cs` file to include several new methods:
- `Location(string url)`: Sets the location header for client-side redirect.
- `PushUrl(string url)`: Pushes a new URL onto the history stack.
- `Redirect(string url)`: Sets the redirect header for client-side redirect.
- `Refresh()`: Enables a client-side full refresh of the page.
- `ReplaceUrl(string url)`: Replaces the current URL in the location bar.
- `Reswap(SwapStyle swapStyle)`: Allows specifying how the response will be swapped.
- `Retarget(string selector)`: Updates the target of content update to a different element on the page.
- `Reselect(string selector)`: Allows choosing which part of the response is used to be swapped in.
- `Trigger(string eventName, object? detail = null, TriggerStyle timing = TriggerStyle.Default)`: Triggers client-side events with optional event details and timing.

These changes enhance functionality related to handling HTMX responses.
Switch from HashSet to retain event ordering

The code changes in this commit switch from using a HashSet to using a List in order to retain the ordering of events. This change ensures that the events are stored and outputted in the same order they were added, which is important for maintaining consistency. Additionally, a boolean variable "exists" is introduced to check if an event already exists before adding it to the list.
This commit adds the HtmxResponse class implementation, which is responsible for handling HTTP responses in the Htmxor library. It includes methods for adding various response headers such as Location, PushUrl, Redirect, Refresh, ReplaceUrl, Reswap, Retarget, Reselect, and Trigger.

The commit also includes unit tests for each of these methods to ensure their functionality. The tests cover scenarios such as adding different headers with different values and checking if the headers are correctly set in the response.

Additionally, a MockHeaderDictionary class is added to provide a mock implementation of the IHeaderDictionary interface for testing purposes. This allows us to simulate HTTP headers in our unit tests without relying on actual HTTP requests or responses.

Overall, this commit introduces the necessary code changes to support Htmx response handling and provides comprehensive test coverage for this functionality.
- Change access modifiers from internal to public in `MockHeaderDictionary`, `MockHttpContext`, `MockHttpRequest`, `MockHttpResponse`, and `MockResponseCookies` classes.
@tanczosm
Copy link
Collaborator Author

tanczosm commented Jan 30, 2024

I cleaned this up a little more this morning and renamed TriggerStyle to TriggerTiming since that may be more appropriate as a name. The mechanism of building up the json node largely has to account for existing headers that haven't been processed by the htmxor handler. Once the header becomes json though the parsing is just deserialize the current header and insert the new event/detail and serialize again. The tests are exhaustive afaik for triggers but I'm not sure what Stryker does.. haven't used that before. If you have a set of fake/mock http objects you prefer using they'll need to be swapped out. I just did some barebones implementations

This implementation also preserves trigger ordering inside the returned response.

@egil
Copy link
Owner

egil commented Jan 30, 2024

Off FFS, I cannot figure out how to get the workflow not to run the specific steps that will not work because forks don't have access to workflow secrets. It seems to completely ignore my if: ... conditions.

If you have any ideas whats going on there do let me know.

@egil
Copy link
Owner

egil commented Jan 30, 2024

I need a bit of time to think this through and understand the functionality of each of the response headers to review this. Do bear with me. It may take some days before I can find the free time to do this.

In the meantime, I would like some more details on your thoughts and design of the Trigger functionality.

Also, I do apologize for not having created a CONTRIBUTING.md yet, and also not having set up code analyzers and style guides that enforce a coding style.

@tanczosm
Copy link
Collaborator Author

tanczosm commented Jan 30, 2024

I need a bit of time to think this through and understand the functionality of each of the response headers to review this. Do bear with me. It may take some days before I can find the free time to do this.

In the meantime, I would like some more details on your thoughts and design of the Trigger functionality.

Also, I do apologize for not having created a CONTRIBUTING.md yet, and also not having set up code analyzers and style guides that enforce a coding style.

No problem. If you want to review in the mean time that works.

Trigger thoughts --

Since Trigger is designed to send events there are two options for sending events that are valid for htmx:

  1. (simple) Comma delimited list of event names
  2. (complex) Serialized json where each property is an event name and the value is a payload that is attached to an event detail

Response headers for a particular key are stored as StringValues, which can be null, have one, or many values as a list. As a result it's possible to append headers to the same header key (like a Trigger header key). Asp.net will concatenate the header list and join with a comma if there are multiple headers under the same key.

So a developer could just keep adding event names to the response trigger header key and it would ultimately end up working because they all would be merged with a comma delimiter by asp.net anyway. However, the moment we would try to add a serialized event it would become invalid because htmx wouldn't expect it.

What Trigger does is attempt to merge the new event into whatever happens to exist already in that particular header.. which can be complicated depending on how a developer or third-party library adds their own trigger headers without using Htmxor.

So in order to perform a merge I build a json tree of any existing triggers that exist and treat all of them as json properties. In cases where a serialized (complex) trigger exists I deserialize the trigger and merge any properties into the json tree. Existing properties (event names) are overwritten with the newer values. This effectively normalizes anything present in StringValues. However, if there are no detail payloads present on any event name node it's unnecessary to send a serialized payload and we can fall back to just providing a comma-delimited list of event names.

MergeTrigger receives the return values json and isComplex from BuildExistingTriggerJson. json is the fully constructed JsonNode of all merged events both simple and complex.

If the new incoming event name doesn't have a detail and isComplex is false then we can iterate through all the json properties and add the events in a comma-delimited events (Hx-Trigger: event1,event2,event3)

Otherwise, having a detail at all means the event must be serialized. The detail itself is first serialized and then a node property is set using the eventname. The entire json object is then serialized as the fully-packaged trigger event.

A hot path scenario resembles more of a deserialize/serialize approach:

  1. Developer issues a trigger e.g. Response.Trigger("showMessage")
  2. Single existing Hx-Trigger header is deserialized to a JsonObject
  3. Trigger is added as property/value to the JsonObject
  4. Hx-Trigger is set to the serialized JsonObject

Khalid's Htmx.net library works similarly, but can bypass creating the Json document somewhat by instead adding the events to a dictionary. The problem with that approach though is dictionaries don't retain key order so the result can look different from the original event sequence. In order to optimize the building of Trigger headers all the actual headers get added after a processing event runs but the event sequence gets lost.

So in order to make a call to perform a trigger in htmx.net you would do something like:

Response.Htmx(h => {
    h.PushUrl("/new-url")
     .WithTrigger("cool")
});

For this library it is more straight-forward:

Response.PushUrl("/new-url");
Response.Trigger("cool");
Response.Trigger("showMessage", new { level = "info", message = "Here Is A Message" }, timing: TriggerTiming.AfterSettle);

The resulting Hx-Trigger payload would then be something like:

{
  "cool": "",
  "showMessage": {
    "level": "info",
    "message": "Here is A Message"
  }
}

@tanczosm
Copy link
Collaborator Author

Since CI isn't working and you'll be delayed processing this I'm going to improve some of the non-trigger methods I think. Right now they just set the header to a value but there are specific rules that we enforce directly. This would make the api a bit more complete than htmx.net.

tanczosm and others added 3 commits January 30, 2024 21:47
- Make _serializerOptions field static for improved performance and memory usage.
…etail into the result

This commit modifies the `MergeTrigger` method in the `HtmxResponse` class. It introduces changes to aggregate existing headers and merge events with details into the result. The code now uses a `StringBuilder` to build the JSON representation of aggregated properties across all header values for a given header key. Duplicate keys are not removed for performance reasons, as the produced JSON is still valid. This approach does not validate any syntax of existing headers as a performance consideration.
@tanczosm
Copy link
Collaborator Author

I improved this code performance significantly by removing any parsing/serializing of existing headers. Instead I assume anything present in existing headers is already valid and simply aggregate all of the events as already serialized strings. I'm retaining the previous code description but here is the revised description for the latest commit regarding Triggers.

If the existing trigger looks like this:

{"showMessage":{"level":"info","message":"Here Is A Message"}}

Then to merge those events I can just check to see if the header starts with an opening curly brace { to see if it's json. If so then just append everything but the opening and closing curly braces to the StringBuilder (sb).

sb contents:

"showMessage":{"level":"info","message":"Here Is A Message"}

If I add another serialized event I just need to concatenate a comma and the event name (serialized) onto the end.

action: add "event1" trigger
sb contents:

"showMessage":{"level":"info","message":"Here Is A Message"},"event1":""

Let's say we add another event?
action: add "event2" trigger
sb contents:

"showMessage":{"level":"info","message":"Here Is A Message"},"event1":"","event2":""

Now duplicate the event1 event:
action: add "event2" trigger
sb contents:

"showMessage":{"level":"info","message":"Here Is A Message"},"event1":"","event2":"","event1":""

At this point we can see two event1 keys in the constructed json, which is still valid json. Javascript will automatically use the last key as the value for the property.

When finished, just wrap sb with {} and you have:

{"showMessage":{"level":"info","message":"Here Is A Message"},"event1":"","event2":"","event1":""}

@egil
Copy link
Owner

egil commented Apr 16, 2024

@tanczosm so sorry about the radio silence on this. I am back playing with htmxor again and ill probably have a look at this soon.

@egil egil merged commit 5404cb3 into egil:main Apr 20, 2024
5 of 7 checks passed
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.

2 participants