OO-4340: prevent imported calendars to block the scheduler

src/main/java/org/olat/commons/calendar/CalendarManager.java

@@ -27,6 +27,7 @@ package org.olat.commons.calendar;
 
 import java.io.File;
 import java.io.InputStream;
+import java.net.URLConnection;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
@@ -410,6 +411,14 @@ public interface CalendarManager {
 	 */
 	public Kalendar buildKalendarFrom(InputStream calendarContent, String calType,  String calId);
 	
+	/**
+	 * Create an URL connection with default settings like time out.
+	 * 
+	 * @param url The URL as string
+	 * @return A connection or null
+	 */
+	public URLConnection getURLConnection(String url);
+	
 	/**
 	 * Synchronize the event of the calendar stream to the target calendar,
 	 * set the synchronized events as managed.

src/main/java/org/olat/commons/calendar/ICalServlet.java

@@ -27,6 +27,7 @@
 package org.olat.commons.calendar;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.Writer;
 import java.net.URISyntaxException;
 import java.net.URL;
@@ -44,13 +45,13 @@ import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.logging.log4j.Logger;
 import org.olat.commons.calendar.model.CalendarFileInfos;
 import org.olat.commons.calendar.model.CalendarUserConfiguration;
 import org.olat.core.CoreSpringFactory;
 import org.olat.core.commons.persistence.DBFactory;
 import org.olat.core.id.Identity;
 import org.olat.core.logging.OLATRuntimeException;
-import org.apache.logging.log4j.Logger;
 import org.olat.core.logging.Tracing;
 import org.olat.core.util.StringHelper;
 import org.olat.core.util.i18n.I18nManager;
@@ -119,29 +120,34 @@ public class ICalServlet extends HttpServlet {
 	}
 
 	@Override
-	protected void doGet(HttpServletRequest request, HttpServletResponse response)
-	throws IOException {
+	protected void doGet(HttpServletRequest request, HttpServletResponse response) {
 		String requestUrl = request.getPathInfo();
 		try {
 			//log need a session before the response is committed
 			request.getSession();
-			if (log.isDebugEnabled()) {
-				log.debug("doGet pathInfo=" + requestUrl);
-			}
+			log.debug("doGet pathInfo={}", requestUrl);
 			if ((requestUrl == null) || (requestUrl.equals(""))) {
 				return; // error
 			}
 
 			getIcalDocument(requestUrl, request, response);
 		} catch (ValidationException e) {
-			log.warn("Validation Error when generate iCal stream for path::" + request.getPathInfo(), e);
-			response.sendError(HttpServletResponse.SC_CONFLICT);
+			log.warn("Validation Error when generate iCal stream for path::{}", request.getPathInfo(), e);
+			sendError(response, HttpServletResponse.SC_CONFLICT);
 		} catch (IOException e) {
-			log.warn("IOException Error when generate iCal stream for path::" + request.getPathInfo(), e);
-			response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+			log.warn("IOException Error when generate iCal stream for path::{}", request.getPathInfo(), e);
+			sendError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
 		} catch (Exception e) {
 			log.warn("Unknown Error in icalservlet", e);
-			response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+			sendError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+		}
+	}
+	
+	private void sendError(HttpServletResponse response, int status) {
+		try {
+			response.sendError(status);
+		} catch (IOException e) {
+			log.error("", e);
 		}
 	}
   
@@ -195,7 +201,7 @@ public class ICalServlet extends HttpServlet {
 			calendarID = userName;
 		} else {
 			response.sendError(HttpServletResponse.SC_BAD_REQUEST, requestUrl);
-			log.warn("Type not supported: " + pathInfo);
+			log.warn("Type not supported: {}", pathInfo);
 			return;
 		}
 		
@@ -212,7 +218,7 @@ public class ICalServlet extends HttpServlet {
 			CalendarUserConfiguration config = calendarManager.getCalendarUserConfiguration(Long.parseLong(userName));
 			String savedToken = config == null ? null : config.getToken();
 			if (authToken == null || savedToken == null || !savedToken.equals(authToken)) {
-				log.warn("Authenticity Check failed for the ical feed path: " + pathInfo);
+				log.warn("Authenticity Check failed for the ical feed path: {}", pathInfo);
 				response.sendError(HttpServletResponse.SC_UNAUTHORIZED, requestUrl);
 			} else {
 				generateAggregatedCalendar(config.getIdentity(), request, response);
@@ -228,7 +234,7 @@ public class ICalServlet extends HttpServlet {
 				savedToken = calendarManager.getCalendarToken(calendarType, calendarID, userName);
 			}
 			if (authToken == null || savedToken == null || !savedToken.equals(authToken)) {
-				log.warn("Authenticity Check failed for the ical feed path: " + pathInfo);
+				log.warn("Authenticity Check failed for the ical feed path: {}", pathInfo);
 				response.sendError(HttpServletResponse.SC_UNAUTHORIZED, requestUrl);
 			} else {
 				// read and return the calendar file
@@ -263,9 +269,9 @@ public class ICalServlet extends HttpServlet {
 			Object pobject = propIter.next();
 			if(pobject instanceof Property) {
 				Property property = (Property)pobject;
-				if(Version.VERSION.equals(property.getName())) {
+				if(Property.VERSION.equals(property.getName())) {
 					//we force version 2.0
-				} else if(Version.CALSCALE.equals(property.getName())) {
+				} else if(Property.CALSCALE.equals(property.getName())) {
 					out.write(property.toString());
 					calScale = true;
 				} else {
@@ -376,7 +382,7 @@ public class ICalServlet extends HttpServlet {
 					if(vTimeZone != null) {
 						out.write(vTimeZone.toString());
 					}
-				} catch (IOException | ParserException e) {
+				} catch (Exception e) {
 					log.error("", e);
 				}
 			}
@@ -540,7 +546,7 @@ public class ICalServlet extends HttpServlet {
 								event.getProperties().add(urlProperty);
 								break;
 							} catch (URISyntaxException e) {
-								log.error("Invalid URL:" + uri);
+								log.error("Invalid URL:{}", uri);
 							}
 						}
 					}
@@ -553,13 +559,12 @@ public class ICalServlet extends HttpServlet {
      * Load the VTimeZone for Outlook. ical4j use a static map to reuse the TimeZone objects, we need to load
      * and save our specialized TimeZone in a separate map.
      */
-    private VTimeZone getOutlookVTimeZone(final String id) throws IOException, ParserException {
-    	return outlookVTimeZones.computeIfAbsent(id, (timeZoneId) -> {
+    private VTimeZone getOutlookVTimeZone(final String id) {
+    	return outlookVTimeZones.computeIfAbsent(id, timeZoneId -> {
         	try {
 				URL resource = ResourceLoader.getResource("zoneinfo-outlook/" + id + ".ics");
-				CalendarBuilder builder = new CalendarBuilder();
-				Calendar calendar = builder.build(resource.openStream());
-				return (VTimeZone)calendar.getComponent(Component.VTIMEZONE);
+				Calendar calendar = buildCalendar(resource);
+				return calendar == null ? null : (VTimeZone)calendar.getComponent(Component.VTIMEZONE);
 			} catch (Exception e) {
 				log.error("", e);
 				return null;
@@ -567,6 +572,16 @@ public class ICalServlet extends HttpServlet {
     	});
     }
     
+    private Calendar buildCalendar(URL resource) {
+    	CalendarBuilder builder = new CalendarBuilder();
+    	try(InputStream in = resource.openStream()) {
+    		return builder.build(in);
+    	} catch(IOException | ParserException e) {
+    		log.error("", e);
+    		return null;
+    	}
+    }
+    
     private enum Agent {
     		unkown,
     		outlook,

src/main/java/org/olat/commons/calendar/_spring/calendarContext.xml

@@ -31,7 +31,7 @@
 	</bean>
 	
 	<bean id="calendarImportJob" class="org.springframework.scheduling.quartz.JobDetailFactoryBean">
-	  <property name="jobClass" value="org.olat.commons.calendar.ImportCalendarJob" />
+	  <property name="jobClass" value="org.olat.commons.calendar.manager.ImportCalendarJob" />
 	  <property name="durability" value="true" />
 	</bean>
 	

src/main/java/org/olat/commons/calendar/manager/ICalFileCalendarManager.java

@@ -32,10 +32,13 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.OutputStream;
 import java.net.URISyntaxException;
+import java.net.URL;
+import java.net.URLConnection;
 import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -166,6 +169,17 @@ public class ICalFileCalendarManager implements CalendarManager, InitializingBea
 		calendarCache = CoordinatorManager.getInstance().getCoordinator().getCacher().getCache(CalendarManager.class.getSimpleName(), "calendar");
 	}
 	
+	public URLConnection getURLConnection(String url) {
+		try {
+			URLConnection conn = new URL(url).openConnection();
+			conn.setConnectTimeout(15000);
+			return conn;
+		} catch (IOException e) {
+			log.error("Cannot open URL connection for: {}", url, e);
+			return null;
+		}
+	}
+	
 	/**
 	 * Check if a calendar already exists for the given id.
 	 * @param calendarID

src/main/java/org/olat/commons/calendar/ImportCalendarJob.java → src/main/java/org/olat/commons/calendar/manager/ImportCalendarJob.java

@@ -23,14 +23,22 @@
 * under the Apache 2.0 license as the original file.  
 * <p>
 */
-package org.olat.commons.calendar;
+package org.olat.commons.calendar.manager;
 
+import java.io.InputStream;
+import java.net.URLConnection;
+import java.util.List;
 import java.util.Random;
 
-import org.olat.commons.calendar.manager.ImportToCalendarManager;
+import org.olat.commons.calendar.CalendarManager;
+import org.olat.commons.calendar.model.ImportedToCalendar;
+import org.olat.commons.calendar.model.Kalendar;
 import org.olat.core.CoreSpringFactory;
+import org.olat.core.commons.persistence.DBFactory;
 import org.olat.core.commons.services.scheduler.JobWithDB;
+import org.olat.core.logging.Tracing;
 import org.quartz.JobExecutionContext;
+import org.quartz.Scheduler;
 
 /**
  * 
@@ -47,19 +55,74 @@ public class ImportCalendarJob extends JobWithDB {
 	@Override
 	public void executeWithDB(JobExecutionContext context) {
 		try {
-			jitter();
-			CoreSpringFactory.getImpl(ImportToCalendarManager.class).updateCalendarIn();
+			Scheduler scheduler = CoreSpringFactory.getImpl(Scheduler.class);
+			jitter(scheduler);
+			if(!scheduler.isShutdown()) {
+				updateCalendarIn(scheduler);
+			}
 		} catch (Exception e) {
 			log.error("", e);
 		}
 	}
 	
-	private void jitter() {
+	private boolean updateCalendarIn(Scheduler scheduler) {
+		CalendarManager calendarManager = CoreSpringFactory.getImpl(CalendarManager.class);
+		ImportToCalendarManager importToCalendarManager = CoreSpringFactory.getImpl(ImportToCalendarManager.class);
+		
+		List<ImportedToCalendar> importedToCalendars = importToCalendarManager.getImportedToCalendars();
+		log.info(Tracing.M_AUDIT, "Begin to update {} calendars.", importedToCalendars.size() );
+		
+		//make a full check only every 10 runs
+		boolean check = importToCalendarManager.check();
+
+		for(ImportedToCalendar importedToCalendar:importedToCalendars) {
+			String type = importedToCalendar.getToType();
+			String id = importedToCalendar.getToCalendarId();
+			String importUrl = importedToCalendar.getUrl();
+			if(check || importToCalendarManager.check(importedToCalendar)) {
+				URLConnection connection = calendarManager.getURLConnection(importUrl);
+				if(connection != null) {
+					try(InputStream in = connection.getInputStream()) {
+						Kalendar cal = calendarManager.getCalendar(type, id);
+						if(calendarManager.synchronizeCalendarFrom(in, importUrl, cal)) {
+							log.info(Tracing.M_AUDIT, "Updated successfully calendar: {} / {}", type, id);
+						} else {
+							log.info(Tracing.M_AUDIT, "Failed to update calendar: {} / {}", type, id);
+						}
+					} catch(Exception ex) {
+						log.warn("Cannot synchronize calendar ({}) from url: {}", importedToCalendar.getKey() , importUrl, ex);
+					}
+				}
+			} else {
+				log.info(Tracing.M_AUDIT, "Delete imported calendar because of missing resource: {} / {} with URL: {}", type, id, importUrl);
+				importToCalendarManager.deleteImportedCalendars(type, id);
+			}
+			
+			DBFactory.getInstance().commit();
+	
+			try {
+				if(scheduler.isShutdown()) {
+					return false;
+				}
+				Thread.sleep(1000);// sleep to don't overload the system
+			} catch (Exception e) {
+				log.error("", e);
+			}
+		}
+		return false;
+	}
+	
+	private void jitter(Scheduler scheduler)  {
 		try {
-			double millis = random.nextDouble() * 180000.0d;
-			long wait = Math.round(millis);
-			Thread.sleep(wait);
-		} catch (InterruptedException e) {
+			double numOfWaitingLoops =  random.nextDouble() * 180.0d;
+			long wait = Math.round(numOfWaitingLoops);
+			for(int i=0; i<wait; i++) {
+				if(scheduler.isShutdown()) {
+					return;
+				}
+				Thread.sleep(1000);
+			}
+		} catch (Exception e) {
 			log.error("", e);
 		}
 	}

src/main/java/org/olat/commons/calendar/manager/ImportToCalendarManager.java

@@ -42,7 +42,6 @@ import org.olat.commons.calendar.model.ImportedToCalendar;
 import org.olat.commons.calendar.model.Kalendar;
 import org.olat.commons.calendar.model.KalendarEvent;
 import org.olat.commons.calendar.ui.components.KalendarRenderWrapper;
-import org.olat.core.commons.persistence.DBFactory;
 import org.olat.core.id.Identity;
 import org.olat.core.id.OLATResourceable;
 import org.olat.core.logging.Tracing;
@@ -81,50 +80,14 @@ public class ImportToCalendarManager {
 	@Autowired
 	private ImportedToCalendarDAO importedToCalendarDao;
 	
-	/**
-	 * Method used by the cron job
-	 * @return
-	 */
-	public boolean updateCalendarIn() {
-		List<ImportedToCalendar> importedToCalendars = importedToCalendarDao.getImportedToCalendars();
-		log.info(Tracing.M_AUDIT, "Begin to update " + importedToCalendars.size() + " calendars.");
-		
-		//make a full check only every 10 runs
-		boolean check = counter.incrementAndGet() % 10 == 0;
-
-		int count = 0;
-		for(ImportedToCalendar importedToCalendar:importedToCalendars) {
-			String type = importedToCalendar.getToType();
-			String id = importedToCalendar.getToCalendarId();
-			String importUrl = importedToCalendar.getUrl();
-			if(check || check(importedToCalendar)) {
-				try(InputStream in = new URL(importUrl).openStream()) {
-					Kalendar cal = calendarManager.getCalendar(type, id);
-					if(calendarManager.synchronizeCalendarFrom(in, importUrl, cal)) {
-						log.info(Tracing.M_AUDIT, "Updated successfully calendar: " + type + " / " + id);
-					} else {
-						log.info(Tracing.M_AUDIT, "Failed to update calendar: " + type + " / " + id);
-					}
-				} catch(Exception ex) {
-					log.warn("Cannot synchronize calendar (" + importedToCalendar.getKey() + ") from url: " + importUrl, ex);
-				}
-			} else {
-				log.info(Tracing.M_AUDIT, "Delete imported calendar because of missing resource: " + type + " " + id + " with URL: " + importUrl);
-				deleteImportedCalendars(type, id);
-			}
-			
-			if(count++ % 20 == 0) {
-				DBFactory.getInstance().commit();
-				
-				try {
-					Thread.sleep(1000);// sleep to don't overload the system
-				} catch (InterruptedException e) {
-					log.error("", e);
-				}
-			}
-		}
-		return false;
+	protected List<ImportedToCalendar> getImportedToCalendars() {
+		return importedToCalendarDao.getImportedToCalendars();
 	}
+	
+	protected boolean check() {
+		return counter.incrementAndGet() % 10 == 0;
+	}
+	
 	public void deleteImportedCalendarsAndEvents(Kalendar cal) {
 		List<ImportedToCalendar> importedToCalendars = importedToCalendarDao
 				.getImportedToCalendars(cal.getCalendarID(), cal.getType());
@@ -156,7 +119,7 @@ public class ImportToCalendarManager {
 		importedToCalendarDao.delete(importedToCalendar);
 	}
 	
-	private boolean check(ImportedToCalendar importedToCalendar) {
+	protected boolean check(ImportedToCalendar importedToCalendar) {
 		String id = importedToCalendar.getToCalendarId();
 		String type = importedToCalendar.getToType();
 		if(CalendarManager.TYPE_USER.equals(type)) {
@@ -238,7 +201,7 @@ public class ImportToCalendarManager {
 		deleteImportedCalendars(CalendarManager.TYPE_COURSE, course.getResourceableId().toString());
 	}
 
-	private void deleteImportedCalendars(String type, String id) {
+	protected void deleteImportedCalendars(String type, String id) {
 		List<ImportedToCalendar> importedToCalendars = importedToCalendarDao.getImportedToCalendars(id, type);
 		for(ImportedToCalendar importedToCalendar:importedToCalendars) {
 			importedToCalendarDao.delete(importedToCalendar);