CORS: allow users to disable preflight caching

[KarolosLykos]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Documentation Update
• Go Version Update
• Dependency Update

Description

This PR follows a different approach (from #249 ) by changing the property to a pointer integer. This change simplifies the codebase, eliminating the need for introducing a new variable and a new check. Despite a slight overhead due to the pointer, this approach results in cleaner code while maintaining the passing status of the existing tests.

Related Tickets & Documents

• Related Issue [FEATURE] Allow users to disable caching of preflight responses #246
• Closes [FEATURE] Allow users to disable caching of preflight responses #246

Added/updated tests?

• Yes
• No, and this is why: please replace this line with details on why tests
  have not been included
• I need help with writing tests

Run verifications and test

• make verify is passing
• make test is passing

[jub0bs]

The downside of using *int is that you're introducing an unnecessary level of indirection. Since the struct fields are unexported, you can afford to introduce a bool field indicating whether a custom max age was set by the user. That's the approach I favoured in #249

[KarolosLykos]

The downside of using *int is that you're introducing an unnecessary level of indirection. Since the struct fields are unexported, you can afford to introduce a bool field indicating whether a custom max age was set by the user. That's the approach I favoured in #249

Thank you for sharing your perspective! From my understanding, using pointer properties offers clarity about the ability to leave a value empty or unset. It helps indicate when a property is intentionally left nil, providing a clear distinction between an intentionally empty value and a default or zero value. While I acknowledge the downsides you mentioned, I'll keep exploring and learning. If there are specific best practices you recommend regarding pointer properties in Go, I'd love to hear more!"

[jub0bs]

No worries. We're all learning, I as well.

using pointer properties offers offers clarity about the ability to leave a value empty or unset

Under the assumption that *int more clearly conveys optionality than the combination of a plain int and a bool would (and I'm not convinced of this), to whom does it offer clarify, though? In this case, only to maintainers of gorilla/handlers, since those fields (your maxAge *int field and my maxAge int and customMaxAge bool fields) would remain non-exported; users of gorilla/handlers could remain blissfully ignorant of those implementation details.

[KarolosLykos]

users of gorilla/handlers could remain blissfully ignorant of those implementation details.

The isn't entirely accurate, as most users tend to inspect the underlying code of a function they wish to use. They often aim to minimize the time needed to comprehend it. In my view, a quick glance at the MaxAg function should allow for a clear understanding of its usage and the expected outcome.

We aim for the user to be able to set the header to a value, 0, or leave it unset. In my view, this functionality is better described using a pointer.

[jub0bs]

@KarolosLykos

The isn't entirely accurate, as most users tend to inspect the underlying code of a function they wish to use.

Users who wish to study the implementation would, of course, see how it's implemented. But the point of information hiding in library design is that users can then afford to be oblivious to the implementation.

They often aim to minimize the time needed to comprehend it. In my view, a quick glance at the MaxAge function should allow for a clear understanding of its usage and the expected outcome.

That's what documentation is about, isn't it? Users shouldn't have to study the source code of a library in order to know how to use it. See Rob Pike's Documentation is for users proverb.

We aim for the user to be able to set the header to a value, 0, or leave it unset. In my view, this functionality is better described using a pointer.

This comment suggests to me that you haven't studied my approach closely enough... 🤔
Which PR (mine or yours) gets merged doesn't matter much, though:

• users would be able to configure the desired max age the exact same way: by calling the MaxAge option;
• maintainers would be free to adopt your implementation first, then change their mind and adopt mine instead, or vice versa.

[jub0bs]

No feedback from maintainers in more than three months... Is this project still maintained?