Skip to content

Commit

Permalink
Reduce cognitive complexity & improve JCalValue unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@mangstadt
mangstadt committed Nov 11, 2023
1 parent 3394da0 commit 7de233f
Show file tree
Hide file tree
Showing 7 changed files with 245 additions and 122 deletions.
114 changes: 64 additions & 50 deletions src/main/java/biweekly/component/VAlarm.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -520,65 +520,79 @@ public void setTrigger(Trigger trigger) {
protected void validate(List<ICalComponent> components, ICalVersion version, List<ValidationWarning> warnings) {
checkRequiredCardinality(warnings, Action.class, Trigger.class);

validateAction(warnings);
validateTrigger(components, warnings);
}

@SuppressWarnings("unchecked")
private void validateAction(List<ValidationWarning> warnings) {
Action action = getAction();
if (action != null) {
//AUDIO alarms should not have more than 1 attachment
if (action.isAudio()) {
if (getAttachments().size() > 1) {
warnings.add(new ValidationWarning(7));
}
}
if (action == null) {
return;
}

//DESCRIPTION is required for DISPLAY alarms
if (action.isDisplay()) {
checkRequiredCardinality(warnings, Description.class);
//AUDIO alarms should not have more than 1 attachment
if (action.isAudio()) {
if (getAttachments().size() > 1) {
warnings.add(new ValidationWarning(7));
}
}

if (action.isEmail()) {
//SUMMARY and DESCRIPTION is required for EMAIL alarms
checkRequiredCardinality(warnings, Summary.class, Description.class);

//EMAIL alarms must have at least 1 ATTENDEE
if (getAttendees().isEmpty()) {
warnings.add(new ValidationWarning(8));
}
} else {
//only EMAIL alarms can have ATTENDEEs
if (!getAttendees().isEmpty()) {
warnings.add(new ValidationWarning(9));
}
}
//DESCRIPTION is required for DISPLAY alarms
if (action.isDisplay()) {
checkRequiredCardinality(warnings, Description.class);
}

if (action.isEmail()) {
//SUMMARY and DESCRIPTION is required for EMAIL alarms
checkRequiredCardinality(warnings, Summary.class, Description.class);

if (action.isProcedure()) {
checkRequiredCardinality(warnings, Description.class);
//EMAIL alarms must have at least 1 ATTENDEE
if (getAttendees().isEmpty()) {
warnings.add(new ValidationWarning(8));
}
} else {
//only EMAIL alarms can have ATTENDEEs
if (!getAttendees().isEmpty()) {
warnings.add(new ValidationWarning(9));
}
}

if (action.isProcedure()) {
checkRequiredCardinality(warnings, Description.class);
}
}

private void validateTrigger(List<ICalComponent> components, List<ValidationWarning> warnings) {
Trigger trigger = getTrigger();
if (trigger != null) {
Related related = trigger.getRelated();
if (related != null) {
ICalComponent parent = components.get(components.size() - 1);

//if the TRIGGER is relative to DTSTART, confirm that DTSTART exists
if (related == Related.START && parent.getProperty(DateStart.class) == null) {
warnings.add(new ValidationWarning(11));
}

//if the TRIGGER is relative to DTEND, confirm that DTEND (or DUE) exists
if (related == Related.END) {
boolean noEndDate = false;

if (parent instanceof VEvent) {
noEndDate = (parent.getProperty(DateEnd.class) == null && (parent.getProperty(DateStart.class) == null || parent.getProperty(DurationProperty.class) == null));
} else if (parent instanceof VTodo) {
noEndDate = (parent.getProperty(DateDue.class) == null && (parent.getProperty(DateStart.class) == null || parent.getProperty(DurationProperty.class) == null));
}

if (noEndDate) {
warnings.add(new ValidationWarning(12));
}
}
if (trigger == null) {
return;
}

Related related = trigger.getRelated();
if (related == null) {
return;
}

ICalComponent parent = components.get(components.size() - 1);

//if the TRIGGER is relative to DTSTART, confirm that DTSTART exists
if (related == Related.START && parent.getProperty(DateStart.class) == null) {
warnings.add(new ValidationWarning(11));
}

//if the TRIGGER is relative to DTEND, confirm that DTEND (or DUE) exists
if (related == Related.END) {
boolean noEndDate = false;

if (parent instanceof VEvent) {
noEndDate = (parent.getProperty(DateEnd.class) == null && (parent.getProperty(DateStart.class) == null || parent.getProperty(DurationProperty.class) == null));
} else if (parent instanceof VTodo) {
noEndDate = (parent.getProperty(DateDue.class) == null && (parent.getProperty(DateStart.class) == null || parent.getProperty(DurationProperty.class) == null));
}

if (noEndDate) {
warnings.add(new ValidationWarning(12));
}
}
}
Expand Down
15 changes: 10 additions & 5 deletions src/main/java/biweekly/io/StreamReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,13 @@ private void handleTimezones(ICalendar ical) {
while (it.hasNext()) {
VTimezone component = it.next();

//make sure the component has an ID
String id = ValuedProperty.getValue(component.getTimezoneId());
if (id == null || id.trim().isEmpty()) {
//note: do not remove invalid VTIMEZONE components from the ICalendar object
TimeZone timezone = buildTimeZone(component);
if (timezone == null) {
//do not remove invalid VTIMEZONE components from the ICalendar object
warnings.add(new ParseWarning.Builder().message(39).build());
continue;
}

TimeZone timezone = new ICalTimeZone(component);
tzinfo.getTimezones().add(new TimezoneAssignment(timezone, component));

//remove the component from the ICalendar object
Expand Down Expand Up @@ -282,6 +280,13 @@ private void handleTimezones(ICalendar ical) {
}
}

private TimeZone buildTimeZone(VTimezone component) {
String id = ValuedProperty.getValue(component.getTimezoneId());
boolean idMissing = (id == null || id.trim().isEmpty());

return idMissing ? null : new ICalTimeZone(component);
}

private void reparseDateUnderDifferentTimezone(TimezonedDate timezonedDate, Calendar cal) {
ICalDate date = timezonedDate.getDate();

Expand Down
71 changes: 46 additions & 25 deletions src/main/java/biweekly/io/json/JCalValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,37 +228,20 @@ public List<List<String>> asStructured() {
List<List<String>> components = new ArrayList<List<String>>(array.size());
for (JsonValue value : array) {
if (value.isNull()) {
components.add(Collections.<String>emptyList());
components.add(asStructuredNull());
continue;
}

Object obj = value.getValue();
if (obj != null) {
String s = obj.toString();
List<String> component = s.isEmpty() ? Collections.<String>emptyList() : Collections.singletonList(s);
components.add(component);
components.add(asStructuredValue(obj));
continue;
}

List<JsonValue> subArray = value.getArray();
if (subArray != null) {
List<String> component = new ArrayList<String>(subArray.size());
for (JsonValue subArrayValue : subArray) {
if (subArrayValue.isNull()) {
component.add("");
continue;
}

obj = subArrayValue.getValue();
if (obj != null) {
component.add(obj.toString());
continue;
}
}
if (component.size() == 1 && component.get(0).isEmpty()) {
component.clear();
}
components.add(component);
components.add(asStructuredSubArray(subArray));
continue;
}
}
return components;
Expand All @@ -269,19 +252,57 @@ public List<List<String>> asStructured() {
Object obj = first.getValue();
if (obj != null) {
List<List<String>> components = new ArrayList<List<String>>(1);
String s = obj.toString();
List<String> component = s.isEmpty() ? Collections.<String>emptyList() : Collections.singletonList(s);
components.add(component);
components.add(asStructuredValue(obj));
return components;
}

//["request-status", {}, "text", null]
if (first.isNull()) {
List<List<String>> components = new ArrayList<List<String>>(1);
components.add(Collections.<String>emptyList());
components.add(asStructuredNull());
return components;
}

/*
* JSON objects are ignored.
*/

return Collections.emptyList();
}

private List<String> asStructuredSubArray(List<JsonValue> subArray) {
List<String> component = new ArrayList<String>(subArray.size());

for (JsonValue subArrayValue : subArray) {
if (subArrayValue.isNull()) {
component.add("");
continue;
}

Object obj = subArrayValue.getValue();
if (obj != null) {
component.add(obj.toString());
continue;
}

/*
* JSON objects and arrays inside of sub-arrays are ignored.
*/
}

if (component.size() == 1 && component.get(0).isEmpty()) {
component.clear();
}

return component;
}

private List<String> asStructuredValue(Object obj) {
String s = obj.toString();
return s.isEmpty() ? Collections.<String> emptyList() : Collections.singletonList(s);
}

private List<String> asStructuredNull() {
return Collections.emptyList();
}

Expand Down
8 changes: 8 additions & 0 deletions src/main/java/biweekly/io/json/JsonValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ public JsonValue(Map<String, JsonValue> object) {
isNull = (object == null);
}

/**
* Creates a null JSON value.
* @return the JSON value
*/
public static JsonValue nullValue() {
return new JsonValue((Object) null);
}

/**
* Gets the JSON value.
* @return the value or null if it's not a JSON value
Expand Down
63 changes: 41 additions & 22 deletions src/main/java/biweekly/io/text/ICalWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@ private void writeComponent(ICalComponent component, ICalComponent parent) throw
writer.writeBeginComponent(componentScribe.getComponentName());

List propertyObjs = componentScribe.getProperties(component);
if (inICalendar && component.getProperty(Version.class) == null) {
propertyObjs.add(0, new Version(getTargetVersion()));
if (inICalendar) {
addVersionIfMissing(component, propertyObjs);
}

for (Object propertyObj : propertyObjs) {
Expand All @@ -252,13 +252,7 @@ private void writeComponent(ICalComponent component, ICalComponent parent) throw

List subComponents = componentScribe.getComponents(component);
if (inICalRoot) {
//add the VTIMEZONE components
Collection<VTimezone> timezones = getTimezoneComponents();
for (VTimezone timezone : timezones) {
if (!subComponents.contains(timezone)) {
subComponents.add(0, timezone);
}
}
addTimezonesIfMissing(subComponents);
}

for (Object subComponentObj : subComponents) {
Expand All @@ -267,24 +261,49 @@ private void writeComponent(ICalComponent component, ICalComponent parent) throw
}

if (inVCalRoot) {
Collection<VTimezone> timezones = getTimezoneComponents();
if (!timezones.isEmpty()) {
VTimezone timezone = timezones.iterator().next();
VCalTimezoneProperties props = convert(timezone, context.getDates());

Timezone tz = props.getTz();
if (tz != null) {
writeProperty(tz);
}
for (Daylight daylight : props.getDaylights()) {
writeProperty(daylight);
}
}
writeVCalTimezones();
}

writer.writeEndComponent(componentScribe.getComponentName());
}

@SuppressWarnings({ "rawtypes", "unchecked" })
private void addVersionIfMissing(ICalComponent component, List propertyObjs) {
if (component.getProperty(Version.class) != null) {
return;
}

propertyObjs.add(0, new Version(getTargetVersion()));
}

@SuppressWarnings({ "unchecked", "rawtypes" })
private void addTimezonesIfMissing(List subComponents) {
Collection<VTimezone> timezones = getTimezoneComponents();
for (VTimezone timezone : timezones) {
if (!subComponents.contains(timezone)) {
subComponents.add(0, timezone);
}
}
}

private void writeVCalTimezones() throws IOException {
Collection<VTimezone> timezones = getTimezoneComponents();
if (timezones.isEmpty()) {
return;
}

VTimezone timezone = timezones.iterator().next();
VCalTimezoneProperties props = convert(timezone, context.getDates());

Timezone tz = props.getTz();
if (tz != null) {
writeProperty(tz);
}
for (Daylight daylight : props.getDaylights()) {
writeProperty(daylight);
}
}

@SuppressWarnings({ "rawtypes", "unchecked" })
private void writeProperty(ICalProperty property) throws IOException {
ICalPropertyScribe scribe = index.getPropertyScribe(property);
Expand Down
Loading

0 comments on commit 7de233f

Please sign in to comment.