[MGNLRSSAGG-166] Re-add again an easy way to schedule periodic imports besides cron Created: 20/Mar/14  Updated: 01/Jul/15  Resolved: 02/Mar/15

Status: Closed
Project: Magnolia RSS Aggregator Module
Component/s: rss_aggregator_ui
Affects Version/s: 2.3, 2.3.1
Fix Version/s: 2.3.4

Type: Bug Priority: Critical
Reporter: Andreas Weder Assignee: Evzen Fochr
Resolution: Fixed Votes: 0
Labels: next, support, usability, ux, uzh
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File Feeds 2 - global config 1.png     PNG File Feeds 2 - global config 2.png     PNG File Feeds 2 - global config 3-1.png     PNG File Feeds 2 - global config 3-2.png     PNG File Feeds 3 - edit agg - override config 2.png    
Issue Links:
causality
caused by MGNLRSSAGG-143 Enable separate scheduling for feeds Closed
relation
is related to MGNLRSSAGG-195 ConcurrentModificationException excep... Closed
is related to MGNLRSSAGG-185 Proposal: Replace crontab setting wit... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

We've removed the way to easily schedule feed import with the recent feature to schedule such imports on a per-feed basis. We know from customers, that users (not admins) are defining such import settings. These users can't be expected to specify their schedule config as a cron string or go to an online tool to develop a string.

I've attached screenshots showing how the easier scheduling can go together with the cron-based scheduling (which - I agree - has some value in its own). Please also revise the help text for the cron field.



 Comments   
Comment by Evzen Fochr [ 26/Jan/15 ]

This issue needs to be postponed to next release due not enough time to test solution.

Comment by Evzen Fochr [ 14/Feb/15 ]

Forgot to add cron field help text.

Comment by Evzen Fochr [ 17/Feb/15 ]

SaveRSSAction class needs fix too

Comment by Jan Haderka [ 20/Feb/15 ]

Shouldn't there be also cleanup task for existing configured jobs that were done using still old fields?

Comment by Federico Grilli [ 24/Feb/15 ]

Trying to import every minute I consistently get this error

015-02-24 10:15:00,995 ERROR info.magnolia.module.scheduler.CommandJob         : Cannot execute command importRss-rss.
java.util.ConcurrentModificationException
	at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:901)
	at java.util.ArrayList$Itr.next(ArrayList.java:851)
	at info.magnolia.module.rssaggregator.importhandler.FastRSSFeedFetcher.fetchAggregateFeeds(FastRSSFeedFetcher.java:98)
	at info.magnolia.module.rssaggregator.command.LaunchSingleRSSCommand.execute(LaunchSingleRSSCommand.java:114)
	at info.magnolia.commands.MgnlCommand.executeSynchronized(MgnlCommand.java:81)
	at info.magnolia.commands.MgnlCommand.execute(MgnlCommand.java:70)
	at info.magnolia.module.scheduler.CommandJob.execute(CommandJob.java:135)
	at org.quartz.core.JobRunShell.run(JobRunShell.java:223)
	at org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:549)

Besides that, this feed http://www.java.net/blog/52359/feed is apparently not valid as I consistently get the error

2015-02-24 10:15:06,585 ERROR gregator.importhandler.DefaultFeedChannelFetchTask: Failed to fetch result for channel 'http://www.java.net/blog/52359/feed' for aggregate 'MagnoliaBlogs' (Thread 217): java.lang.IllegalArgumentException: Invalid document
Comment by Jan Haderka [ 24/Feb/15 ]

Feed is perfectly valid if you do wget --no-check-certificate "http://www.java.net/blog/52359/feed" ... the problem is most likely that it redirects from http to https and we don't like the certificate ... which imo is perfectly valid situation and we should be able to cope with cert.

Comment by Evzen Fochr [ 24/Feb/15 ]

Same error java.util.ConcurrentModificationException is in previous version of rss. Possibly caused by two jobs(but same feed) concurently try to execute at same time. It start after i set "Use different settings for this feed". Creating new rss ticket http://jira.magnolia-cms.com/browse/MGNLRSSAGG-195.

Comment by Milan Divilek [ 24/Feb/15 ]

Reopen:

please remove unnecessary if statements

RSSSwitchableFieldTransformer#readFromItem
        String propertyName = definePropertyName();
        if (relatedFormItem.getItemProperty(propertyName) != null) {
            switchableFieldNewValues.addItemProperty(propertyName, relatedFormItem.getItemProperty(propertyName));
        }
        if (relatedFormItem.getItemProperty(propertyName) != null) {
            switchableFieldNewValues.addItemProperty(FIELD_KEY_DISABLED, relatedFormItem.getItemProperty(FIELD_KEY_DISABLED));
        }
        if (relatedFormItem.getItemProperty(CONFIG_PROPERTY_NAME_CRON) != null) {
            switchableFieldNewValues.addItemProperty(FIELD_KEY_CRONMAKER, new DefaultProperty<PropertysetItem>(PropertysetItem.class, cronStringToCompositeTransform()));
        }
        if (relatedFormItem.getItemProperty(CONFIG_PROPERTY_NAME_CRON) != null) {
            switchableFieldNewValues.addItemProperty(FIELD_KEY_CRONSTRING, relatedFormItem.getItemProperty(CONFIG_PROPERTY_NAME_CRON));
        }
Comment by Mikaël Geljić [ 27/Feb/15 ]

SaveRSSAction no longer sets node name properly

-                setNodeName(node, item);
Comment by Jan Haderka [ 27/Feb/15 ]

Since it came back as a regression, perhaps it deserves unit test to make sure anyone touching that code in the future will not have to wait for ui tests to figure it out?

Comment by Evzen Fochr [ 02/Mar/15 ]

just need to add test

Generated at Mon Feb 12 07:06:12 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.