collective / icalendar

icalendar parser library for Python
https://icalendar.readthedocs.io
Other
1k stars 170 forks source link

unique identifier must be included in VEVENT, VTODO, and VJOURNAL #315

Open joooeey opened 4 years ago

joooeey commented 4 years ago

Issue

As per RFC 5545 every VEVENT, VTODO, and VJOURNAL entry must have a unique identifier (UID). This module does not currently implement this requirement and hence produces output incompatible with RFC 5545 if the user does not manually supply an UID.

Proposed solution 1

Every time an event is initialized with icalendar.Event(), the UID field is added automatically and populated with a unique identifier. Then the user can still specify their own UID.

The question is how to get a unique identifier. Perhaps do it similar to the example given in RFC 5545: concatenate a timestamp, a sufficiently large pseudo-random string, sequential number, and "@collective.github.io".

Proposed solution 2

Emit a warning or error if the user does not specify the UID property.

joooeey commented 4 years ago

I overlooked that we already have a way to get a UID:

from tools import UIDGenerator
uid = UIDGenerator.uid("@collective.github.io")
joooeey commented 4 years ago

Anyways, the issue appears to be in cal.py in the last line of the Event class: ignore_exceptions = True. This looks intended. Why do we do this?

I think this flag applies to both parsing and file generation. But then I think there's never a reason to ignore exceptions when generating a file? It certainly shouldn't be the default!

niccokunzmann commented 2 years ago

@jacadzaca did some work on this in #390.

I have suggestions of how to go about this:

  1. Add a is_valid_rfc_5455() to the classes with requirements. These functions document what is required to be RFC 5545 compatible. They might also check for invalid attribute names. Also the function doc string links to (2).
  2. How to generate valid components should be stated in the documentation. One subsection per component as an example.
  3. Make to_ical() raise a warning with the docstring content/a link to the documentation if is_valid_rfc_5455() returns false. This should not break production code and also guide while developing.

@joooeey @jacadzaca do you have thoughs on this?

joooeey commented 2 years ago

This sounds good. Better than my original idea of putting the responsibility of choosing UIDs on the icalendar software.

Regarding 3, I don't think a warning is enough in the long run. I propose making this a deprecation warning which will be an error a few versions down the line as @geier suggested here. Compatibility is crucial.

niccokunzmann commented 2 years ago

Warnings have filters. We can use the filter to make the warning become an error as well as provide an option to allow RFC-invalid icals - switch from error to warning to mute.

niccokunzmann commented 5 months ago

See also: https://github.com/collective/icalendar/discussions/662