URL support for parsing (issue #77) #78

Courtcircuits · 2023-09-24T11:23:58Z

In response to issue #77, I have made some improvements to the codebase. Specifically, I have added a new method ParseCalendarFromUrl(string) and a new test case that verifies the absence of errors when invoking this method.

Currently, this method initializes a basic http.Client and forwards the response body to the ParseCalendar(string) function. However, as suggested in issue #77, it may be beneficial to allow users to use a custom http.Client to address authentication, CORS, and similar concerns.

I am currently exploring the best design approach to accommodate this feature request. Any input or suggestions on the optimal design for this enhancement would be greatly appreciated.

james-elastic · 2023-10-19T19:34:39Z

This would be a great help if it was integrated. Thank you!

arran4 · 2023-10-19T21:40:09Z

Okay noted. I will (try to make time to...) mock the active HTTP get and make what ever changes necessary to do that this weekend. Unless it's something either of you want to do? @james-elastic @Courtcircuits

Courtcircuits · 2023-10-20T13:23:59Z

By the way, I can enhance this pull request. Currently, there isn't much to it. I'm considering implementing the "functional option pattern" to instantiate the HTTP client (more about this pattern : YouTube video).

@arran4, I can also include a few more tests (e.g mock of HTTP connection). I recognize that the ones I added may not be entirely relevant.

arran4 · 2023-10-21T02:39:52Z

Thanks.

The functional options pattern is fine, however just a var-args with a type swtich could do as we don't really need meta information around the http.Client.

I am not sure if more tests are warranted as there isn't much logic, definitely need to test the http.Options solution. Also for people have need custom headers, ie authorization, post, etc. You might want to split it into two (or more) functions, where the http.Request is created in the Get and passed to a http.Do function, which itself is also a constructor.

Please try use fmt.Errorf to wrap the errors if it adds value.

arran4

As discussed.

arran4 · 2023-10-24T23:07:30Z

calendar_test.go

+func TestIssue77(t *testing.T) {
+	url := "https://proseconsult.umontpellier.fr/jsp/custom/modules/plannings/direct_cal.jsp?data=58c99062bab31d256bee14356aca3f2423c0f022cb9660eba051b2653be722c4c7f281e4e3ad06b85d3374100ac416a4dc5c094f7d1a811b903031bde802c7f50e0bd1077f9461bed8f9a32b516a3c63525f110c026ed6da86f487dd451ca812c1c60bb40b1502b6511435cf9908feb2166c54e36382c1aa3eb0ff5cb8980cdb,1"
+
+	cal, err := ParseCalendarFromUrl(url)


This looks like it would be making a real HTTPS request in tests. It should be using a mock http client.

arran4 · 2023-10-24T23:08:03Z

README.md

-Creating:
+Usage, parsing from a URL :
+```golang
+    cal, err := ParseCalendar("an-ics-url")


Minor, but an example URL + a http client custom example might be nice. I've been contemplating making this more readable.

arran4 · 2024-09-28T08:35:45Z

Resolved conflicts in #102

arran4 · 2024-09-28T13:18:09Z

@Courtcircuits I created a new PR to up date this, can you please review it: #102

I will merge it (or make more changes) in the coming days if no response.

arran4 · 2024-10-15T12:09:33Z

Yay!

* master: Duplication issue fixed. Test fixed. Resynced and moved some functions around #78 added functionnal option pattern for url parsing usage example, README calendar parsing url support # Conflicts: # calendar_test.go

* master: Duplication issue fixed. Some multiple-ness. Test fixed. Resynced and moved some functions around #78 added functionnal option pattern for url parsing usage example, README calendar parsing url support # Conflicts: # components.go

* origin/master: Duplication issue fixed. Some multiple-ness. Test fixed. Resynced and moved some functions around #78 added functionnal option pattern for url parsing usage example, README calendar parsing url support

* origin/master: Duplication issue fixed. Some multiple-ness. Test fixed. Resynced and moved some functions around #78 added functionnal option pattern for url parsing usage example, README calendar parsing url support # Conflicts: # calendar.go # calendar_test.go # components.go

* origin/master: Duplication issue fixed. Some multiple-ness. Test fixed. Resynced and moved some functions around #78 added functionnal option pattern for url parsing usage example, README calendar parsing url support # Conflicts: # calendar.go

* Add leaks and vunerability checks * Requires secrets now * Renamed interface as it's useful own it's own. Made it public. Added a comment * fix: omit zone in "AllDay" event helpers For a date-only event (i.e., an event that lasts for the full day) the iCalendar specification indicates that the value for DTSTART / DTEND should be a DATE https://icalendar.org/iCalendar-RFC-5545/3-6-1-event-component.html > The "VEVENT" is also the calendar component used to specify an > anniversary or daily reminder within a calendar. These events have a > DATE value type for the "DTSTART" property instead of the default value > type of DATE-TIME. If such a "VEVENT" has a "DTEND" property, it MUST be > specified as a DATE value also The DATE format (https://icalendar.org/iCalendar-RFC-5545/3-3-4-date.html) should omit both time and zone/location elements and additionally notes that "The "TZID" property parameter MUST NOT be applied to DATE properties" As per the specification, this PR also adds an explicit "VALUE=DATE" parameter when the AllDay helpers were called, to indicate that the property's default value type has been overridden and the VEVENT is intended to be an all-day event https://icalendar.org/iCalendar-RFC-5545/3-2-20-value-data-types.html Finally the SetDuration call has been updated to preserve the "AllDay" characteristics if the existing start or end has been specified in DATE format, which is also a requirement of the spec. Contributes-to: arran4#55 * calendar parsing url support * usage example, README * added functionnal option pattern for url parsing * Should be able to distinguish unset from invalid time properties * Improve error for property not found Co-authored-by: Arran Ubels <arran4@gmail.com> * Remove deprecated ioutil * Move targeted Go version to 1.20 And test the target forever * Add method to remove property Fixes arran4#95 * Merged * New tool * Reintegration of arran4#67 * Resynced and moved some functions around arran4#78 * Test fixed. * Create bug.md * Create other.md * Create default.md * Rename default.md to pull_request_template.md * Minor update * refactor: remove unnecessary named return values, harmonizing code base * refactor: rename var to reflect what it is These functions take optional variadic PropertyParam arguments, in ical speak they are not properties, but parameters for a property. * refactor: use ReplaceAll * refactor: prefer switch for readability * refactor: use consistent receiver names * refactor: rename unused arg as '_' * Tests added. * Some larger upgrades. * Fix * Some multiple-ness. * Duplication issue fixed. * Merged * Merge remote-tracking branch 'origin/master' into issue97 * origin/master: Duplication issue fixed. Some multiple-ness. Test fixed. Resynced and moved some functions around arran4#78 added functionnal option pattern for url parsing usage example, README calendar parsing url support # Conflicts: # calendar.go # calendar_test.go # components.go --------- Co-authored-by: Dominic Evans <dominic.evans@uk.ibm.com> Co-authored-by: tradulescu <tristan-mihai.radulescu@etu.umontpellier.fr> Co-authored-by: Bracken Dawson <abdawson@gmail.com> Co-authored-by: Daniel Lublin <daniel@lublin.se>

Courtcircuits added 2 commits September 23, 2023 23:56

calendar parsing url support

d60522a

usage example, README

dadd290

arran4 self-requested a review October 21, 2023 02:45

arran4 requested changes Oct 21, 2023

View reviewed changes

added functionnal option pattern for url parsing

6d9f344

arran4 requested changes Oct 24, 2023

View reviewed changes

arran4 added a commit that referenced this pull request Sep 28, 2024

Resynced and moved some functions around #78

59af956

arran4 mentioned this pull request Sep 28, 2024

URL-support-for-parsing-78 #102

Merged

arran4 merged commit 56133a5 into arran4:master Oct 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

URL support for parsing (issue #77) #78

URL support for parsing (issue #77) #78

Courtcircuits commented Sep 24, 2023

james-elastic commented Oct 19, 2023

arran4 commented Oct 19, 2023

Courtcircuits commented Oct 20, 2023 •

edited

Loading

arran4 commented Oct 21, 2023 •

edited

Loading

arran4 left a comment

arran4 Oct 24, 2023

arran4 Oct 24, 2023

arran4 commented Sep 28, 2024

arran4 commented Sep 28, 2024

arran4 commented Oct 15, 2024

URL support for parsing (issue #77) #78

URL support for parsing (issue #77) #78

Conversation

Courtcircuits commented Sep 24, 2023

james-elastic commented Oct 19, 2023

arran4 commented Oct 19, 2023

Courtcircuits commented Oct 20, 2023 • edited Loading

arran4 commented Oct 21, 2023 • edited Loading

arran4 left a comment

Choose a reason for hiding this comment

arran4 Oct 24, 2023

Choose a reason for hiding this comment

arran4 Oct 24, 2023

Choose a reason for hiding this comment

arran4 commented Sep 28, 2024

arran4 commented Sep 28, 2024

arran4 commented Oct 15, 2024

Courtcircuits commented Oct 20, 2023 •

edited

Loading

arran4 commented Oct 21, 2023 •

edited

Loading