[MGNLRSSAGG-143] Enable separate scheduling for feeds Created: 18/Dec/13 Updated: 06/Jun/14 Resolved: 08/May/14 |
|
| Status: | Closed |
| Project: | Magnolia RSS Aggregator Module |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 2.3 |
| Type: | Improvement | Priority: | Neutral |
| Reporter: | Ondrej Chytil | Assignee: | Ondrej Chytil |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||||||||||||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||||||||||
| 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)
|
||||||||||||||||||||||||||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||||||||||||||||||||||||||
| Description |
|
Right now scheduled feed update can be set only for all feeds together. We should introduce a way to do that for every feed separately. |
| Comments |
| Comment by Ondrej Chytil [ 19/Dec/13 ] |
|
Basic layout |
| Comment by Andreas Weder [ 20/Dec/13 ] |
|
Ondrej, I had a look at your mockups (wasn't too bad, was it? Use global settings or override
Other changes
What do you think? Does this actually meet what you're required to do? Note that not everything would have to be done in one step, but I want to make sure that the reworked designs still matches the principal goal defined by this issue. |
| Comment by Andreas Weder [ 20/Dec/13 ] |
|
BTW, we could also add an issue to rework the form on the first tab. That could see some improvements as well, although the problems caused by the restricted width of the form will only by tackled by a future full-screen/full-width feature planned for forms. But we could improve labels a.o. I also wonder if we shouldn't create yet another issue for renaming "RSS Aggregator" to "RSS aggregator" everywhere. I know, I asked for the change during the last review, but I'm not longer convinced that "RSS Aggregator" is actually a name, and thus it should be written lower-case. |
| Comment by Thomas Comiotto [ 06/Jan/14 ] |
|
Why not just drop the scheduler alltogether and replace it by a global feed cache with configurable expiration settings? Feeds should be imported on demand, e.g. when the corresponding pages are visited! A periodic import is a waste of bandwith and cpu time. Especially for feeds embedded in low/no traffic pages. The UZH website uses over 500 feeds, managed on a per user basis. We can't force users to choose appropriate refresh rates. IĀ fear users will tend to choose low rates irrespective of if the content is time critical or not. And this will bring the system down no matter what configuration level you choose. |
| Comment by Jan Haderka [ 09/Jan/14 ] |
The reason feeds are in Magnolia is to avoid importing them on demand. There were way too many issues in the past with doing this resulting in unpredictable and slow page loading times when feeds were not readily available. Having feeds in Magnolia serves to avoid external feed provider connectivity issues and to remove need for parsing xml while putting together content for rendering feed data (and/or when combining multiple feeds). However if you want to load feeds only on demand you can always do this directly by overriding component model to fetch data from the source instead of from Magnolia |
| Comment by Jan Haderka [ 16/Jan/14 ] |
|
Why is dialog squashed and not using full size of available screen (see attached screenshot)? There's not that many fields so it could not fit the screen. |
| Comment by Jan Haderka [ 21/Jan/14 ] |
Agreed. I also don't see need for that second option ... @Andreas?
Could you please start attaching stack traces into separate files rather then adding them directly in comments? It makes it easier to read the issue change log. Thank you. |
| Comment by Andreas Weder [ 07/Feb/14 ] |
|
I'm re-opening the issue to get some more UI work done. |
| Comment by Jan Haderka [ 21/Mar/14 ] |
|
In order to fulfil your expectations ...
|
| Comment by Ondrej Chytil [ 21/Mar/14 ] |
|
1. Ups, leftover. I use explicit comparing in development. It's more clear.
Change of mind? info.magnolia.module.rssaggregator.importhandler.RSSFeedImportHandler#378 and blame. Removed. |
| Comment by Jan Haderka [ 23/Mar/14 ] |
Nope, more like a slip. If you check issue to which this points ( |
| Comment by Daniel Lipp [ 10/Apr/14 ] |
|
I reopened the ticket: fix was causing test errors on hudson so I reverted it from master. Pls fix locally first. public class FeedGeneratorResolverTest { ... @Before public void setUp() { MockWebContext ctx = new MockWebContext(); MgnlContext.setInstance(ctx); parameters = new Hashtable<String, String[]>(); ModuleDefinition def = new ModuleDefinition(); def.setClassName(RSSAggregator.class.getCanonicalName()); moduleRegistery = new ModuleRegistryImpl(); ComponentsTestUtil.setInstance(ModuleRegistry.class, moduleRegistery); ComponentsTestUtil.setInstance(SystemContext.class, ctx); moduleRegistery.registerModuleDefinition("rssaggregator", def); moduleRegistery.registerModuleInstance("rssaggregator", new RSSAggregator()); resolver = new FeedGeneratorResolver(moduleRegistery,new RSSModuleFeedGenerator()); } @After public void tearDown() { moduleRegistery = null; ComponentsTestUtil.clear(); MgnlContext.setInstance(null); } would fix it. The fact that it doesn't fail on console makes me suspect there's another test that normally runs before this one. And that previous one doesn't do proper teardown... http://jenkins.magnolia-cms.com/job/magnolia-module-rssaggregator/1088/ |
| Comment by Ondrej Chytil [ 10/Apr/14 ] |
|
I saw that Hudson build failed because of test coverage so I started to add some tests (although I made mostly UI changes so I'll not be able to cover everything) but build went fine locally (clover is disabled on default). I didn't see any test failing but will investigate and fix it. |
| Comment by Jan Haderka [ 24/Apr/14 ] |
|
port to 2.2.x is still pending |
| Comment by Milan Divilek [ 28/Apr/14 ] |
|
Reopen: ConcurrentModificationException during runImportOnAllRss. 2014-04-28 13:23:21,070 ERROR fo.magnolia.ui.contentapp.browser.BrowserPresenter: An error occurred while executing action [runImportOnAllRSS] info.magnolia.ui.api.action.ActionExecutionException: java.util.ConcurrentModificationException at info.magnolia.module.rssaggregator.action.RunEveryRSSImportAction.execute(RunEveryRSSImportAction.java:84) at info.magnolia.ui.api.action.AbstractActionExecutor.execute(AbstractActionExecutor.java:78) at info.magnolia.ui.contentapp.browser.BrowserPresenter.executeAction(BrowserPresenter.java:391) at info.magnolia.ui.contentapp.browser.BrowserPresenter.onActionbarItemClicked(BrowserPresenter.java:331) at info.magnolia.ui.actionbar.ActionbarPresenter.onActionbarItemClicked(ActionbarPresenter.java:183) at info.magnolia.ui.vaadin.actionbar.Actionbar$1.onActionTriggered(Actionbar.java:70) at sun.reflect.GeneratedMethodAccessor894.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at com.vaadin.server.ServerRpcManager.applyInvocation(ServerRpcManager.java:168) at com.vaadin.server.ServerRpcManager.applyInvocation(ServerRpcManager.java:118) at com.vaadin.server.communication.ServerRpcHandler.handleBurst(ServerRpcHandler.java:214) at com.vaadin.server.communication.ServerRpcHandler.handleRpc(ServerRpcHandler.java:111) at com.vaadin.server.communication.UidlRequestHandler.synchronizedHandleRequest(UidlRequestHandler.java:91) at com.vaadin.server.SynchronizedRequestHandler.handleRequest(SynchronizedRequestHandler.java:37) at com.vaadin.server.VaadinService.handleRequest(VaadinService.java:1371) at com.vaadin.server.VaadinServlet.service(VaadinServlet.java:238) at info.magnolia.ui.admincentral.AdmincentralVaadinServlet.service(AdmincentralVaadinServlet.java:131) at javax.servlet.http.HttpServlet.service(HttpServlet.java:728) at info.magnolia.cms.filters.ServletDispatchingFilter.doFilter(ServletDispatchingFilter.java:126) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:82) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:82) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:82) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:82) at info.magnolia.cms.filters.CompositeFilter.doFilter(CompositeFilter.java:65) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.filters.VirtualUriFilter.doFilter(VirtualUriFilter.java:68) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.module.cache.executor.Bypass.processCacheRequest(Bypass.java:58) at info.magnolia.module.cache.executor.CompositeExecutor.processCacheRequest(CompositeExecutor.java:66) at info.magnolia.module.cache.filter.CacheFilter.doFilter(CacheFilter.java:153) at info.magnolia.cms.filters.OncePerRequestAbstractMgnlFilter.doFilter(OncePerRequestAbstractMgnlFilter.java:58) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.i18n.I18nContentSupportFilter.doFilter(I18nContentSupportFilter.java:73) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.filters.RangeSupportFilter.doFilter(RangeSupportFilter.java:84) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.security.BaseSecurityFilter.doFilter(BaseSecurityFilter.java:57) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:82) at info.magnolia.cms.security.SecurityCallbackFilter.doFilter(SecurityCallbackFilter.java:83) at info.magnolia.cms.filters.OncePerRequestAbstractMgnlFilter.doFilter(OncePerRequestAbstractMgnlFilter.java:58) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.security.LogoutFilter.doFilter(LogoutFilter.java:93) at info.magnolia.cms.filters.OncePerRequestAbstractMgnlFilter.doFilter(OncePerRequestAbstractMgnlFilter.java:58) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.module.templatingkit.filters.SiteMergeFilter.doFilter(SiteMergeFilter.java:112) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.multisite.filters.MultiSiteFilter.doFilter(MultiSiteFilter.java:106) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.filters.MultiChannelFilter.doFilter(MultiChannelFilter.java:82) at info.magnolia.cms.filters.OncePerRequestAbstractMgnlFilter.doFilter(OncePerRequestAbstractMgnlFilter.java:58) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.module.cache.filter.GZipFilter.doFilter(GZipFilter.java:73) at info.magnolia.cms.filters.OncePerRequestAbstractMgnlFilter.doFilter(OncePerRequestAbstractMgnlFilter.java:58) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:82) at info.magnolia.cms.security.auth.login.LoginFilter.doFilter(LoginFilter.java:104) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.enterprise.registration.RegistrationFilter.doFilter(RegistrationFilter.java:56) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:82) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:82) at info.magnolia.module.devicedetection.filter.DeviceDetectionFilter.doFilter(DeviceDetectionFilter.java:71) at info.magnolia.cms.filters.OncePerRequestAbstractMgnlFilter.doFilter(OncePerRequestAbstractMgnlFilter.java:58) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.filters.ContentTypeFilter.doFilter(ContentTypeFilter.java:103) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.filters.ContextFilter.doFilter(ContextFilter.java:129) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.MgnlFilterChain.doFilter(MgnlFilterChain.java:80) at info.magnolia.cms.filters.CompositeFilter.doFilter(CompositeFilter.java:65) at info.magnolia.cms.filters.AbstractMgnlFilter.doFilter(AbstractMgnlFilter.java:89) at info.magnolia.cms.filters.SafeDestroyMgnlFilterWrapper.doFilter(SafeDestroyMgnlFilterWrapper.java:106) at info.magnolia.cms.filters.MgnlFilterDispatcher.doDispatch(MgnlFilterDispatcher.java:66) at info.magnolia.cms.filters.MgnlMainFilter.doFilter(MgnlMainFilter.java:107) at info.magnolia.cms.filters.MgnlMainFilter.doFilter(MgnlMainFilter.java:93) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:243) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210) at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:222) at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:123) at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:502) at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:171) at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:100) at org.apache.catalina.valves.AccessLogValve.invoke(AccessLogValve.java:953) at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:118) at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:408) at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1041) at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:603) at org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run(JIoEndpoint.java:312) at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:895) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:918) at java.lang.Thread.run(Thread.java:695) Caused by: java.util.ConcurrentModificationException at java.util.Hashtable$Enumerator.next(Hashtable.java:1031) at info.magnolia.module.rssaggregator.action.RunEveryRSSImportAction.execute(RunEveryRSSImportAction.java:73) ... 104 more Redproduce: but if only "data" subnode under "MagnoliaBlogs" feed is removed then import pass successfully. So it seems this is somehow related to order of feeds. |
| Comment by Milan Divilek [ 28/Apr/14 ] |
|
Reopen #2: Removing of feed doesn't unregister import action from scheduler. 2014-04-28 15:37:00,007 ERROR info.magnolia.module.scheduler.CommandJob : Can't execute command rss-importRss java.util.NoSuchElementException at org.apache.jackrabbit.commons.iterator.FilteringNodeIterator.nextNode(FilteringNodeIterator.java:74) at info.magnolia.module.rssaggregator.command.LaunchSingleRSSCommand.getRSSNodeByFeedName(LaunchSingleRSSCommand.java:120) at info.magnolia.module.rssaggregator.command.LaunchSingleRSSCommand.execute(LaunchSingleRSSCommand.java:110) 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:128) at org.quartz.core.JobRunShell.run(JobRunShell.java:223) at org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:549) |
| Comment by Milan Divilek [ 28/Apr/14 ] |
|
Reopen #3: Changing of default configuration has no effect on already registered feeds. |
| Comment by Milan Divilek [ 30/Apr/14 ] |
|
Reopen: 1. info.magnolia.module.rssaggregator.RSSAggregator try{ init(); } catch(Exception e) { throw new RuntimeException(e); } why catch and rethrow exception? 2. Please add tests for last changes. |
| Comment by Ondrej Chytil [ 06/May/14 ] |
|
Re-opened for correcting the node type to observe. |
| Comment by Milan Divilek [ 06/May/14 ] |
|
Reopen: NoSuchElementException after deletion of event. WARN org.apache.jackrabbit.core.observation.ObservationDispatcher 06.05.2014 14:27:40 -- EventConsumer info.magnolia.module.rssaggregator.RSSAggregator$RSSEventListener threw exception java.lang.RuntimeException: java.lang.RuntimeException: java.util.NoSuchElementException at info.magnolia.module.rssaggregator.RSSAggregator$RSSEventListener.onEvent(RSSAggregator.java:337) at org.apache.jackrabbit.core.observation.EventConsumer.consumeEvents(EventConsumer.java:249) at org.apache.jackrabbit.core.observation.ObservationDispatcher.run(ObservationDispatcher.java:161) at java.lang.Thread.run(Thread.java:695) Caused by: java.lang.RuntimeException: java.util.NoSuchElementException at info.magnolia.module.rssaggregator.RSSAggregator$2.exec(RSSAggregator.java:249) at info.magnolia.module.rssaggregator.RSSAggregator$2.exec(RSSAggregator.java:238) at info.magnolia.context.MgnlContext.doInSystemContext(MgnlContext.java:385) at info.magnolia.context.MgnlContext.doInSystemContext(MgnlContext.java:371) at info.magnolia.module.rssaggregator.RSSAggregator.mapRSSJob(RSSAggregator.java:238) at info.magnolia.module.rssaggregator.RSSAggregator$RSSEventListener.onEvent(RSSAggregator.java:335) ... 3 more Caused by: java.util.NoSuchElementException at org.apache.jackrabbit.commons.iterator.FilteringNodeIterator.nextNode(FilteringNodeIterator.java:74) at info.magnolia.module.rssaggregator.RSSAggregator$2.exec(RSSAggregator.java:243) ... 8 more |
| Comment by Milan Divilek [ 06/May/14 ] |
|
Reopen: When change default configuration (doesn't matter if from the rss app or in configuration app) then 2014-05-06 15:33:44,940 ERROR info.magnolia.context.MgnlContext : MgnlContext is not initialized. This could happen if the request does not go through the Magnolia default filters. java.lang.IllegalStateException: MgnlContext is not set for this thread at info.magnolia.context.MgnlContext.getInstance(MgnlContext.java:300) at info.magnolia.context.MgnlContext.getJCRSession(MgnlContext.java:649) at info.magnolia.cms.util.QueryUtil.search(QueryUtil.java:256) at info.magnolia.cms.util.QueryUtil.search(QueryUtil.java:243) at info.magnolia.module.rssaggregator.RSSAggregator.searchRSSNode(RSSAggregator.java:303) at info.magnolia.module.rssaggregator.RSSAggregator.registerObservation(RSSAggregator.java:293) at info.magnolia.module.rssaggregator.RSSAggregator.init(RSSAggregator.java:117) at info.magnolia.module.rssaggregator.RSSAggregator.access$600(RSSAggregator.java:76) at info.magnolia.module.rssaggregator.RSSAggregator$RSSConfigEventListener.onEvent(RSSAggregator.java:348) at org.apache.jackrabbit.core.observation.EventConsumer.consumeEvents(EventConsumer.java:249) at org.apache.jackrabbit.core.observation.ObservationDispatcher.run(ObservationDispatcher.java:161) at java.lang.Thread.run(Thread.java:695) 2014-05-06 15:33:44,941 WARN .jackrabbit.core.observation.ObservationDispatcher: EventConsumer info.magnolia.module.rssaggregator.RSSAggregator$RSSConfigEventListener threw exception java.lang.IllegalStateException: MgnlContext is not set for this thread at info.magnolia.context.MgnlContext.getInstance(MgnlContext.java:300) at info.magnolia.context.MgnlContext.getJCRSession(MgnlContext.java:649) at info.magnolia.cms.util.QueryUtil.search(QueryUtil.java:256) at info.magnolia.cms.util.QueryUtil.search(QueryUtil.java:243) at info.magnolia.module.rssaggregator.RSSAggregator.searchRSSNode(RSSAggregator.java:303) at info.magnolia.module.rssaggregator.RSSAggregator.registerObservation(RSSAggregator.java:293) at info.magnolia.module.rssaggregator.RSSAggregator.init(RSSAggregator.java:117) at info.magnolia.module.rssaggregator.RSSAggregator.access$600(RSSAggregator.java:76) at info.magnolia.module.rssaggregator.RSSAggregator$RSSConfigEventListener.onEvent(RSSAggregator.java:348) at org.apache.jackrabbit.core.observation.EventConsumer.consumeEvents(EventConsumer.java:249) at org.apache.jackrabbit.core.observation.ObservationDispatcher.run(ObservationDispatcher.java:161) at java.lang.Thread.run(Thread.java:695) |