bamaer commented on code in PR #8270: URL: https://github.com/apache/hop/pull/8270#discussion_r3943975741
########## ui/src/main/java/org/apache/hop/ui/hopgui/notifications/providers/RssNotificationProvider.java: ########## @@ -0,0 +1,503 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.ui.hopgui.notifications.providers; + +import java.io.BufferedInputStream; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import java.util.Locale; +import javax.xml.parsers.DocumentBuilder; +import org.apache.hc.client5.http.classic.methods.HttpGet; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.HttpEntity; +import org.apache.hop.core.exception.HopException; +import org.apache.hop.core.logging.LogChannel; +import org.apache.hop.core.notifications.INotificationProvider; +import org.apache.hop.core.notifications.Notification; +import org.apache.hop.core.notifications.NotificationCategory; +import org.apache.hop.core.notifications.NotificationPriority; +import org.apache.hop.core.xml.XmlParserFactoryProducer; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; + +/** + * RSS/Atom feed notification provider. Supports both RSS 2.0 and Atom 1.0 feeds. Can be configured + * with any feed URL. + */ +public class RssNotificationProvider implements INotificationProvider { + private String feedUrl; + private String providerId; + private String providerName; + private boolean enabled = true; + private long pollInterval = 3600000; // 1 hour default + private String username; + private String password; + + /** What the feed last answered, so a poll that changes nothing costs a 304. */ + private final NotificationHttp.Conditional conditional = new NotificationHttp.Conditional(); + + /** The entries of the last answer, replayed while the feed keeps saying "not modified". */ + private List<Notification> lastFetched = new ArrayList<>(); + + /** + * Create a new RSS notification provider + * + * @param feedUrl The URL of the RSS/Atom feed + * @param providerId Unique identifier for this provider instance + * @param providerName Human-readable name for this provider + */ + public RssNotificationProvider(String feedUrl, String providerId, String providerName) { + this.feedUrl = feedUrl; + this.providerId = providerId; + this.providerName = providerName; + } + + @Override + public String getId() { + return providerId; + } + + @Override + public String getName() { + return providerName; + } + + @Override + public String getDescription() { + return "RSS/Atom feed provider for: " + feedUrl; + } + + @Override + public List<Notification> fetchNotifications() throws HopException { + List<Notification> notifications = new ArrayList<>(); + + if (feedUrl == null || feedUrl.isEmpty()) { + return notifications; + } + + try { + CloseableHttpClient client = NotificationHttp.newClient(username, password); + HttpGet request = new HttpGet(feedUrl); Review Comment: Fixed the two parts that hold up. `NotificationHttp.requestable()` now rejects anything that isn't an absolute http(s) URL naming a host, called before `HttpGet` in both providers — `NotificationLinks` had to become public first, since it sits in `..notifications` while the providers are in `..notifications.providers`. Response bodies are bounded at 8MB and throw rather than truncate, so a half-read feed fails clearly instead of surfacing later as a parse error. Covered by `NotificationHttpTest`. I've left out the loopback/link-local denylist. A Hop Web user who can add an RSS source can already build a pipeline with an HTTP transform, which is a strictly more capable primitive with no denylist on it, so the boundary this would imply doesn't exist elsewhere in the product — and a pre-resolution IP check loses to DNS rebinding anyway. If Hop Web wants egress policy I think it belongs at the platform level rather than in one provider. Happy to be overruled. The credential-leak-on-redirect half is handled by the AuthScope fix below. ########## rap/src/main/java/org/apache/hop/ui/hopgui/notifications/NotificationServiceImpl.java: ########## @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hop.ui.hopgui.notifications; + +import org.apache.hop.ui.hopgui.ISingletonProvider; +import org.eclipse.rap.rwt.RWT; +import org.eclipse.rap.rwt.SingletonUtil; + +/** + * One NotificationService per user session. Hop Web serves many users from one process, and this + * holds state that belongs to one of them. Review Comment: Fixed, and measured. Added `NotificationFetchCache`: process-wide, keyed by source id. Concurrent callers wait on the in-flight fetch instead of starting their own; later callers reuse the answer for half a poll interval. Each caller gets copies — this needed a `Notification` copy constructor, because read state is set on the notification object and sharing instances would leak one user's reads to every other session. Verified on a real two-session Hop Web against a controlled feed, counting requests server-side: | Build | Sessions | Fetches | |---|---|---| | This branch before the fix | 2 | 2 | | With the cache | 2 | 1 | Same container, same feed, same two logins, only the jars swapped. One correction on the diagnosis: the sustained cost isn't there. The providers already make conditional requests (`NotificationHttp.Conditional`, same commit) and GitHub doesn't count 304s against the rate limit, so steady state is free. What's real is the sign-in burst, which is what the cache collapses. Failures are deliberately not cached — I had it that way first and three existing tests failed, correctly: holding a failure keeps a recovered source looking broken to every session. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
