Skip to content

Commit

Permalink
Fixed resolving timezones that was impacting performance when parsing…
Browse files Browse the repository at this point in the history 
… large calendar files
  • Loading branch information
@benfortuna
benfortuna committed Mar 26, 2020
1 parent ed75510 commit c9a3068
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 56 deletions.
100 changes: 48 additions & 52 deletions src/main/java/net/fortuna/ical4j/data/DefaultContentHandler.java
View file
@@ -1,11 +1,14 @@
package net.fortuna.ical4j.data;

import net.fortuna.ical4j.model.*;
import net.fortuna.ical4j.model.component.*;
import net.fortuna.ical4j.model.component.CalendarComponent;
import net.fortuna.ical4j.model.component.VTimeZone;
import net.fortuna.ical4j.model.parameter.TzId;
import net.fortuna.ical4j.model.property.DateListProperty;
import net.fortuna.ical4j.model.property.DateProperty;
import net.fortuna.ical4j.util.Constants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.net.URISyntaxException;
Expand All @@ -17,6 +20,8 @@

public class DefaultContentHandler implements ContentHandler {

private static final Logger LOG = LoggerFactory.getLogger(DefaultContentHandler.class);

private final Supplier<List<ParameterFactory<?>>> parameterFactorySupplier;

private final Supplier<List<PropertyFactory<?>>> propertyFactorySupplier;
Expand All @@ -25,7 +30,9 @@ public class DefaultContentHandler implements ContentHandler {

private final TimeZoneRegistry tzRegistry;

private List<TzId> propertiesWithTzId;
private List<Property> propertiesWithTzId;

private boolean propertyHasTzId = false;

private final Consumer<Calendar> consumer;

Expand Down Expand Up @@ -62,25 +69,8 @@ public void startCalendar() {

@Override
public void endCalendar() throws IOException {
if (propertiesWithTzId.size() > 0 && tzRegistry != null) {
for (CalendarComponent component : calendar.getComponents()) {
resolveTimezones(component.getProperties());
resolveTimezones();

if (component instanceof VAvailability) {
for (Component available : ((VAvailability) component).getAvailable()) {
resolveTimezones(available.getProperties());
}
} else if (component instanceof VEvent) {
for (Component alarm : ((VEvent) component).getAlarms()) {
resolveTimezones(alarm.getProperties());
}
} else if (component instanceof VToDo) {
for (Component todo : ((VToDo) component).getAlarms()) {
resolveTimezones(todo.getProperties());
}
}
}
}
consumer.accept(calendar);
}

Expand Down Expand Up @@ -119,6 +109,7 @@ public void endComponent(String name) {
@Override
public void startProperty(String name) {
propertyBuilder = new PropertyBuilder().factories(propertyFactorySupplier.get()).name(name);
propertyHasTzId = false;
}

@Override
Expand All @@ -131,6 +122,9 @@ public void endProperty(String name) throws URISyntaxException, ParseException,
assertProperty(propertyBuilder);
Property property = propertyBuilder.build();

if (propertyHasTzId) {
propertiesWithTzId.add(property);
}
// replace with a constant instance if applicable..
property = Constants.forProperty(property);
if (componentBuilder != null) {
Expand All @@ -157,7 +151,7 @@ public void parameter(String name, String value) throws URISyntaxException {
// VTIMEZONE may be defined later, so so keep
// track of dates until all components have been
// parsed, and then try again later
propertiesWithTzId.add((TzId) parameter);
propertyHasTzId = true;
}

propertyBuilder.parameter(parameter);
Expand All @@ -175,39 +169,41 @@ private void assertProperty(PropertyBuilder property) {
}
}

private void resolveTimezones(List<Property> properties) throws IOException {
private void resolveTimezones() throws IOException {

// Go through each property and try to resolve the TZID.
for (TzId tzParam : propertiesWithTzId) {

//lookup timezone
final TimeZone timezone = tzRegistry.getTimeZone(tzParam.getValue());

// If timezone found, then update date property
if (timezone != null) {
for (Property property : properties) {
if (tzParam.equals(property.getParameter(Parameter.TZID))) {

// Get the String representation of date(s) as
// we will need this after changing the timezone
final String strDate = property.getValue();

// Change the timezone
if (property instanceof DateProperty) {
((DateProperty) property).setTimeZone(timezone);
} else if (property instanceof DateListProperty) {
((DateListProperty) property).setTimeZone(timezone);
} else {
throw new CalendarException("Invalid parameter: " + tzParam.getName());
}

// Reset value
try {
property.setValue(strDate);
} catch (ParseException | URISyntaxException e) {
// shouldn't happen as its already been parsed
throw new CalendarException(e);
}
for (Property property : propertiesWithTzId) {

TzId tzParam = property.getParameter(Parameter.TZID);

// extra null check in case validation has removed the TZID param..
if (tzParam != null) {

//lookup timezone
final TimeZone timezone = tzRegistry.getTimeZone(tzParam.getValue());

// If timezone found, then update date property
if (timezone != null) {

// Get the String representation of date(s) as
// we will need this after changing the timezone
final String strDate = property.getValue();

// Change the timezone
if (property instanceof DateProperty) {
((DateProperty) property).setTimeZone(timezone);
} else if (property instanceof DateListProperty) {
((DateListProperty) property).setTimeZone(timezone);
} else {
LOG.warn("Property [%s] doesn't support parameter [%s]", property.getName(), tzParam.getName());
}

// Reset value
try {
property.setValue(strDate);
} catch (ParseException | URISyntaxException e) {
// shouldn't happen as its already been parsed
throw new CalendarException(e);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/net/fortuna/ical4j/model/PropertyBuilder.java
View file
Expand Up @@ -54,9 +54,9 @@ public Property build() throws ParseException, IOException, URISyntaxException {

if (property == null) {
if (isExperimentalName(name)) {
return new XProperty(name, parameters, value);
property = new XProperty(name, parameters, value);
} else if (allowIllegalNames()) {
return new XProperty(name, parameters, value);
property = new XProperty(name, parameters, value);
} else {
throw new IllegalArgumentException("Illegal property [" + name + "]");
}
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/net/fortuna/ical4j/model/property/RRule.java
View file
Expand Up @@ -33,6 +33,7 @@

import net.fortuna.ical4j.model.*;
import net.fortuna.ical4j.model.Recur.Frequency;
import net.fortuna.ical4j.validate.ParameterValidator;
import net.fortuna.ical4j.validate.ValidationException;

import java.io.IOException;
Expand Down Expand Up @@ -123,7 +124,7 @@ public final String getValue() {

@Override
public void validate() throws ValidationException {

ParameterValidator.assertNone(Parameter.TZID, getParameters());
}

public static class Factory extends Content.Factory implements PropertyFactory {
Expand Down
5 changes: 5 additions & 0 deletions src/test/groovy/net/fortuna/ical4j/model/ParameterBuilderTest.groovy
View file
Expand Up @@ -2,10 +2,15 @@ package net.fortuna.ical4j.model


import net.fortuna.ical4j.model.parameter.Value
import net.fortuna.ical4j.util.CompatibilityHints
import spock.lang.Specification

class ParameterBuilderTest extends Specification {

def setupSpec() {
CompatibilityHints.setHintEnabled(CompatibilityHints.KEY_RELAXED_PARSING, false)
}

def 'test build parameter'() {
given: 'a parameter builder instance'
ParameterBuilder builder = []
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/net/fortuna/ical4j/data/CalendarBuilderTest.java
View file
Expand Up @@ -148,7 +148,7 @@ public static Test suite() throws FileNotFoundException {
File[] testFiles = null;

// valid tests..
testFiles = new File("src/test/resources/samples/valid").listFiles(f -> !f.isDirectory() && f.getName().endsWith(".ics"));
testFiles = new File("src/test/resources/samples/valid").listFiles(f -> !f.isDirectory() && f.getName().endsWith("japan_west.ics"));
for (int i = 0; i < testFiles.length; i++) {
log.info("Sample [" + testFiles[i] + "]");
suite.addTest(new CalendarBuilderTest("testBuildValid", testFiles[i].getPath()));
Expand Down

0 comments on commit c9a3068

Please sign in to comment.