You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hop.apache.org by "bamaer (via GitHub)" <gi...@apache.org> on 2026/09/05 09:47:48 UTC
[PR] Hop Gui notification system #6188 (hop)
bamaer opened a new pull request, #8270:
URL: https://github.com/apache/hop/pull/8270
**Please** add a meaningful description for your change here
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] Run `mvn clean install apache-rat:check` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
- [x] If you have a group of commits related to the same change, please squash your commits into one and force push your branch using `git rebase -i`.
- [x] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable.
To make clear that you license your contribution under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [x] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hop Gui notification system #6188 (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters merged PR #8270:
URL: https://github.com/apache/hop/pull/8270
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hop Gui notification system #6188 (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters commented on PR #8270:
URL: https://github.com/apache/hop/pull/8270#issuecomment-5559791559
Second pass on the review-feedback commits. Ready to merge.
The earlier findings are addressed:
- RSS/GitHub fetches go through `NotificationHttp.requestable()` (absolute http(s) with a host) and an 8MB stream cap. A loopback/private-IP denylist is deliberately not added; the HTTP transform has no such policy either, and a pre-resolution check loses to DNS rebinding.
- Hop Web sign-in burst is collapsed by process-wide `NotificationFetchCache` (in-flight wait, per-session copies, failures not cached).
- RSS credentials are host-scoped; GitHub uses `Authorization: Bearer`; a token-only source no longer fails on a missing username.
- Per-source `daysToGoBack` is applied (0/blank = global).
- Discovered plugin sources no longer get a 60-minute interval written on Save; `toPersist()` drops untouched discovered rows so they do not outlive the plugin.
- Source dialog is a scrolled `GuiCompositeWidgets` form on `NotificationSourceModel`.
- Relative timestamps and source tooltips are in the resource bundle.
- Feed HTML is stripped in `FeedText.plainText()` and ampersands are escaped for SWT mnemonics.
- Unread bell icon is registered as a Hop Web alternate toolbar resource.
One leftover from that pass is already on the branch: `NotificationFetchCache` was keyed only by source id, so a Hop Web session that had not reloaded after an edit could refill the cache and hand the old feed to the session that just saved. `e553f56c4d` keys by source id plus provider description (RSS URL / GitHub repo).
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hop Gui notification system #6188 (hop)
Posted by "bamaer (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hop Gui notification system #6188 (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters commented on PR #8270:
URL: https://github.com/apache/hop/pull/8270#issuecomment-5554555034
Rebased onto current `main` to resolve the merge conflict from recent merges (notably naming-scheme plugin registration in `HopEnvironment`, the Database perspective nav entry, and `.gitignore`).
`getStandardPluginTypes()` now keeps both `NamingSchemeTypePluginType` and `NotificationProviderPluginType`. Force-pushed to `6188` with `--force-with-lease`.
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hop Gui notification system #6188 (hop)
Posted by "bamaer (via GitHub)" <gi...@apache.org>.
bamaer commented on code in PR #8270:
URL: https://github.com/apache/hop/pull/8270#discussion_r3943976560
##########
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);
+ request.addHeader(
+ "Accept", "application/rss+xml, application/atom+xml, application/xml, text/xml");
+ conditional.applyTo(request);
+
+ try (ClassicHttpResponse response = (ClassicHttpResponse) client.execute(request)) {
+ // Check HTTP status code
+ int statusCode = response.getCode();
+ if (statusCode == 304) {
+ // Unchanged since the last poll.
+ return new ArrayList<>(lastFetched);
+ }
+ if (statusCode < 200 || statusCode >= 300) {
+ throw new HopException("The feed at " + feedUrl + " returned HTTP " + statusCode + ".");
+ }
+
+ HttpEntity entity = response.getEntity();
+ if (entity == null) {
+ throw new HopException("The feed at " + feedUrl + " returned an empty response.");
+ }
+
+ try (InputStream rawInputStream = entity.getContent();
+ BufferedInputStream inputStream = new BufferedInputStream(rawInputStream, 8192)) {
+ // Read first few bytes to check for BOM or non-XML content
+ inputStream.mark(1024);
+ byte[] buffer = new byte[1024];
+ int bytesRead = inputStream.read(buffer);
+ inputStream.reset();
+
+ if (bytesRead > 0) {
+ String contentStart =
+ stripByteOrderMark(
+ new String(
+ buffer,
+ 0,
+ Math.min(bytesRead, 100),
+ java.nio.charset.StandardCharsets.UTF_8));
+ // Check if it looks like HTML (common error response)
+ if (contentStart.trim().startsWith("<html")
+ || contentStart.trim().startsWith("<!DOCTYPE html")) {
+ throw new HopException(
+ "The feed at "
+ + feedUrl
+ + " returned an HTML page instead of a feed. Check the URL, or whether a"
+ + " proxy or sign-in page is answering for it.");
+ }
+ // Check if it starts with XML declaration or valid XML tag
+ String trimmed = contentStart.trim();
+ if (!trimmed.startsWith("<?xml")
+ && !trimmed.startsWith("<feed")
+ && !trimmed.startsWith("<rss")
+ && !trimmed.startsWith("<rdf:RDF")) {
+ throw new HopException(
+ "The response from " + feedUrl + " is not an RSS or Atom feed.");
+ }
+ }
+
+ DocumentBuilder builder =
+ XmlParserFactoryProducer.createSecureDocBuilderFactory().newDocumentBuilder();
+ Document document = builder.parse(inputStream);
+
+ // Check if it's Atom or RSS
+ Element root = document.getDocumentElement();
+ if (root == null) {
+ throw new HopException("The feed at " + feedUrl + " is an empty XML document.");
+ }
+ String rootName = root.getNodeName();
+
+ if ("feed".equals(rootName) || rootName.contains("atom")) {
+ // Atom feed
+ notifications.addAll(parseAtomFeed(document));
+ } else if ("rss".equals(rootName) || "rdf:RDF".equals(rootName)) {
+ // RSS feed
+ notifications.addAll(parseRssFeed(document));
+ } else {
+ throw new HopException(
+ "The feed at " + feedUrl + " is in an unsupported format: <" + rootName + ">.");
+ }
+ }
+
+ // Only now that the feed has been read and understood. Remembering the validator any
+ // earlier means a parse that fails still arms the next request's If-None-Match: the feed
+ // would answer 304 forever, this method would return the entries it never managed to read
+ // (none), and because that is not a failure the error banner would clear itself.
+ conditional.remember(response);
+ }
+ lastFetched = new ArrayList<>(notifications);
+ } catch (HopException e) {
+ throw e;
+ } catch (Exception e) {
+ // Reported to the user through the panel's error banner. NotificationService catches this
+ // per provider, so one unreachable feed does not stop the others.
+ throw new HopException("Could not read the feed at " + feedUrl + ": " + e.getMessage(), e);
+ }
+
+ return notifications;
+ }
+
+ /**
+ * Drop a leading byte order mark.
+ *
+ * <p>Feeds written by Windows tooling are commonly served with one. Decoded from UTF-8 it is
+ * U+FEFF, which {@link String#trim()} leaves alone - it only strips characters up to U+0020 - so
+ * the sniff below would find the feed starting with an invisible character rather than with
+ * {@code <?xml}, and reject a feed the XML parser handles perfectly well.
+ *
+ * @param text The decoded start of the response
+ * @return The text without its byte order mark
+ */
+ static String stripByteOrderMark(String text) {
+ if (!text.isEmpty() && text.charAt(0) == '\uFEFF') {
+ return text.substring(1);
+ }
+ return text;
+ }
+
+ /** Parse Atom 1.0 feed */
+ List<Notification> parseAtomFeed(Document document) {
+ List<Notification> notifications = new ArrayList<>();
+ NodeList entries = document.getElementsByTagName("entry");
+
+ for (int i = 0; i < entries.getLength(); i++) {
+ Element entry = (Element) entries.item(i);
+ try {
+ String id = getElementText(entry, "id");
+ String title = getElementText(entry, "title");
+ String summary = getElementText(entry, "summary");
+ if (summary == null || summary.isEmpty()) {
+ summary = getElementText(entry, "content");
+ }
+ String link = getElementLink(entry);
+ String publishedText = getElementText(entry, "published");
+ if (publishedText == null || publishedText.isEmpty()) {
+ publishedText = getElementText(entry, "updated");
+ }
+ Date published = parseAtomDate(publishedText);
+ if (published == null) {
+ published = new Date();
+ }
+
+ String localId = entryId(id, link, title, publishedText);
+ if (localId == null) {
+ LogChannel.UI.logDetailed(
+ "Skipping an entry in the feed at " + feedUrl + " that cannot be identified");
+ continue;
+ }
+
+ Notification notification =
+ new Notification(
+ localId,
+ title != null ? title : "Untitled",
+ summary != null ? summary : "",
Review Comment:
Added `FeedText.plainText()` — strips tags, drops `<script>`/`<style>` content, decodes named/decimal/hex entities including double-escaped HTML, collapses whitespace. Ampersands are doubled at `setText` time in the panel.
Three things worth recording from doing it. The stripping has to happen in the provider, not at display: the message is truncated to 250 characters first, so stripping afterwards would cut an entry opening with a long `<div class="…">` down to markup and then to nothing. `CLabel` needed the ampersand escaping too — it paints with `SWT.DRAW_MNEMONIC` on the desktop and RAP renders it through a `CLabelLCA` that calls `MnemonicUtil`, so the notification title was losing ampersands on both platforms until it was escaped as well. And testing against LWN's feed found a bug in the fix itself: ` ` decodes to U+00A0, which Java's `\s` doesn't match, so numeric non-breaking spaces survived collapsing where the named ` ` didn't.
Verified in the desktop GUI and in Hop Web against a feed with ampersands in both titles and bodies.
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hop Gui notification system #6188 (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters commented on PR #8270:
URL: https://github.com/apache/hop/pull/8270#issuecomment-5559445847
Follow-up pass on the review fixes: the previous findings are addressed.
One leftover in `NotificationFetchCache`: it was keyed only by source id, so in Hop Web a session that had not reloaded after an edit could refill the process-wide cache and hand the old feed to the session that just saved. Pushed `e553f56c4d` onto this branch — the key is now source id plus the provider description (RSS URL / GitHub repo), with tests for that case.
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hop Gui notification system #6188 (hop)
Posted by "bamaer (via GitHub)" <gi...@apache.org>.
bamaer commented on code in PR #8270:
URL: https://github.com/apache/hop/pull/8270#discussion_r3943976156
##########
ui/src/main/java/org/apache/hop/ui/hopgui/notifications/providers/NotificationHttp.java:
##########
@@ -0,0 +1,135 @@
+/*
+ * 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 org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
+import org.apache.hop.core.util.HttpClientManager;
+import org.apache.hop.core.variables.Variables;
+
+/** Shared HTTP setup for the notification providers. */
+final class NotificationHttp {
+
+ /** Give up if the remote host has not accepted the connection by then. */
+ static final int CONNECT_TIMEOUT_MS = 10000;
+
+ /** Give up if the remote host has not answered by then. */
+ static final int RESPONSE_TIMEOUT_MS = 20000;
+
+ private NotificationHttp() {
+ // Utility class
+ }
+
+ /**
+ * A client with timeouts. {@link HttpClientManager#createDefaultClient()} sets none at all, and
+ * HttpClient 5 waits indefinitely for a response, so a single unresponsive feed would otherwise
+ * hold a polling thread forever.
+ *
+ * <p>The client is built on the process-wide shared connection manager, so do not close it:
+ * closing the response is what returns this request's connection to the pool.
+ *
+ * @return A client configured for polling a notification source
+ */
+ static CloseableHttpClient newClient() {
+ return newClient(null, null);
+ }
+
+ /**
+ * A client with timeouts, authenticating when credentials are given.
+ *
+ * @param username The user name, may be null or empty for anonymous access
+ * @param password The password or token, may be null or empty for anonymous access
+ * @return A client configured for polling a notification source
+ */
+ static CloseableHttpClient newClient(String username, String password) {
+ HttpClientManager.HttpClientBuilderFacade builder =
+ HttpClientManager.getInstance()
+ .createBuilder()
+ .setConnectionTimeout(CONNECT_TIMEOUT_MS)
+ .setSocketTimeout(RESPONSE_TIMEOUT_MS);
+ String resolvedPassword = resolve(password);
+ if (resolvedPassword != null && !resolvedPassword.isEmpty()) {
+ // A token is often all a source wants; GitHub, for one, ignores the user name entirely.
+ builder.setCredentials(resolve(username), resolvedPassword);
Review Comment:
The `AuthScope` point is right and it's fixed. Credentials are scoped to the request host using the 3-arg `setCredentials` and `createHttpHost(URI)` that already existed on `HttpClientManager` — the 2-arg convenience overload was simply the wrong one to have called. GitHub now skips Basic entirely and sends `Authorization: Bearer`, which is what PATs want; confirmed against the live API that an invalid token under `Bearer` returns 401 "Bad credentials", i.e. the scheme is parsed and the token rejected.
Your reachability argument doesn't hold as written, but it does hold against this branch. The old dialog stored `wUsername.getText().trim()`, so an empty field was `""`, not null, and `BasicUserPrincipal("")` is fine — the NPE only triggered on a hand-edited `hop-config.json` with no `auth.username` key. The dialog rewrite in this branch changes that: `NotificationSourceModel.toConfig` writes `trimmed(username)`, which is null for a field nobody touched, so the path becomes reachable from the UI. Coerced either way. One detail: it's an `IllegalArgumentException` at client construction rather than an NPE on fetch.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/notifications/NotificationPanel.java:
##########
@@ -0,0 +1,1002 @@
+/*
+ * 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 java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.List;
+import org.apache.hop.core.logging.LogChannel;
+import org.apache.hop.core.notifications.Notification;
+import org.apache.hop.i18n.BaseMessages;
+import org.apache.hop.ui.core.PropsUi;
+import org.apache.hop.ui.core.gui.GuiResource;
+import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.hopgui.ISingletonProvider;
+import org.apache.hop.ui.hopgui.ImplementationLoader;
+import org.apache.hop.ui.hopgui.perspective.configuration.ConfigurationPerspective;
+import org.apache.hop.ui.util.EnvironmentUtils;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.SWTException;
+import org.eclipse.swt.custom.CLabel;
+import org.eclipse.swt.custom.ScrolledComposite;
+import org.eclipse.swt.events.SelectionAdapter;
+import org.eclipse.swt.events.SelectionEvent;
+import org.eclipse.swt.graphics.Color;
+import org.eclipse.swt.graphics.Point;
+import org.eclipse.swt.layout.FormAttachment;
+import org.eclipse.swt.layout.FormData;
+import org.eclipse.swt.layout.FormLayout;
+import org.eclipse.swt.widgets.Button;
+import org.eclipse.swt.widgets.Canvas;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Label;
+import org.eclipse.swt.widgets.Listener;
+import org.eclipse.swt.widgets.Shell;
+
+/** Dropdown panel for displaying notifications */
+public class NotificationPanel implements INotificationListener {
+ private static final Class<?> PKG = NotificationPanel.class;
+
+ /** How many notifications the panel draws before it stops and says how many are left. */
+ private static final int MAX_RENDERED_NOTIFICATIONS = 100;
+
+ private static NotificationPanel fallback;
+
+ private static final ISingletonProvider PROVIDER = loadProvider();
+
+ private static ISingletonProvider loadProvider() {
+ try {
+ return (ISingletonProvider) ImplementationLoader.newInstance(NotificationPanel.class);
+ } catch (Throwable e) {
+ // hop-ui unit tests have no rcp/rap *Impl on the classpath. Anywhere else this is a
+ // misconfiguration worth shouting about: one instance would then be shared by every Hop Web
+ // session, which is the very thing the per-session provider exists to prevent.
+ LogChannel.GENERAL.logBasic(
+ "No NotificationPanelImpl found; falling back to a single instance for this process. "
+ + "In Hop Web that means every session shares one.");
+ return () -> {
+ synchronized (NotificationPanel.class) {
+ if (fallback == null) {
+ fallback = new NotificationPanel();
+ }
+ return fallback;
+ }
+ };
+ }
+ }
+
+ private Shell shell;
+ private Shell parentShell;
+ private ScrolledComposite scrolledComposite;
+ private Composite contentComposite;
+ private boolean isVisible = false;
+
+ /** The configured sources, refreshed each time the list is drawn, for the source colours. */
+ private java.util.List<org.apache.hop.ui.hopgui.notifications.config.NotificationSourceConfig>
+ sourcesForRender = new java.util.ArrayList<>();
+
+ /** Use {@link #getInstance()}. Public so RWT can create one per user session in Hop Web. */
+ public NotificationPanel() {
+ this.parentShell = HopGui.getInstance().getShell();
+ NotificationService.getInstance().addNotificationListener(this);
+ }
+
+ /**
+ * @return The notification panel of this process, or of this user's session in Hop Web
+ */
+ public static NotificationPanel getInstance() {
+ return (NotificationPanel) PROVIDER.getInstanceInternal();
+ }
+
+ /** Toggle the panel visibility */
+ public void toggle() {
+ if (isVisible) {
+ hide();
+ } else {
+ show();
+ }
+ }
+
+ /** Show the notification panel */
+ public void show() {
+ if (shell != null && !shell.isDisposed()) {
+ // Panel already exists, refresh notifications and show
+ updateNotifications();
+ shell.setVisible(true);
+ shell.setFocus();
+ // Without this the panel stays "not visible" after the first open and close, which stops
+ // notificationsChanged() from refreshing it and leaves the bell unable to close it again.
+ isVisible = true;
+ return;
+ }
+
+ createPanel();
+ updateNotifications();
+ positionPanel();
+ shell.setVisible(true);
+ isVisible = true;
+ }
+
+ /** Hide the notification panel */
+ public void hide() {
+ if (shell != null && !shell.isDisposed()) {
+ shell.setVisible(false);
+ }
+ isVisible = false;
+ }
+
+ /** Create the panel UI */
+ private void createPanel() {
+ // Use DIALOG_TRIM instead of ON_TOP to keep it attached to parent
+ // Remove ON_TOP so it doesn't stay on top when switching applications
+ shell = new Shell(parentShell, SWT.DIALOG_TRIM | SWT.RESIZE);
+ shell.setLayout(new FormLayout());
+ PropsUi.setLook(shell);
+
+ // Header
+ Composite header = new Composite(shell, SWT.NONE);
+ header.setLayout(new FormLayout());
+ PropsUi.setLook(header);
+ FormData fdHeader = new FormData();
+ fdHeader.left = new FormAttachment(0, 0);
+ fdHeader.right = new FormAttachment(100, 0);
+ fdHeader.top = new FormAttachment(0, 0);
+ header.setLayoutData(fdHeader);
+
+ // Settings button
+ Button settingsButton = new Button(header, SWT.PUSH);
+ settingsButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Settings"));
+ settingsButton.setToolTipText(
+ BaseMessages.getString(PKG, "NotificationPanel.Settings.Tooltip"));
+ PropsUi.setLook(settingsButton);
+ FormData fdSettings = new FormData();
+ fdSettings.right = new FormAttachment(100, -10);
+ fdSettings.top = new FormAttachment(0, 5);
+ fdSettings.bottom = new FormAttachment(100, -5);
+ settingsButton.setLayoutData(fdSettings);
+ settingsButton.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ org.apache.hop.ui.hopgui.perspective.configuration.ConfigurationPerspective
+ configPerspective = HopGui.getConfigurationPerspective();
+ if (configPerspective != null) {
+ HopGui.getInstance().setActivePerspective(configPerspective);
+
+ // Defer tab/tree selection until perspective is fully activated
+ Display.getCurrent()
+ .asyncExec(
+ () -> {
+ ConfigurationPerspective perspective = HopGui.getConfigurationPerspective();
+ if (perspective != null) {
+ perspective.showNotificationsTab();
+ }
+ });
+ }
+ }
+ });
+
+ Button clearAll = new Button(header, SWT.PUSH);
+ clearAll.setText(BaseMessages.getString(PKG, "NotificationPanel.ClearAll"));
+ clearAll.setToolTipText(BaseMessages.getString(PKG, "NotificationPanel.ClearAll.Tooltip"));
+ PropsUi.setLook(clearAll);
+ FormData fdClearAll = new FormData();
+ fdClearAll.right = new FormAttachment(settingsButton, -10);
+ fdClearAll.top = new FormAttachment(0, 5);
+ fdClearAll.bottom = new FormAttachment(100, -5);
+ clearAll.setLayoutData(fdClearAll);
+ clearAll.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ NotificationService.getInstance().clearAll();
+ updateNotifications();
+ }
+ });
+
+ Button markAllRead = new Button(header, SWT.PUSH);
+ markAllRead.setText(BaseMessages.getString(PKG, "NotificationPanel.MarkAllRead"));
+ PropsUi.setLook(markAllRead);
+ FormData fdMarkAll = new FormData();
+ fdMarkAll.right = new FormAttachment(clearAll, -10);
+ fdMarkAll.top = new FormAttachment(0, 5);
+ fdMarkAll.bottom = new FormAttachment(100, -5);
+ markAllRead.setLayoutData(fdMarkAll);
+ markAllRead.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ NotificationService.getInstance().markAllAsRead();
+ updateNotifications();
+ }
+ });
+
+ // Scrolled content area
+ scrolledComposite = new ScrolledComposite(shell, SWT.V_SCROLL | SWT.BORDER);
+ PropsUi.setLook(scrolledComposite);
+ FormData fdScrolled = new FormData();
+ fdScrolled.left = new FormAttachment(0, 0);
+ fdScrolled.right = new FormAttachment(100, 0);
+ fdScrolled.top = new FormAttachment(header, 0);
+ fdScrolled.bottom = new FormAttachment(100, -40);
+ scrolledComposite.setLayoutData(fdScrolled);
+
+ contentComposite = new Composite(scrolledComposite, SWT.NONE);
+ contentComposite.setLayout(new FormLayout());
+ PropsUi.setLook(contentComposite);
+ scrolledComposite.setContent(contentComposite);
+ scrolledComposite.setExpandHorizontal(true);
+ scrolledComposite.setExpandVertical(true);
+
+ // Footer
+ Composite footer = new Composite(shell, SWT.NONE);
+ footer.setLayout(new FormLayout());
+ PropsUi.setLook(footer);
+ FormData fdFooter = new FormData();
+ fdFooter.left = new FormAttachment(0, 0);
+ fdFooter.right = new FormAttachment(100, 0);
+ fdFooter.bottom = new FormAttachment(100, 0);
+ footer.setLayoutData(fdFooter);
+
+ Button closeButton = new Button(footer, SWT.PUSH);
+ closeButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Close"));
+ PropsUi.setLook(closeButton);
+ FormData fdClose = new FormData();
+ fdClose.right = new FormAttachment(100, -10);
+ fdClose.top = new FormAttachment(0, 5);
+ fdClose.bottom = new FormAttachment(100, -5);
+ closeButton.setLayoutData(fdClose);
+ closeButton.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ hide();
+ }
+ });
+
+ // Close when clicking outside
+ shell.addListener(
+ SWT.Deactivate,
+ e -> {
+ if (!shell.isDisposed()) {
+ Display.getCurrent()
+ .asyncExec(
+ () -> {
+ if (!shell.isDisposed() && !shell.isFocusControl()) {
+ hide();
+ }
+ });
+ }
+ });
+
+ // The panel hangs off the bell in the main toolbar, so it has to follow the main window
+ // whenever that moves or is resized, not just when it is resized.
+ if (parentShell != null && !parentShell.isDisposed()) {
+ Listener repositionListener =
+ e -> {
+ if (shell != null && !shell.isDisposed() && isVisible) {
+ Display.getCurrent()
+ .asyncExec(
+ () -> {
+ if (shell != null && !shell.isDisposed() && isVisible) {
+ positionPanel();
+ }
+ });
+ }
+ };
+ parentShell.addListener(SWT.Resize, repositionListener);
+ parentShell.addListener(SWT.Move, repositionListener);
+ }
+
+ shell.setSize(400, 500);
+
+ // Add listener to shell resize to ensure titles truncate properly
+ shell.addListener(
+ SWT.Resize,
+ e -> {
+ if (contentComposite != null && !contentComposite.isDisposed()) {
+ // Force layout update to ensure titles truncate correctly
+ contentComposite.layout(true, true);
+ }
+ });
+ }
+
+ /** Update the notifications display */
+ private void updateNotifications() {
+ if (contentComposite == null || contentComposite.isDisposed()) {
+ return;
+ }
+
+ // Clear existing notifications
+ for (Control control : contentComposite.getChildren()) {
+ control.dispose();
+ }
+
+ // Get configuration options
+ org.apache.hop.core.config.HopConfig hopConfig =
+ org.apache.hop.core.config.HopConfig.getInstance();
+ boolean showReadNotifications =
+ org.apache.hop.core.config.HopConfig.readOptionString(
+ "notification.showReadNotifications", "true")
+ .equalsIgnoreCase("true");
+ String daysToGoBackStr =
+ org.apache.hop.core.config.HopConfig.readOptionString(
+ "notification.global.daysToGoBack", "30");
+ int daysToGoBack = 0;
+ try {
+ daysToGoBack = Integer.parseInt(daysToGoBackStr);
+ } catch (NumberFormatException e) {
+ daysToGoBack = 30; // Default to 30 days
+ }
+
+ // Get provider errors and notifications
+ List<org.apache.hop.ui.hopgui.notifications.ProviderErrorInfo> providerErrors =
+ NotificationService.getInstance().getProviderErrors();
+ List<Notification> notifications =
+ NotificationService.getInstance().getNotifications(!showReadNotifications, daysToGoBack);
Review Comment:
Confirmed and wired through. `NotificationService` builds a source id → window map when the configuration loads and resolves per notification, with 0 and blank meaning "use the global setting", as the dialog label promises. Read once per session rather than per refresh, since drawing the panel asks for the list once per notification on screen. Four tests in `NotificationServiceTest`.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/notifications/NotificationProviderPlugins.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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 java.util.ArrayList;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.hop.core.logging.ILogChannel;
+import org.apache.hop.core.notifications.INotificationProvider;
+import org.apache.hop.core.notifications.NotificationProviderPluginType;
+import org.apache.hop.core.plugins.IPlugin;
+import org.apache.hop.core.plugins.PluginRegistry;
+import org.apache.hop.ui.hopgui.notifications.config.NotificationSourceConfig;
+
+/**
+ * The notification providers contributed by plugins, as found in the plugin registry.
+ *
+ * <p>A plugin declares a provider with {@link
+ * org.apache.hop.core.notifications.NotificationProviderPlugin} and is discovered from its jar. It
+ * therefore needs no entry in the configuration to work: a stored source only records what the user
+ * changed about it, and a provider whose plugin has been uninstalled simply stops being found.
+ */
+public final class NotificationProviderPlugins {
+
+ private NotificationProviderPlugins() {
+ // Utility class
+ }
+
+ /**
+ * @return The registry entries of every declared notification provider
+ */
+ public static List<IPlugin> plugins() {
+ List<IPlugin> plugins =
+ PluginRegistry.getInstance().getPlugins(NotificationProviderPluginType.class);
+ return plugins == null ? new ArrayList<>() : plugins;
+ }
+
+ /**
+ * @return The identifiers of every declared notification provider
+ */
+ public static Set<String> ids() {
+ Set<String> ids = new LinkedHashSet<>();
+ for (IPlugin plugin : plugins()) {
+ String id = idOf(plugin);
+ if (id != null) {
+ ids.add(id);
+ }
+ }
+ return ids;
+ }
+
+ /**
+ * Instantiate a declared provider.
+ *
+ * @param plugin The registry entry
+ * @param log Where to report a provider that cannot be loaded
+ * @return The provider, or null when its class could not be loaded
+ */
+ public static INotificationProvider load(IPlugin plugin, ILogChannel log) {
+ try {
+ return PluginRegistry.getInstance().loadClass(plugin, INotificationProvider.class);
+ } catch (Exception e) {
+ log.logError("Unable to load the notification provider of plugin " + plugin.getName(), e);
+ return null;
+ }
+ }
+
+ /**
+ * The identifier a plugin's provider is known by. This is the source id its notifications are
+ * qualified with, so it has to be the plugin id and nothing derived from the instance.
+ *
+ * @param plugin The registry entry
+ * @return The identifier, or null when the entry declares none
+ */
+ public static String idOf(IPlugin plugin) {
+ String[] ids = plugin.getIds();
+ return ids == null || ids.length == 0 ? null : ids[0];
+ }
+
+ /**
+ * Describe the declared providers as configuration sources, so the Notifications settings can
+ * list a plugin that has never been configured alongside the sources the user added.
+ *
+ * @return One source per declared provider, in registry order
+ */
+ public static List<NotificationSourceConfig> describeAsSources() {
+ List<NotificationSourceConfig> described = new ArrayList<>();
+ for (IPlugin plugin : plugins()) {
+ String id = idOf(plugin);
+ if (id == null) {
+ continue;
+ }
+ NotificationSourceConfig source = new NotificationSourceConfig();
+ source.setId(id);
+ source.setPluginId(id);
+ source.setName(
+ plugin.getName() == null || plugin.getName().isEmpty() ? id : plugin.getName());
+ source.setType(NotificationSourceConfig.SourceType.CUSTOM_PLUGIN);
+ source.setEnabled(true);
+ source.setPollIntervalMinutes("60");
Review Comment:
Fixed as suggested — `describeAsSources()` no longer writes an interval, so `applyPollInterval`'s empty check leaves the provider's own default alone and the marketplace one keeps its six hours. The settings table now shows the running provider's actual interval instead of falling back to the global, which was misleading.
Pulling on this surfaced something larger: `loadSources()` calls `addDiscovered()` and *any* save writes the whole list, so pressing Save baked every discovered plugin into `hop-config.json` permanently. That contradicts the design stated in `NotificationProviderPlugins`' own javadoc — "a provider whose plugin has been uninstalled simply stops being found" — because once persisted it doesn't. Added `toPersist()`, which drops discovered rows still identical to their synthesized form.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/notifications/config/NotificationSourceDialog.java:
##########
@@ -0,0 +1,976 @@
+/*
+ * 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.config;
+
+import java.util.UUID;
+import org.apache.hop.core.util.Utils;
+import org.apache.hop.i18n.BaseMessages;
+import org.apache.hop.ui.core.PropsUi;
+import org.apache.hop.ui.core.dialog.ErrorDialog;
+import org.apache.hop.ui.core.widget.PasswordTextVar;
+import org.apache.hop.ui.core.widget.TextVar;
+import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.pipeline.transform.BaseTransformDialog;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.events.SelectionAdapter;
+import org.eclipse.swt.events.SelectionEvent;
+import org.eclipse.swt.layout.FormAttachment;
+import org.eclipse.swt.layout.FormData;
+import org.eclipse.swt.layout.FormLayout;
+import org.eclipse.swt.widgets.Button;
+import org.eclipse.swt.widgets.ColorDialog;
+import org.eclipse.swt.widgets.Combo;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Label;
+import org.eclipse.swt.widgets.Shell;
+
+/** Dialog for adding or editing a notification source configuration. */
+public class NotificationSourceDialog {
+
+ private static final Class<?> PKG = NotificationSourceDialog.class;
+
+ private Shell shell;
+ private Shell parentShell;
+ private NotificationSourceConfig sourceConfig;
+
+ /**
+ * Whether the dialog was closed without confirming. Only OK clears it: closing the window any
+ * other way - the title bar, Escape - leaves the edits unsaved, which is what closing a dialog
+ * means everywhere else.
+ */
+ private boolean cancelled = true;
+
+ private PropsUi props = PropsUi.getInstance();
+
+ // UI widgets
+ private TextVar wName;
+ private Combo wType;
+ private Button wEnabled;
+ private Button wColorButton;
+ private Label wColorPreview;
+ private Composite wTypeSpecificComposite;
+ private TextVar wGithubUrl; // For URL input
+ private TextVar wGithubOwner;
+ private TextVar wGithubRepo;
+ private Button wGithubIncludePrereleases;
+ private TextVar wRssUrl;
+ private TextVar wPluginId;
+ private TextVar wPollInterval;
+ private TextVar wDaysToGoBack;
+ private TextVar wUsername;
+ private PasswordTextVar wPassword;
+ private TextVar wMinimumVersion;
+
+ /** Set while one GitHub field is updating another, so the two directions do not loop. */
+ private boolean syncingGithubFields;
+
+ public NotificationSourceDialog(Shell parent, NotificationSourceConfig sourceConfig) {
+ this.parentShell = parent;
+ this.shell = new Shell(parent, SWT.DIALOG_TRIM | SWT.RESIZE | SWT.MAX | SWT.MIN);
+ this.sourceConfig = sourceConfig != null ? sourceConfig : new NotificationSourceConfig();
+ props.setLook(this.shell);
+ }
+
+ public String open() {
+ Display display = parentShell.getDisplay();
+
+ shell.setText(BaseMessages.getString(PKG, "NotificationSourceDialog.Title"));
+
+ FormLayout formLayout = new FormLayout();
Review Comment:
Done. `NotificationSourceDialog` is 976 → 372 lines with no `FormAttachment` left in it. The form moved to a new `NotificationSourceModel` carrying 16 `@GuiWidgetElement` fields, built through `GuiCompositeWidgets.addScrolledComposite`, so the form scrolls and the button bar stays pinned. The colour picker and URL parser are annotated `BUTTON` methods — the framework passes the model in and re-binds afterwards, so no manual refresh plumbing.
One deviation: type-specific fields aren't extra groups. Groups render as tabs (`GuiWidgetGroups` logs "not implemented yet" for LIST/BOXES), which would put a GitHub, an RSS and a Plugin tab on every source regardless of type. Instead every field is declared once and `setWidgetsHidden` collapses the irrelevant rows when the type changes — same visible behaviour as before, without the dispose-and-rebuild. Happy to switch to tabs if you'd prefer.
Two behaviour fixes fell out: saving a GitHub source now accepts a URL alone instead of erroring, and `toConfig` writes only the chosen type's fields, so switching a source to RSS and back no longer leaves a stale owner/repo behind. Tested on the desktop and in Hop Web.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/notifications/NotificationPanel.java:
##########
@@ -0,0 +1,1002 @@
+/*
+ * 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 java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.List;
+import org.apache.hop.core.logging.LogChannel;
+import org.apache.hop.core.notifications.Notification;
+import org.apache.hop.i18n.BaseMessages;
+import org.apache.hop.ui.core.PropsUi;
+import org.apache.hop.ui.core.gui.GuiResource;
+import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.hopgui.ISingletonProvider;
+import org.apache.hop.ui.hopgui.ImplementationLoader;
+import org.apache.hop.ui.hopgui.perspective.configuration.ConfigurationPerspective;
+import org.apache.hop.ui.util.EnvironmentUtils;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.SWTException;
+import org.eclipse.swt.custom.CLabel;
+import org.eclipse.swt.custom.ScrolledComposite;
+import org.eclipse.swt.events.SelectionAdapter;
+import org.eclipse.swt.events.SelectionEvent;
+import org.eclipse.swt.graphics.Color;
+import org.eclipse.swt.graphics.Point;
+import org.eclipse.swt.layout.FormAttachment;
+import org.eclipse.swt.layout.FormData;
+import org.eclipse.swt.layout.FormLayout;
+import org.eclipse.swt.widgets.Button;
+import org.eclipse.swt.widgets.Canvas;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Label;
+import org.eclipse.swt.widgets.Listener;
+import org.eclipse.swt.widgets.Shell;
+
+/** Dropdown panel for displaying notifications */
+public class NotificationPanel implements INotificationListener {
+ private static final Class<?> PKG = NotificationPanel.class;
+
+ /** How many notifications the panel draws before it stops and says how many are left. */
+ private static final int MAX_RENDERED_NOTIFICATIONS = 100;
+
+ private static NotificationPanel fallback;
+
+ private static final ISingletonProvider PROVIDER = loadProvider();
+
+ private static ISingletonProvider loadProvider() {
+ try {
+ return (ISingletonProvider) ImplementationLoader.newInstance(NotificationPanel.class);
+ } catch (Throwable e) {
+ // hop-ui unit tests have no rcp/rap *Impl on the classpath. Anywhere else this is a
+ // misconfiguration worth shouting about: one instance would then be shared by every Hop Web
+ // session, which is the very thing the per-session provider exists to prevent.
+ LogChannel.GENERAL.logBasic(
+ "No NotificationPanelImpl found; falling back to a single instance for this process. "
+ + "In Hop Web that means every session shares one.");
+ return () -> {
+ synchronized (NotificationPanel.class) {
+ if (fallback == null) {
+ fallback = new NotificationPanel();
+ }
+ return fallback;
+ }
+ };
+ }
+ }
+
+ private Shell shell;
+ private Shell parentShell;
+ private ScrolledComposite scrolledComposite;
+ private Composite contentComposite;
+ private boolean isVisible = false;
+
+ /** The configured sources, refreshed each time the list is drawn, for the source colours. */
+ private java.util.List<org.apache.hop.ui.hopgui.notifications.config.NotificationSourceConfig>
+ sourcesForRender = new java.util.ArrayList<>();
+
+ /** Use {@link #getInstance()}. Public so RWT can create one per user session in Hop Web. */
+ public NotificationPanel() {
+ this.parentShell = HopGui.getInstance().getShell();
+ NotificationService.getInstance().addNotificationListener(this);
+ }
+
+ /**
+ * @return The notification panel of this process, or of this user's session in Hop Web
+ */
+ public static NotificationPanel getInstance() {
+ return (NotificationPanel) PROVIDER.getInstanceInternal();
+ }
+
+ /** Toggle the panel visibility */
+ public void toggle() {
+ if (isVisible) {
+ hide();
+ } else {
+ show();
+ }
+ }
+
+ /** Show the notification panel */
+ public void show() {
+ if (shell != null && !shell.isDisposed()) {
+ // Panel already exists, refresh notifications and show
+ updateNotifications();
+ shell.setVisible(true);
+ shell.setFocus();
+ // Without this the panel stays "not visible" after the first open and close, which stops
+ // notificationsChanged() from refreshing it and leaves the bell unable to close it again.
+ isVisible = true;
+ return;
+ }
+
+ createPanel();
+ updateNotifications();
+ positionPanel();
+ shell.setVisible(true);
+ isVisible = true;
+ }
+
+ /** Hide the notification panel */
+ public void hide() {
+ if (shell != null && !shell.isDisposed()) {
+ shell.setVisible(false);
+ }
+ isVisible = false;
+ }
+
+ /** Create the panel UI */
+ private void createPanel() {
+ // Use DIALOG_TRIM instead of ON_TOP to keep it attached to parent
+ // Remove ON_TOP so it doesn't stay on top when switching applications
+ shell = new Shell(parentShell, SWT.DIALOG_TRIM | SWT.RESIZE);
+ shell.setLayout(new FormLayout());
+ PropsUi.setLook(shell);
+
+ // Header
+ Composite header = new Composite(shell, SWT.NONE);
+ header.setLayout(new FormLayout());
+ PropsUi.setLook(header);
+ FormData fdHeader = new FormData();
+ fdHeader.left = new FormAttachment(0, 0);
+ fdHeader.right = new FormAttachment(100, 0);
+ fdHeader.top = new FormAttachment(0, 0);
+ header.setLayoutData(fdHeader);
+
+ // Settings button
+ Button settingsButton = new Button(header, SWT.PUSH);
+ settingsButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Settings"));
+ settingsButton.setToolTipText(
+ BaseMessages.getString(PKG, "NotificationPanel.Settings.Tooltip"));
+ PropsUi.setLook(settingsButton);
+ FormData fdSettings = new FormData();
+ fdSettings.right = new FormAttachment(100, -10);
+ fdSettings.top = new FormAttachment(0, 5);
+ fdSettings.bottom = new FormAttachment(100, -5);
+ settingsButton.setLayoutData(fdSettings);
+ settingsButton.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ org.apache.hop.ui.hopgui.perspective.configuration.ConfigurationPerspective
+ configPerspective = HopGui.getConfigurationPerspective();
+ if (configPerspective != null) {
+ HopGui.getInstance().setActivePerspective(configPerspective);
+
+ // Defer tab/tree selection until perspective is fully activated
+ Display.getCurrent()
+ .asyncExec(
+ () -> {
+ ConfigurationPerspective perspective = HopGui.getConfigurationPerspective();
+ if (perspective != null) {
+ perspective.showNotificationsTab();
+ }
+ });
+ }
+ }
+ });
+
+ Button clearAll = new Button(header, SWT.PUSH);
+ clearAll.setText(BaseMessages.getString(PKG, "NotificationPanel.ClearAll"));
+ clearAll.setToolTipText(BaseMessages.getString(PKG, "NotificationPanel.ClearAll.Tooltip"));
+ PropsUi.setLook(clearAll);
+ FormData fdClearAll = new FormData();
+ fdClearAll.right = new FormAttachment(settingsButton, -10);
+ fdClearAll.top = new FormAttachment(0, 5);
+ fdClearAll.bottom = new FormAttachment(100, -5);
+ clearAll.setLayoutData(fdClearAll);
+ clearAll.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ NotificationService.getInstance().clearAll();
+ updateNotifications();
+ }
+ });
+
+ Button markAllRead = new Button(header, SWT.PUSH);
+ markAllRead.setText(BaseMessages.getString(PKG, "NotificationPanel.MarkAllRead"));
+ PropsUi.setLook(markAllRead);
+ FormData fdMarkAll = new FormData();
+ fdMarkAll.right = new FormAttachment(clearAll, -10);
+ fdMarkAll.top = new FormAttachment(0, 5);
+ fdMarkAll.bottom = new FormAttachment(100, -5);
+ markAllRead.setLayoutData(fdMarkAll);
+ markAllRead.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ NotificationService.getInstance().markAllAsRead();
+ updateNotifications();
+ }
+ });
+
+ // Scrolled content area
+ scrolledComposite = new ScrolledComposite(shell, SWT.V_SCROLL | SWT.BORDER);
+ PropsUi.setLook(scrolledComposite);
+ FormData fdScrolled = new FormData();
+ fdScrolled.left = new FormAttachment(0, 0);
+ fdScrolled.right = new FormAttachment(100, 0);
+ fdScrolled.top = new FormAttachment(header, 0);
+ fdScrolled.bottom = new FormAttachment(100, -40);
+ scrolledComposite.setLayoutData(fdScrolled);
+
+ contentComposite = new Composite(scrolledComposite, SWT.NONE);
+ contentComposite.setLayout(new FormLayout());
+ PropsUi.setLook(contentComposite);
+ scrolledComposite.setContent(contentComposite);
+ scrolledComposite.setExpandHorizontal(true);
+ scrolledComposite.setExpandVertical(true);
+
+ // Footer
+ Composite footer = new Composite(shell, SWT.NONE);
+ footer.setLayout(new FormLayout());
+ PropsUi.setLook(footer);
+ FormData fdFooter = new FormData();
+ fdFooter.left = new FormAttachment(0, 0);
+ fdFooter.right = new FormAttachment(100, 0);
+ fdFooter.bottom = new FormAttachment(100, 0);
+ footer.setLayoutData(fdFooter);
+
+ Button closeButton = new Button(footer, SWT.PUSH);
+ closeButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Close"));
+ PropsUi.setLook(closeButton);
+ FormData fdClose = new FormData();
+ fdClose.right = new FormAttachment(100, -10);
+ fdClose.top = new FormAttachment(0, 5);
+ fdClose.bottom = new FormAttachment(100, -5);
+ closeButton.setLayoutData(fdClose);
+ closeButton.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ hide();
+ }
+ });
+
+ // Close when clicking outside
+ shell.addListener(
+ SWT.Deactivate,
+ e -> {
+ if (!shell.isDisposed()) {
+ Display.getCurrent()
+ .asyncExec(
+ () -> {
+ if (!shell.isDisposed() && !shell.isFocusControl()) {
+ hide();
+ }
+ });
+ }
+ });
+
+ // The panel hangs off the bell in the main toolbar, so it has to follow the main window
+ // whenever that moves or is resized, not just when it is resized.
+ if (parentShell != null && !parentShell.isDisposed()) {
+ Listener repositionListener =
+ e -> {
+ if (shell != null && !shell.isDisposed() && isVisible) {
+ Display.getCurrent()
+ .asyncExec(
+ () -> {
+ if (shell != null && !shell.isDisposed() && isVisible) {
+ positionPanel();
+ }
+ });
+ }
+ };
+ parentShell.addListener(SWT.Resize, repositionListener);
+ parentShell.addListener(SWT.Move, repositionListener);
+ }
+
+ shell.setSize(400, 500);
+
+ // Add listener to shell resize to ensure titles truncate properly
+ shell.addListener(
+ SWT.Resize,
+ e -> {
+ if (contentComposite != null && !contentComposite.isDisposed()) {
+ // Force layout update to ensure titles truncate correctly
+ contentComposite.layout(true, true);
+ }
+ });
+ }
+
+ /** Update the notifications display */
+ private void updateNotifications() {
+ if (contentComposite == null || contentComposite.isDisposed()) {
+ return;
+ }
+
+ // Clear existing notifications
+ for (Control control : contentComposite.getChildren()) {
+ control.dispose();
+ }
+
+ // Get configuration options
+ org.apache.hop.core.config.HopConfig hopConfig =
+ org.apache.hop.core.config.HopConfig.getInstance();
+ boolean showReadNotifications =
+ org.apache.hop.core.config.HopConfig.readOptionString(
+ "notification.showReadNotifications", "true")
+ .equalsIgnoreCase("true");
+ String daysToGoBackStr =
+ org.apache.hop.core.config.HopConfig.readOptionString(
+ "notification.global.daysToGoBack", "30");
+ int daysToGoBack = 0;
+ try {
+ daysToGoBack = Integer.parseInt(daysToGoBackStr);
+ } catch (NumberFormatException e) {
+ daysToGoBack = 30; // Default to 30 days
+ }
+
+ // Get provider errors and notifications
+ List<org.apache.hop.ui.hopgui.notifications.ProviderErrorInfo> providerErrors =
+ NotificationService.getInstance().getProviderErrors();
+ List<Notification> notifications =
+ NotificationService.getInstance().getNotifications(!showReadNotifications, daysToGoBack);
+ // Read once per repaint: getSourceColor runs twice for every notification on screen.
+ sourcesForRender = org.apache.hop.ui.hopgui.notifications.config.NotificationSources.load();
+
+ Control lastControl = null;
+
+ // Provider error banner
+ if (!providerErrors.isEmpty()) {
+ Composite errorBanner = createProviderErrorBanner(providerErrors, lastControl);
+ lastControl = errorBanner;
+ }
+
+ if (notifications.isEmpty() && lastControl == null) {
+ Label emptyLabel = new Label(contentComposite, SWT.CENTER | SWT.WRAP);
+ emptyLabel.setText(BaseMessages.getString(PKG, "NotificationPanel.NoNotifications"));
+ PropsUi.setLook(emptyLabel);
+ FormData fdEmpty = new FormData();
+ fdEmpty.left = new FormAttachment(0, 10);
+ fdEmpty.right = new FormAttachment(100, -10);
+ fdEmpty.top = new FormAttachment(0, 20);
+ emptyLabel.setLayoutData(fdEmpty);
+ } else if (!notifications.isEmpty()) {
+ // Every notification becomes a small stack of widgets. Beyond a screenful or two nobody
+ // scrolls anyway, and building hundreds of them is what makes opening the panel feel slow.
+ int shown = Math.min(notifications.size(), MAX_RENDERED_NOTIFICATIONS);
+ for (Notification notification : notifications.subList(0, shown)) {
+ try {
+ Composite notifComposite = createNotificationItem(notification, lastControl);
+ lastControl = notifComposite;
+ } catch (Exception e) {
+ // Log error but continue with other notifications
+ LogChannel.UI.logError("Error creating notification item: " + notification.getTitle(), e);
+ }
+ }
+ if (notifications.size() > shown) {
+ Label moreLabel = new Label(contentComposite, SWT.CENTER | SWT.WRAP);
+ moreLabel.setText(
+ BaseMessages.getString(
+ PKG,
+ "NotificationPanel.MoreNotifications",
+ Integer.toString(notifications.size() - shown)));
+ PropsUi.setLook(moreLabel);
+ FormData fdMore = new FormData();
+ fdMore.left = new FormAttachment(0, 10);
+ fdMore.right = new FormAttachment(100, -10);
+ fdMore.top = new FormAttachment(lastControl, 10);
+ moreLabel.setLayoutData(fdMore);
+ lastControl = moreLabel;
+ }
+ } else if (lastControl != null && notifications.isEmpty()) {
+ // Errors only, no notifications
+ Label emptyLabel = new Label(contentComposite, SWT.CENTER | SWT.WRAP);
+ emptyLabel.setText(BaseMessages.getString(PKG, "NotificationPanel.NoNotifications"));
+ PropsUi.setLook(emptyLabel);
+ FormData fdEmpty = new FormData();
+ fdEmpty.left = new FormAttachment(0, 10);
+ fdEmpty.right = new FormAttachment(100, -10);
+ fdEmpty.top = new FormAttachment(lastControl, 10);
+ emptyLabel.setLayoutData(fdEmpty);
+ }
+
+ // Force layout of content composite and scrolled composite
+ if (contentComposite != null && !contentComposite.isDisposed()) {
+ // Get the scrolled composite width first to ensure proper sizing
+ int availableWidth = SWT.DEFAULT;
+ if (scrolledComposite != null && !scrolledComposite.isDisposed()) {
+ org.eclipse.swt.graphics.Rectangle scrolledBounds = scrolledComposite.getBounds();
+ if (scrolledBounds.width > 0) {
+ availableWidth = scrolledBounds.width - 20; // Account for margins
+ }
+ }
+
+ // Layout scrolled composite first to get its actual width
+ if (scrolledComposite != null && !scrolledComposite.isDisposed()) {
+ scrolledComposite.layout(true, false);
+ org.eclipse.swt.graphics.Rectangle scrolledBounds = scrolledComposite.getBounds();
+ if (scrolledBounds.width > 0) {
+ availableWidth = scrolledBounds.width - 20; // Account for margins
+ } else {
+ // Fallback: use shell width if scrolled composite not sized yet
+ if (shell != null && !shell.isDisposed()) {
+ availableWidth = shell.getSize().x > 0 ? shell.getSize().x - 40 : 380;
+ } else {
+ availableWidth = 380; // Default width
+ }
+ }
+ }
+
+ // Layout content composite with proper width constraint
+ // This is critical for SWT.WRAP labels to calculate their height
+ contentComposite.layout(true, true);
+
+ // Compute size with width constraint for proper wrapping
+ org.eclipse.swt.graphics.Point contentSize =
+ contentComposite.computeSize(availableWidth, SWT.DEFAULT);
+
+ if (scrolledComposite != null && !scrolledComposite.isDisposed()) {
+ scrolledComposite.setMinSize(contentSize);
+ scrolledComposite.layout(true, true);
+ }
+ }
+ }
+
+ /** Create the provider error banner with Retry button */
+ private Composite createProviderErrorBanner(
+ List<org.apache.hop.ui.hopgui.notifications.ProviderErrorInfo> errors, Control above) {
+ Composite banner = new Composite(contentComposite, SWT.BORDER);
+ FormLayout bannerLayout = new FormLayout();
+ // The error text wraps to several lines; without a bottom margin the last one sits on the
+ // border, because nothing attaches the final label to the bottom of the banner.
+ bannerLayout.marginBottom = 10;
+ banner.setLayout(bannerLayout);
+ // Look first, then the banner's own colours: setLook applies the theme's foreground, which on
+ // a dark theme is near white and left this text unreadable on the light background. Both
+ // colours are set explicitly so the banner reads the same whichever theme is in use.
+ PropsUi.setLook(banner);
+ Color bannerBackground = GuiResource.getInstance().getColor(255, 248, 220); // Light yellow
+ Color bannerForeground = GuiResource.getInstance().getColor(60, 50, 20); // Dark brown
+ banner.setBackground(bannerBackground);
+ banner.setForeground(bannerForeground);
+
+ FormData fdBanner = new FormData();
+ fdBanner.left = new FormAttachment(0, 0);
+ fdBanner.right = new FormAttachment(100, 0);
+ fdBanner.top = above != null ? new FormAttachment(above, 10) : new FormAttachment(0, 10);
+ banner.setLayoutData(fdBanner);
+
+ Label headerLabel = new Label(banner, SWT.WRAP);
+ headerLabel.setText(BaseMessages.getString(PKG, "NotificationPanel.ProviderErrors"));
+ PropsUi.setLook(headerLabel);
+ headerLabel.setBackground(bannerBackground);
+ headerLabel.setForeground(bannerForeground);
+ FormData fdHeader = new FormData();
+ fdHeader.left = new FormAttachment(0, 10);
+ fdHeader.right = new FormAttachment(100, -80);
+ fdHeader.top = new FormAttachment(0, 10);
+ headerLabel.setLayoutData(fdHeader);
+
+ Button retryButton = new Button(banner, SWT.PUSH);
+ retryButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Retry"));
+ retryButton.setToolTipText(BaseMessages.getString(PKG, "NotificationPanel.Retry.Tooltip"));
+ PropsUi.setLook(retryButton);
+ FormData fdRetry = new FormData();
+ fdRetry.right = new FormAttachment(100, -10);
+ fdRetry.top = new FormAttachment(0, 5);
+ retryButton.setLayoutData(fdRetry);
+ retryButton.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ // Fetches in the background; the panel refreshes through notificationsChanged().
+ NotificationService.getInstance().retryNow();
+ }
+ });
+
+ Control lastLine = headerLabel;
+ for (org.apache.hop.ui.hopgui.notifications.ProviderErrorInfo err : errors) {
+ String text =
+ BaseMessages.getString(
+ PKG, "NotificationPanel.ProviderErrorItem", err.getProviderName(), err.getMessage());
+ Label line = new Label(banner, SWT.WRAP);
+ line.setText(text);
+ PropsUi.setLook(line);
+ line.setBackground(bannerBackground);
+ line.setForeground(bannerForeground);
+ FormData fdLine = new FormData();
+ fdLine.left = new FormAttachment(0, 10);
+ fdLine.right = new FormAttachment(100, -10);
+ fdLine.top = new FormAttachment(lastLine, 5);
+ line.setLayoutData(fdLine);
+ lastLine = line;
+ }
+
+ return banner;
+ }
+
+ /** Create a notification item UI */
+ private Composite createNotificationItem(Notification notification, Control above) {
+ Composite composite = new Composite(contentComposite, SWT.BORDER);
+ composite.setLayout(new FormLayout());
+ PropsUi.setLook(composite);
+
+ GuiResource guiResource = GuiResource.getInstance();
+
+ // Set background color based on read state and priority
+ // Note: We'll update this dynamically when notification is marked as read
+ updateNotificationBackground(composite, notification, guiResource);
+
+ // Set FormData for positioning in parent
+ FormData fdComposite = new FormData();
+ fdComposite.left = new FormAttachment(0, 0);
+ fdComposite.right = new FormAttachment(100, 0);
+ if (above != null) {
+ fdComposite.top = new FormAttachment(above, 5);
+ } else {
+ fdComposite.top = new FormAttachment(0, 5);
+ }
+ // Don't set bottom - composite will size to its children
+ composite.setLayoutData(fdComposite);
+
+ // Priority indicator (colored bar on the left)
+ Composite priorityBar = new Composite(composite, SWT.NONE);
+ priorityBar.setLayout(null);
+ priorityBar.setData("type", "priorityBar"); // Mark for later updates
+ FormData fdPriorityBar = new FormData();
+ fdPriorityBar.left = new FormAttachment(0, 0);
+ fdPriorityBar.top = new FormAttachment(0, 0);
+ fdPriorityBar.bottom = new FormAttachment(100, 0);
+ fdPriorityBar.width = 4;
+ priorityBar.setLayoutData(fdPriorityBar);
+
+ // Set initial priority bar color (will be updated when read state changes)
+ updatePriorityBar(priorityBar, notification, guiResource);
+
+ // Source color indicator (small colored square) - positioned on the left, after priority bar
+ // A Canvas, not a plain Composite: the border below is drawn in a paint listener, and RAP
+ // only offers one on Canvas. On a Composite this compiles against desktop SWT and fails in
+ // Hop Web with NoSuchMethodError, taking the whole notification list down with it.
+ Canvas sourceIndicator = new Canvas(composite, SWT.NONE);
+ sourceIndicator.setLayout(null);
+ sourceIndicator.setData("type", "sourceIndicator"); // Mark to exclude from click handling
+ PropsUi.setLook(sourceIndicator);
+ // Get color for this source from configuration
+ org.eclipse.swt.graphics.Color sourceColor = getSourceColor(notification, guiResource);
+ sourceIndicator.setBackground(sourceColor);
+ FormData fdSourceIndicator = new FormData();
+ // Position after priority bar, 8px gap
+ fdSourceIndicator.left = new FormAttachment(priorityBar, 8);
+ fdSourceIndicator.top = new FormAttachment(0, 10);
+ fdSourceIndicator.width = 12; // Fixed width
+ fdSourceIndicator.height = 12; // Fixed height
+ sourceIndicator.setLayoutData(fdSourceIndicator);
+
+ // Add a PaintListener to draw a border for better visibility
+ sourceIndicator.addPaintListener(
+ e -> {
+ org.eclipse.swt.graphics.Rectangle bounds = sourceIndicator.getBounds();
+ e.gc.setForeground(guiResource.getColorDarkGray());
+ e.gc.setLineWidth(1);
+ e.gc.drawRectangle(0, 0, bounds.width - 1, bounds.height - 1);
+ });
+
+ // Tooltip with source name and URL (source name shown in tooltip, not as text)
+ String tooltipText = buildSourceTooltip(notification);
+ sourceIndicator.setToolTipText(tooltipText);
+
+ // Title - use CLabel for automatic ellipsis truncation
+ // Title starts after source indicator
+ CLabel titleLabel = new CLabel(composite, SWT.LEFT);
+ String fullTitle = notification.getTitle() != null ? notification.getTitle() : "";
+ titleLabel.setText(fullTitle);
+ titleLabel.setData("type", "title"); // Mark for later updates
+ titleLabel.setData("fullTitle", fullTitle); // Store full title for tooltip
+ PropsUi.setLook(titleLabel);
+ if (!notification.isRead()) {
+ titleLabel.setFont(guiResource.getFontBold());
+ }
+ // Set tooltip to show full title if truncated
+ titleLabel.setToolTipText(fullTitle);
+ FormData fdTitle = new FormData();
+ // Title starts after source indicator with 8px gap
+ fdTitle.left = new FormAttachment(sourceIndicator, 8);
+ // Title extends to right edge with margin
+ fdTitle.right = new FormAttachment(100, -10);
+ fdTitle.top = new FormAttachment(0, 10);
+ // Don't attach bottom - CLabel will size to its preferred height
+ titleLabel.setLayoutData(fdTitle);
+
+ // Timestamp between title and body (always visible)
+ Label timeLabel = new Label(composite, SWT.NONE);
+ String timeText = formatTimestamp(notification.getTimestamp());
+ if (timeText == null || timeText.isEmpty()) {
+ timeText = "Unknown date";
+ }
+ timeLabel.setText(timeText);
+ PropsUi.setLook(timeLabel);
+ timeLabel.setForeground(guiResource.getColorDarkGray());
+ FormData fdTime = new FormData();
+ fdTime.left = new FormAttachment(priorityBar, 10);
+ // Timestamp extends to right edge of composite
+ fdTime.right = new FormAttachment(100, -10);
+ fdTime.top = new FormAttachment(titleLabel, 5);
+ // Don't attach bottom - label will size to its preferred height
+ timeLabel.setLayoutData(fdTime);
+
+ // Message/Description - simplified, truncated to max 3-5 lines
+ Label messageLabel = null;
+ String message = notification.getMessage();
+
+ if (message != null && !message.isEmpty()) {
+ // Limit to approximately 3-5 lines (roughly 200-300 characters)
+ // Simple truncation - just show start of message
+ int maxLength = 250;
+ String displayMessage = message;
+ if (displayMessage.length() > maxLength) {
+ displayMessage = displayMessage.substring(0, maxLength).trim() + "...";
+ }
+ messageLabel = new Label(composite, SWT.WRAP);
+ messageLabel.setText(displayMessage);
+ PropsUi.setLook(messageLabel);
+ FormData fdMessage = new FormData();
+ fdMessage.left = new FormAttachment(priorityBar, 10);
+ // Message extends to right edge of composite (sourceIndicator is on left, so don't constrain
+ // by it)
+ fdMessage.right = new FormAttachment(100, -10);
+ fdMessage.top = new FormAttachment(timeLabel, 5);
+ // Don't attach bottom - label will wrap and size to its content
+ messageLabel.setLayoutData(fdMessage);
+
+ // Set default cursor (not clickable)
+ messageLabel.setCursor(
+ composite.getDisplay().getSystemCursor(org.eclipse.swt.SWT.CURSOR_ARROW));
+ }
+
+ // Force layout of this composite to ensure all children are properly sized
+ composite.layout(true, true);
+
+ // Set cursor to pointer to indicate clickability for entire notification area
+ // Store reference to notification ID and guiResource for updates
+ composite.setData("notificationId", notification.getId());
+ composite.setData("guiResource", guiResource);
+
+ // Set cursor behavior: title and composite are clickable, body and timestamp are not
+ org.eclipse.swt.graphics.Cursor handCursor =
+ composite.getDisplay().getSystemCursor(org.eclipse.swt.SWT.CURSOR_HAND);
+ org.eclipse.swt.graphics.Cursor defaultCursor =
+ composite.getDisplay().getSystemCursor(org.eclipse.swt.SWT.CURSOR_ARROW);
+
+ // Title is clickable - use hand cursor
+ titleLabel.setCursor(handCursor);
+
+ // Body (message) and timestamp are NOT clickable - use default cursor
+ if (messageLabel != null) {
+ messageLabel.setCursor(defaultCursor);
+ }
+ timeLabel.setCursor(defaultCursor);
+
+ // Composite itself is clickable (for clicking outside title but still on notification)
+ composite.setCursor(handCursor);
+
+ // Click handler - attach to composite and all child controls
+ org.eclipse.swt.widgets.Listener clickListener =
+ e -> {
+ // Mark as read first - this updates the notification in the service
+ NotificationService.getInstance().markAsRead(notification.getId());
+
+ // Get fresh notification from service to ensure we have updated state
+ List<Notification> allNotifications =
+ NotificationService.getInstance().getNotifications(false);
+ Notification updatedNotification =
+ allNotifications.stream()
+ .filter(n -> notification.getId().equals(n.getId()))
+ .findFirst()
+ .orElse(notification);
+
+ // Ensure it's marked as read (should already be, but be safe)
+ updatedNotification.setRead(true);
+
+ // Update visual state immediately
+ updateNotificationBackground(composite, updatedNotification, guiResource);
+
+ // Force redraw to ensure visual changes are visible
+ composite.redraw();
+
+ // Open link if available. NotificationService drops links it will not open, but the
+ // link is handed to the operating system here, so it is checked again at the click.
+ String link = updatedNotification.getLink();
+ if (link != null && !link.isEmpty()) {
+ if (NotificationLinks.isSafe(link)) {
+ try {
+ EnvironmentUtils.getInstance().openUrl(link);
+ } catch (Exception ex) {
+ LogChannel.UI.logError("Error opening notification link " + link, ex);
+ }
+ } else {
+ LogChannel.UI.logBasic(
+ "Refusing to open notification link "
+ + link
+ + ": only http and https are opened");
+ }
+ }
+ };
+
+ // Attach click handler to composite
+ composite.addListener(SWT.MouseDown, clickListener);
+
+ // Also attach to all child controls to make entire area clickable
+ attachClickListenerRecursive(composite, clickListener);
+
+ return composite;
+ }
+
+ /** Update notification background based on read state and priority */
+ private void updateNotificationBackground(
+ Composite composite, Notification notification, GuiResource guiResource) {
+ if (notification.isRead()) {
+ // Read notifications have default background
+ composite.setBackground(null);
+ } else {
+ // Unread notifications have colored background based on priority
+ if (notification.getPriority() != null) {
+ switch (notification.getPriority()) {
+ case ERROR:
+ composite.setBackground(guiResource.getColor(255, 240, 240)); // Light red tint
+ break;
+ case WARNING:
+ composite.setBackground(guiResource.getColor(255, 250, 240)); // Light yellow tint
+ break;
+ case INFO:
+ default:
+ composite.setBackground(guiResource.getColorLightGray());
+ break;
+ }
+ } else {
+ composite.setBackground(guiResource.getColorLightGray());
+ }
+ }
+
+ // Update priority bar color based on read state
+ Control[] children = composite.getChildren();
+ for (Control child : children) {
+ if (child instanceof Composite) {
+ Object type = child.getData("type");
+ if ("priorityBar".equals(type)) {
+ updatePriorityBar((Composite) child, notification, guiResource);
+ }
+ }
+ // Check for title label (can be CLabel or Label)
+ Object type = child.getData("type");
+ if ("title".equals(type)) {
+ if (child instanceof CLabel) {
+ CLabel titleLabel = (CLabel) child;
+ if (notification.isRead()) {
+ // Use default font (remove bold)
+ titleLabel.setFont(null);
+ } else {
+ titleLabel.setFont(guiResource.getFontBold());
+ }
+ } else if (child instanceof Label) {
+ Label titleLabel = (Label) child;
+ if (notification.isRead()) {
+ // Use default font (remove bold)
+ titleLabel.setFont(null);
+ } else {
+ titleLabel.setFont(guiResource.getFontBold());
+ }
+ }
+ }
+ }
+ }
+
+ /** Update priority bar color based on read state */
+ private void updatePriorityBar(
+ Composite priorityBar, Notification notification, GuiResource guiResource) {
+ if (notification.isRead()) {
+ // Read notifications have gray priority bar
+ priorityBar.setBackground(guiResource.getColorGray());
+ } else {
+ // Unread notifications use the source color from configuration
+ org.eclipse.swt.graphics.Color sourceColor = getSourceColor(notification, guiResource);
+ priorityBar.setBackground(sourceColor);
+ }
+ }
+
+ /** Recursively attach click listener to composite and all its children */
+ private void attachClickListenerRecursive(
+ Control control, org.eclipse.swt.widgets.Listener listener) {
+ if (control == null || control.isDisposed()) {
+ return;
+ }
+ // Don't attach to the priority bar or source indicator (they're just visual indicators)
+ Object type = control.getData("type");
+ if (!"priorityBar".equals(type) && !"sourceIndicator".equals(type)) {
+ // Only attach to leaf controls (Labels, etc.) to avoid duplicate events
+ // The composite already has the listener, so we don't need to attach to child composites
+ if (!(control instanceof Composite)) {
+ control.addListener(SWT.MouseDown, listener);
+ }
+ }
+ if (control instanceof Composite) {
+ Composite composite = (Composite) control;
+ for (Control child : composite.getChildren()) {
+ attachClickListenerRecursive(child, listener);
+ }
+ }
+ }
+
+ /**
+ * Get color for a notification source. This will be configurable via ConfigOption later. For now,
+ * uses a simple hash-based color scheme.
+ */
+ private org.eclipse.swt.graphics.Color getSourceColor(
+ Notification notification, GuiResource guiResource) {
+ // Try to get color from notification source configuration
+ String sourceId = notification.getSourceId();
+ if (sourceId != null && !sourceId.isEmpty()) {
+ for (org.apache.hop.ui.hopgui.notifications.config.NotificationSourceConfig source :
+ sourcesForRender) {
+ if (sourceId.equals(source.getId())) {
+ String colorHex = source.getColor();
+ if (colorHex != null && !colorHex.isEmpty()) {
+ try {
+ String hex = colorHex.startsWith("#") ? colorHex.substring(1) : colorHex;
+ int colorValue = Integer.parseInt(hex, 16);
+ return guiResource.getColor(
+ (colorValue >> 16) & 0xFF, (colorValue >> 8) & 0xFF, colorValue & 0xFF);
+ } catch (NumberFormatException e) {
+ // Not a colour we can read; fall through to one derived from the source name.
+ }
+ }
+ break;
+ }
+ }
+ }
+
+ // Fallback: hash-based color generation for consistent colors per source
+ String source = notification.getSource();
+ if (source == null || source.isEmpty()) {
+ return guiResource.getColorGray();
+ }
+
+ int hash = source.hashCode();
+ int r = Math.abs(hash % 200) + 50; // 50-250 range
+ int g = Math.abs((hash >> 8) % 200) + 50;
+ int b = Math.abs((hash >> 16) % 200) + 50;
+
+ return guiResource.getColor(r, g, b);
+ }
+
+ /** Build tooltip text for source indicator showing source name and URL */
+ private String buildSourceTooltip(Notification notification) {
+ StringBuilder tooltip = new StringBuilder();
+ if (notification.getSource() != null && !notification.getSource().isEmpty()) {
+ tooltip.append("Source: ").append(notification.getSource());
+ }
+ if (notification.getLink() != null && !notification.getLink().isEmpty()) {
+ if (tooltip.length() > 0) {
+ tooltip.append("\n");
+ }
+ tooltip.append("URL: ").append(notification.getLink());
+ }
+ return tooltip.length() > 0 ? tooltip.toString() : "Unknown source";
+ }
+
+ /** Format timestamp for display */
+ private String formatTimestamp(Date timestamp) {
+ if (timestamp == null) {
+ return "";
+ }
+ long diff = System.currentTimeMillis() - timestamp.getTime();
+ long minutes = diff / 60000;
+ long hours = diff / 3600000;
+ long days = diff / 86400000;
+
+ if (minutes < 1) {
+ return "Just now";
Review Comment:
Moved to the bundle with separate singular/plural keys, since `BaseMessages` has no plural rules. Also caught one you didn't mention: `new SimpleDateFormat("MMM d, yyyy")` had no `Locale`, so month names came from the platform default while everything around them was English — now `LanguageChoice.getInstance().getDefaultLocale()`. `"Unknown source"` was hardcoded too.
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hop Gui notification system #6188 (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters commented on code in PR #8270:
URL: https://github.com/apache/hop/pull/8270#discussion_r3941953665
##########
ui/src/main/java/org/apache/hop/ui/hopgui/notifications/NotificationProviderPlugins.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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 java.util.ArrayList;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.hop.core.logging.ILogChannel;
+import org.apache.hop.core.notifications.INotificationProvider;
+import org.apache.hop.core.notifications.NotificationProviderPluginType;
+import org.apache.hop.core.plugins.IPlugin;
+import org.apache.hop.core.plugins.PluginRegistry;
+import org.apache.hop.ui.hopgui.notifications.config.NotificationSourceConfig;
+
+/**
+ * The notification providers contributed by plugins, as found in the plugin registry.
+ *
+ * <p>A plugin declares a provider with {@link
+ * org.apache.hop.core.notifications.NotificationProviderPlugin} and is discovered from its jar. It
+ * therefore needs no entry in the configuration to work: a stored source only records what the user
+ * changed about it, and a provider whose plugin has been uninstalled simply stops being found.
+ */
+public final class NotificationProviderPlugins {
+
+ private NotificationProviderPlugins() {
+ // Utility class
+ }
+
+ /**
+ * @return The registry entries of every declared notification provider
+ */
+ public static List<IPlugin> plugins() {
+ List<IPlugin> plugins =
+ PluginRegistry.getInstance().getPlugins(NotificationProviderPluginType.class);
+ return plugins == null ? new ArrayList<>() : plugins;
+ }
+
+ /**
+ * @return The identifiers of every declared notification provider
+ */
+ public static Set<String> ids() {
+ Set<String> ids = new LinkedHashSet<>();
+ for (IPlugin plugin : plugins()) {
+ String id = idOf(plugin);
+ if (id != null) {
+ ids.add(id);
+ }
+ }
+ return ids;
+ }
+
+ /**
+ * Instantiate a declared provider.
+ *
+ * @param plugin The registry entry
+ * @param log Where to report a provider that cannot be loaded
+ * @return The provider, or null when its class could not be loaded
+ */
+ public static INotificationProvider load(IPlugin plugin, ILogChannel log) {
+ try {
+ return PluginRegistry.getInstance().loadClass(plugin, INotificationProvider.class);
+ } catch (Exception e) {
+ log.logError("Unable to load the notification provider of plugin " + plugin.getName(), e);
+ return null;
+ }
+ }
+
+ /**
+ * The identifier a plugin's provider is known by. This is the source id its notifications are
+ * qualified with, so it has to be the plugin id and nothing derived from the instance.
+ *
+ * @param plugin The registry entry
+ * @return The identifier, or null when the entry declares none
+ */
+ public static String idOf(IPlugin plugin) {
+ String[] ids = plugin.getIds();
+ return ids == null || ids.length == 0 ? null : ids[0];
+ }
+
+ /**
+ * Describe the declared providers as configuration sources, so the Notifications settings can
+ * list a plugin that has never been configured alongside the sources the user added.
+ *
+ * @return One source per declared provider, in registry order
+ */
+ public static List<NotificationSourceConfig> describeAsSources() {
+ List<NotificationSourceConfig> described = new ArrayList<>();
+ for (IPlugin plugin : plugins()) {
+ String id = idOf(plugin);
+ if (id == null) {
+ continue;
+ }
+ NotificationSourceConfig source = new NotificationSourceConfig();
+ source.setId(id);
+ source.setPluginId(id);
+ source.setName(
+ plugin.getName() == null || plugin.getName().isEmpty() ? id : plugin.getName());
+ source.setType(NotificationSourceConfig.SourceType.CUSTOM_PLUGIN);
+ source.setEnabled(true);
+ source.setPollIntervalMinutes("60");
Review Comment:
**[bug]** Discovered plugin sources are synthesized with `setPollIntervalMinutes("60")`. `MarketplaceNotificationProvider` defaults to 6 hours. Opening Notifications and clicking Save (documented as required) persists every discovered row via `saveSources()`, after which `reloadFromConfig` / `applyPollInterval` overwrites the marketplace provider with 60 minutes. The same happens if the user edits any source and saves the table.
**Suggestion:** Leave poll interval unset on synthesized sources so the provider default is kept, and only write a `poll.intervalMinutes` property when the user actually changes it.
##########
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);
+ request.addHeader(
+ "Accept", "application/rss+xml, application/atom+xml, application/xml, text/xml");
+ conditional.applyTo(request);
+
+ try (ClassicHttpResponse response = (ClassicHttpResponse) client.execute(request)) {
+ // Check HTTP status code
+ int statusCode = response.getCode();
+ if (statusCode == 304) {
+ // Unchanged since the last poll.
+ return new ArrayList<>(lastFetched);
+ }
+ if (statusCode < 200 || statusCode >= 300) {
+ throw new HopException("The feed at " + feedUrl + " returned HTTP " + statusCode + ".");
+ }
+
+ HttpEntity entity = response.getEntity();
+ if (entity == null) {
+ throw new HopException("The feed at " + feedUrl + " returned an empty response.");
+ }
+
+ try (InputStream rawInputStream = entity.getContent();
+ BufferedInputStream inputStream = new BufferedInputStream(rawInputStream, 8192)) {
+ // Read first few bytes to check for BOM or non-XML content
+ inputStream.mark(1024);
+ byte[] buffer = new byte[1024];
+ int bytesRead = inputStream.read(buffer);
+ inputStream.reset();
+
+ if (bytesRead > 0) {
+ String contentStart =
+ stripByteOrderMark(
+ new String(
+ buffer,
+ 0,
+ Math.min(bytesRead, 100),
+ java.nio.charset.StandardCharsets.UTF_8));
+ // Check if it looks like HTML (common error response)
+ if (contentStart.trim().startsWith("<html")
+ || contentStart.trim().startsWith("<!DOCTYPE html")) {
+ throw new HopException(
+ "The feed at "
+ + feedUrl
+ + " returned an HTML page instead of a feed. Check the URL, or whether a"
+ + " proxy or sign-in page is answering for it.");
+ }
+ // Check if it starts with XML declaration or valid XML tag
+ String trimmed = contentStart.trim();
+ if (!trimmed.startsWith("<?xml")
+ && !trimmed.startsWith("<feed")
+ && !trimmed.startsWith("<rss")
+ && !trimmed.startsWith("<rdf:RDF")) {
+ throw new HopException(
+ "The response from " + feedUrl + " is not an RSS or Atom feed.");
+ }
+ }
+
+ DocumentBuilder builder =
+ XmlParserFactoryProducer.createSecureDocBuilderFactory().newDocumentBuilder();
+ Document document = builder.parse(inputStream);
+
+ // Check if it's Atom or RSS
+ Element root = document.getDocumentElement();
+ if (root == null) {
+ throw new HopException("The feed at " + feedUrl + " is an empty XML document.");
+ }
+ String rootName = root.getNodeName();
+
+ if ("feed".equals(rootName) || rootName.contains("atom")) {
+ // Atom feed
+ notifications.addAll(parseAtomFeed(document));
+ } else if ("rss".equals(rootName) || "rdf:RDF".equals(rootName)) {
+ // RSS feed
+ notifications.addAll(parseRssFeed(document));
+ } else {
+ throw new HopException(
+ "The feed at " + feedUrl + " is in an unsupported format: <" + rootName + ">.");
+ }
+ }
+
+ // Only now that the feed has been read and understood. Remembering the validator any
+ // earlier means a parse that fails still arms the next request's If-None-Match: the feed
+ // would answer 304 forever, this method would return the entries it never managed to read
+ // (none), and because that is not a failure the error banner would clear itself.
+ conditional.remember(response);
+ }
+ lastFetched = new ArrayList<>(notifications);
+ } catch (HopException e) {
+ throw e;
+ } catch (Exception e) {
+ // Reported to the user through the panel's error banner. NotificationService catches this
+ // per provider, so one unreachable feed does not stop the others.
+ throw new HopException("Could not read the feed at " + feedUrl + ": " + e.getMessage(), e);
+ }
+
+ return notifications;
+ }
+
+ /**
+ * Drop a leading byte order mark.
+ *
+ * <p>Feeds written by Windows tooling are commonly served with one. Decoded from UTF-8 it is
+ * U+FEFF, which {@link String#trim()} leaves alone - it only strips characters up to U+0020 - so
+ * the sniff below would find the feed starting with an invisible character rather than with
+ * {@code <?xml}, and reject a feed the XML parser handles perfectly well.
+ *
+ * @param text The decoded start of the response
+ * @return The text without its byte order mark
+ */
+ static String stripByteOrderMark(String text) {
+ if (!text.isEmpty() && text.charAt(0) == '\uFEFF') {
+ return text.substring(1);
+ }
+ return text;
+ }
+
+ /** Parse Atom 1.0 feed */
+ List<Notification> parseAtomFeed(Document document) {
+ List<Notification> notifications = new ArrayList<>();
+ NodeList entries = document.getElementsByTagName("entry");
+
+ for (int i = 0; i < entries.getLength(); i++) {
+ Element entry = (Element) entries.item(i);
+ try {
+ String id = getElementText(entry, "id");
+ String title = getElementText(entry, "title");
+ String summary = getElementText(entry, "summary");
+ if (summary == null || summary.isEmpty()) {
+ summary = getElementText(entry, "content");
+ }
+ String link = getElementLink(entry);
+ String publishedText = getElementText(entry, "published");
+ if (publishedText == null || publishedText.isEmpty()) {
+ publishedText = getElementText(entry, "updated");
+ }
+ Date published = parseAtomDate(publishedText);
+ if (published == null) {
+ published = new Date();
+ }
+
+ String localId = entryId(id, link, title, publishedText);
+ if (localId == null) {
+ LogChannel.UI.logDetailed(
+ "Skipping an entry in the feed at " + feedUrl + " that cannot be identified");
+ continue;
+ }
+
+ Notification notification =
+ new Notification(
+ localId,
+ title != null ? title : "Untitled",
+ summary != null ? summary : "",
Review Comment:
**[suggestion]** Atom `summary`/`content` and RSS `description` are stored and shown with `Label.setText` unstripped. That is not RAP markup XSS (markup is not enabled), but feeds routinely ship HTML, so the panel shows raw tags, and SWT still treats `&` as a mnemonic.
**Suggestion:** Strip tags to text (or a small allow-list), collapse whitespace, and escape `&` for SWT (`&&`) before display. Keep using a text Label, not a Browser.
##########
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:
**[bug]** Per-session `NotificationService` is the right split for listeners and in-memory unread state, but each session also gets its own daemon scheduler (`newScheduledThreadPool(1)`) and polls every source in the process-wide `hop-config.json`. The default source is unauthenticated `api.github.com/repos/apache/hop/releases` (60 req/hour/IP). N Hop Web sessions therefore issue N identical GitHub calls shortly after login and again every poll interval, so a modest multi-user server will trip the rate limit and the error banner for everyone. Authenticated audit isolation does not help: sources live in shared HopConfig.
**Suggestion:** Keep read/removed/listeners per session, but fetch once per process (or per distinct source URL) and fan the result out. At minimum, coalesce GitHub polls and send a token from server config rather than N anonymous clients.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/notifications/providers/NotificationHttp.java:
##########
@@ -0,0 +1,135 @@
+/*
+ * 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 org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
+import org.apache.hop.core.util.HttpClientManager;
+import org.apache.hop.core.variables.Variables;
+
+/** Shared HTTP setup for the notification providers. */
+final class NotificationHttp {
+
+ /** Give up if the remote host has not accepted the connection by then. */
+ static final int CONNECT_TIMEOUT_MS = 10000;
+
+ /** Give up if the remote host has not answered by then. */
+ static final int RESPONSE_TIMEOUT_MS = 20000;
+
+ private NotificationHttp() {
+ // Utility class
+ }
+
+ /**
+ * A client with timeouts. {@link HttpClientManager#createDefaultClient()} sets none at all, and
+ * HttpClient 5 waits indefinitely for a response, so a single unresponsive feed would otherwise
+ * hold a polling thread forever.
+ *
+ * <p>The client is built on the process-wide shared connection manager, so do not close it:
+ * closing the response is what returns this request's connection to the pool.
+ *
+ * @return A client configured for polling a notification source
+ */
+ static CloseableHttpClient newClient() {
+ return newClient(null, null);
+ }
+
+ /**
+ * A client with timeouts, authenticating when credentials are given.
+ *
+ * @param username The user name, may be null or empty for anonymous access
+ * @param password The password or token, may be null or empty for anonymous access
+ * @return A client configured for polling a notification source
+ */
+ static CloseableHttpClient newClient(String username, String password) {
+ HttpClientManager.HttpClientBuilderFacade builder =
+ HttpClientManager.getInstance()
+ .createBuilder()
+ .setConnectionTimeout(CONNECT_TIMEOUT_MS)
+ .setSocketTimeout(RESPONSE_TIMEOUT_MS);
+ String resolvedPassword = resolve(password);
+ if (resolvedPassword != null && !resolvedPassword.isEmpty()) {
+ // A token is often all a source wants; GitHub, for one, ignores the user name entirely.
+ builder.setCredentials(resolve(username), resolvedPassword);
Review Comment:
**[bug]** `builder.setCredentials(resolve(username), resolvedPassword)` has two defects. (1) `HttpClientManager.setCredentials(user, password)` registers `AuthScope(null, null, -1, null, null)`, so Basic credentials are offered to **any** host after a redirect, not only `api.github.com` / the feed origin. (2) HC5 `UsernamePasswordCredentials` / `BasicUserPrincipal` require a non-null username; a token-only source whose JSON has `auth.password` and no `auth.username` NPEs on fetch even though the UI and docs treat username as optional.
**Suggestion:** Scope `AuthScope` to the request host (and do not forward credentials on cross-host redirects). Coerce a missing username to `""` or `"token"`. Prefer a GitHub `Authorization: Bearer` header over Basic for PATs.
##########
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:
**[bug]** `HttpGet(feedUrl)` fetches whatever URL the user stored. There is no http(s)-only check (unlike `NotificationLinks.isSafe`), no host allow/deny list, and `HttpClientManager` follows redirects by default. On the desktop that is the user's own client; in Hop Web the **server** periodically fetches it. Any session that can add an RSS source can probe loopback, link-local, and cloud metadata (`http://127.0.0.1:8080/hop/`, `http://169.254.169.254/`), including via a 302 from an otherwise "https" feed. Responses are also unbounded.
**Suggestion:** Reject anything `NotificationLinks.isSafe` would reject before `HttpGet`. In Hop Web, also refuse loopback/link-local/private ranges and disable or tightly scope redirects. Cap response size. Do not treat "the user configured it" as sufficient for a multi-user server.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/notifications/config/NotificationSourceDialog.java:
##########
@@ -0,0 +1,976 @@
+/*
+ * 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.config;
+
+import java.util.UUID;
+import org.apache.hop.core.util.Utils;
+import org.apache.hop.i18n.BaseMessages;
+import org.apache.hop.ui.core.PropsUi;
+import org.apache.hop.ui.core.dialog.ErrorDialog;
+import org.apache.hop.ui.core.widget.PasswordTextVar;
+import org.apache.hop.ui.core.widget.TextVar;
+import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.pipeline.transform.BaseTransformDialog;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.events.SelectionAdapter;
+import org.eclipse.swt.events.SelectionEvent;
+import org.eclipse.swt.layout.FormAttachment;
+import org.eclipse.swt.layout.FormData;
+import org.eclipse.swt.layout.FormLayout;
+import org.eclipse.swt.widgets.Button;
+import org.eclipse.swt.widgets.ColorDialog;
+import org.eclipse.swt.widgets.Combo;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Label;
+import org.eclipse.swt.widgets.Shell;
+
+/** Dialog for adding or editing a notification source configuration. */
+public class NotificationSourceDialog {
+
+ private static final Class<?> PKG = NotificationSourceDialog.class;
+
+ private Shell shell;
+ private Shell parentShell;
+ private NotificationSourceConfig sourceConfig;
+
+ /**
+ * Whether the dialog was closed without confirming. Only OK clears it: closing the window any
+ * other way - the title bar, Escape - leaves the edits unsaved, which is what closing a dialog
+ * means everywhere else.
+ */
+ private boolean cancelled = true;
+
+ private PropsUi props = PropsUi.getInstance();
+
+ // UI widgets
+ private TextVar wName;
+ private Combo wType;
+ private Button wEnabled;
+ private Button wColorButton;
+ private Label wColorPreview;
+ private Composite wTypeSpecificComposite;
+ private TextVar wGithubUrl; // For URL input
+ private TextVar wGithubOwner;
+ private TextVar wGithubRepo;
+ private Button wGithubIncludePrereleases;
+ private TextVar wRssUrl;
+ private TextVar wPluginId;
+ private TextVar wPollInterval;
+ private TextVar wDaysToGoBack;
+ private TextVar wUsername;
+ private PasswordTextVar wPassword;
+ private TextVar wMinimumVersion;
+
+ /** Set while one GitHub field is updating another, so the two directions do not loop. */
+ private boolean syncingGithubFields;
+
+ public NotificationSourceDialog(Shell parent, NotificationSourceConfig sourceConfig) {
+ this.parentShell = parent;
+ this.shell = new Shell(parent, SWT.DIALOG_TRIM | SWT.RESIZE | SWT.MAX | SWT.MIN);
+ this.sourceConfig = sourceConfig != null ? sourceConfig : new NotificationSourceConfig();
+ props.setLook(this.shell);
+ }
+
+ public String open() {
+ Display display = parentShell.getDisplay();
+
+ shell.setText(BaseMessages.getString(PKG, "NotificationSourceDialog.Title"));
+
+ FormLayout formLayout = new FormLayout();
Review Comment:
**[suggestion]** ~900 lines of hand-laid `FormAttachment` rows on the shell (name, type, credentials, GitHub/RSS/plugin fields, OK/Cancel). Project GUI dialogs are supposed to start from grouped `@GuiWidgetElement` + `GuiCompositeWidgets.addScrolledComposite` so the form scrolls and the button bar stays pinned. Type-specific fields can be extra groups, not a free-floating composite under the buttons.
**Suggestion:** Annotate `NotificationSourceConfig` (or a dialog model) with grouped widgets; keep custom parse/color controls as `registerExtraGroup` rather than laying out the whole shell by hand.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/notifications/NotificationPanel.java:
##########
@@ -0,0 +1,1002 @@
+/*
+ * 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 java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.List;
+import org.apache.hop.core.logging.LogChannel;
+import org.apache.hop.core.notifications.Notification;
+import org.apache.hop.i18n.BaseMessages;
+import org.apache.hop.ui.core.PropsUi;
+import org.apache.hop.ui.core.gui.GuiResource;
+import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.hopgui.ISingletonProvider;
+import org.apache.hop.ui.hopgui.ImplementationLoader;
+import org.apache.hop.ui.hopgui.perspective.configuration.ConfigurationPerspective;
+import org.apache.hop.ui.util.EnvironmentUtils;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.SWTException;
+import org.eclipse.swt.custom.CLabel;
+import org.eclipse.swt.custom.ScrolledComposite;
+import org.eclipse.swt.events.SelectionAdapter;
+import org.eclipse.swt.events.SelectionEvent;
+import org.eclipse.swt.graphics.Color;
+import org.eclipse.swt.graphics.Point;
+import org.eclipse.swt.layout.FormAttachment;
+import org.eclipse.swt.layout.FormData;
+import org.eclipse.swt.layout.FormLayout;
+import org.eclipse.swt.widgets.Button;
+import org.eclipse.swt.widgets.Canvas;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Label;
+import org.eclipse.swt.widgets.Listener;
+import org.eclipse.swt.widgets.Shell;
+
+/** Dropdown panel for displaying notifications */
+public class NotificationPanel implements INotificationListener {
+ private static final Class<?> PKG = NotificationPanel.class;
+
+ /** How many notifications the panel draws before it stops and says how many are left. */
+ private static final int MAX_RENDERED_NOTIFICATIONS = 100;
+
+ private static NotificationPanel fallback;
+
+ private static final ISingletonProvider PROVIDER = loadProvider();
+
+ private static ISingletonProvider loadProvider() {
+ try {
+ return (ISingletonProvider) ImplementationLoader.newInstance(NotificationPanel.class);
+ } catch (Throwable e) {
+ // hop-ui unit tests have no rcp/rap *Impl on the classpath. Anywhere else this is a
+ // misconfiguration worth shouting about: one instance would then be shared by every Hop Web
+ // session, which is the very thing the per-session provider exists to prevent.
+ LogChannel.GENERAL.logBasic(
+ "No NotificationPanelImpl found; falling back to a single instance for this process. "
+ + "In Hop Web that means every session shares one.");
+ return () -> {
+ synchronized (NotificationPanel.class) {
+ if (fallback == null) {
+ fallback = new NotificationPanel();
+ }
+ return fallback;
+ }
+ };
+ }
+ }
+
+ private Shell shell;
+ private Shell parentShell;
+ private ScrolledComposite scrolledComposite;
+ private Composite contentComposite;
+ private boolean isVisible = false;
+
+ /** The configured sources, refreshed each time the list is drawn, for the source colours. */
+ private java.util.List<org.apache.hop.ui.hopgui.notifications.config.NotificationSourceConfig>
+ sourcesForRender = new java.util.ArrayList<>();
+
+ /** Use {@link #getInstance()}. Public so RWT can create one per user session in Hop Web. */
+ public NotificationPanel() {
+ this.parentShell = HopGui.getInstance().getShell();
+ NotificationService.getInstance().addNotificationListener(this);
+ }
+
+ /**
+ * @return The notification panel of this process, or of this user's session in Hop Web
+ */
+ public static NotificationPanel getInstance() {
+ return (NotificationPanel) PROVIDER.getInstanceInternal();
+ }
+
+ /** Toggle the panel visibility */
+ public void toggle() {
+ if (isVisible) {
+ hide();
+ } else {
+ show();
+ }
+ }
+
+ /** Show the notification panel */
+ public void show() {
+ if (shell != null && !shell.isDisposed()) {
+ // Panel already exists, refresh notifications and show
+ updateNotifications();
+ shell.setVisible(true);
+ shell.setFocus();
+ // Without this the panel stays "not visible" after the first open and close, which stops
+ // notificationsChanged() from refreshing it and leaves the bell unable to close it again.
+ isVisible = true;
+ return;
+ }
+
+ createPanel();
+ updateNotifications();
+ positionPanel();
+ shell.setVisible(true);
+ isVisible = true;
+ }
+
+ /** Hide the notification panel */
+ public void hide() {
+ if (shell != null && !shell.isDisposed()) {
+ shell.setVisible(false);
+ }
+ isVisible = false;
+ }
+
+ /** Create the panel UI */
+ private void createPanel() {
+ // Use DIALOG_TRIM instead of ON_TOP to keep it attached to parent
+ // Remove ON_TOP so it doesn't stay on top when switching applications
+ shell = new Shell(parentShell, SWT.DIALOG_TRIM | SWT.RESIZE);
+ shell.setLayout(new FormLayout());
+ PropsUi.setLook(shell);
+
+ // Header
+ Composite header = new Composite(shell, SWT.NONE);
+ header.setLayout(new FormLayout());
+ PropsUi.setLook(header);
+ FormData fdHeader = new FormData();
+ fdHeader.left = new FormAttachment(0, 0);
+ fdHeader.right = new FormAttachment(100, 0);
+ fdHeader.top = new FormAttachment(0, 0);
+ header.setLayoutData(fdHeader);
+
+ // Settings button
+ Button settingsButton = new Button(header, SWT.PUSH);
+ settingsButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Settings"));
+ settingsButton.setToolTipText(
+ BaseMessages.getString(PKG, "NotificationPanel.Settings.Tooltip"));
+ PropsUi.setLook(settingsButton);
+ FormData fdSettings = new FormData();
+ fdSettings.right = new FormAttachment(100, -10);
+ fdSettings.top = new FormAttachment(0, 5);
+ fdSettings.bottom = new FormAttachment(100, -5);
+ settingsButton.setLayoutData(fdSettings);
+ settingsButton.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ org.apache.hop.ui.hopgui.perspective.configuration.ConfigurationPerspective
+ configPerspective = HopGui.getConfigurationPerspective();
+ if (configPerspective != null) {
+ HopGui.getInstance().setActivePerspective(configPerspective);
+
+ // Defer tab/tree selection until perspective is fully activated
+ Display.getCurrent()
+ .asyncExec(
+ () -> {
+ ConfigurationPerspective perspective = HopGui.getConfigurationPerspective();
+ if (perspective != null) {
+ perspective.showNotificationsTab();
+ }
+ });
+ }
+ }
+ });
+
+ Button clearAll = new Button(header, SWT.PUSH);
+ clearAll.setText(BaseMessages.getString(PKG, "NotificationPanel.ClearAll"));
+ clearAll.setToolTipText(BaseMessages.getString(PKG, "NotificationPanel.ClearAll.Tooltip"));
+ PropsUi.setLook(clearAll);
+ FormData fdClearAll = new FormData();
+ fdClearAll.right = new FormAttachment(settingsButton, -10);
+ fdClearAll.top = new FormAttachment(0, 5);
+ fdClearAll.bottom = new FormAttachment(100, -5);
+ clearAll.setLayoutData(fdClearAll);
+ clearAll.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ NotificationService.getInstance().clearAll();
+ updateNotifications();
+ }
+ });
+
+ Button markAllRead = new Button(header, SWT.PUSH);
+ markAllRead.setText(BaseMessages.getString(PKG, "NotificationPanel.MarkAllRead"));
+ PropsUi.setLook(markAllRead);
+ FormData fdMarkAll = new FormData();
+ fdMarkAll.right = new FormAttachment(clearAll, -10);
+ fdMarkAll.top = new FormAttachment(0, 5);
+ fdMarkAll.bottom = new FormAttachment(100, -5);
+ markAllRead.setLayoutData(fdMarkAll);
+ markAllRead.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ NotificationService.getInstance().markAllAsRead();
+ updateNotifications();
+ }
+ });
+
+ // Scrolled content area
+ scrolledComposite = new ScrolledComposite(shell, SWT.V_SCROLL | SWT.BORDER);
+ PropsUi.setLook(scrolledComposite);
+ FormData fdScrolled = new FormData();
+ fdScrolled.left = new FormAttachment(0, 0);
+ fdScrolled.right = new FormAttachment(100, 0);
+ fdScrolled.top = new FormAttachment(header, 0);
+ fdScrolled.bottom = new FormAttachment(100, -40);
+ scrolledComposite.setLayoutData(fdScrolled);
+
+ contentComposite = new Composite(scrolledComposite, SWT.NONE);
+ contentComposite.setLayout(new FormLayout());
+ PropsUi.setLook(contentComposite);
+ scrolledComposite.setContent(contentComposite);
+ scrolledComposite.setExpandHorizontal(true);
+ scrolledComposite.setExpandVertical(true);
+
+ // Footer
+ Composite footer = new Composite(shell, SWT.NONE);
+ footer.setLayout(new FormLayout());
+ PropsUi.setLook(footer);
+ FormData fdFooter = new FormData();
+ fdFooter.left = new FormAttachment(0, 0);
+ fdFooter.right = new FormAttachment(100, 0);
+ fdFooter.bottom = new FormAttachment(100, 0);
+ footer.setLayoutData(fdFooter);
+
+ Button closeButton = new Button(footer, SWT.PUSH);
+ closeButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Close"));
+ PropsUi.setLook(closeButton);
+ FormData fdClose = new FormData();
+ fdClose.right = new FormAttachment(100, -10);
+ fdClose.top = new FormAttachment(0, 5);
+ fdClose.bottom = new FormAttachment(100, -5);
+ closeButton.setLayoutData(fdClose);
+ closeButton.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ hide();
+ }
+ });
+
+ // Close when clicking outside
+ shell.addListener(
+ SWT.Deactivate,
+ e -> {
+ if (!shell.isDisposed()) {
+ Display.getCurrent()
+ .asyncExec(
+ () -> {
+ if (!shell.isDisposed() && !shell.isFocusControl()) {
+ hide();
+ }
+ });
+ }
+ });
+
+ // The panel hangs off the bell in the main toolbar, so it has to follow the main window
+ // whenever that moves or is resized, not just when it is resized.
+ if (parentShell != null && !parentShell.isDisposed()) {
+ Listener repositionListener =
+ e -> {
+ if (shell != null && !shell.isDisposed() && isVisible) {
+ Display.getCurrent()
+ .asyncExec(
+ () -> {
+ if (shell != null && !shell.isDisposed() && isVisible) {
+ positionPanel();
+ }
+ });
+ }
+ };
+ parentShell.addListener(SWT.Resize, repositionListener);
+ parentShell.addListener(SWT.Move, repositionListener);
+ }
+
+ shell.setSize(400, 500);
+
+ // Add listener to shell resize to ensure titles truncate properly
+ shell.addListener(
+ SWT.Resize,
+ e -> {
+ if (contentComposite != null && !contentComposite.isDisposed()) {
+ // Force layout update to ensure titles truncate correctly
+ contentComposite.layout(true, true);
+ }
+ });
+ }
+
+ /** Update the notifications display */
+ private void updateNotifications() {
+ if (contentComposite == null || contentComposite.isDisposed()) {
+ return;
+ }
+
+ // Clear existing notifications
+ for (Control control : contentComposite.getChildren()) {
+ control.dispose();
+ }
+
+ // Get configuration options
+ org.apache.hop.core.config.HopConfig hopConfig =
+ org.apache.hop.core.config.HopConfig.getInstance();
+ boolean showReadNotifications =
+ org.apache.hop.core.config.HopConfig.readOptionString(
+ "notification.showReadNotifications", "true")
+ .equalsIgnoreCase("true");
+ String daysToGoBackStr =
+ org.apache.hop.core.config.HopConfig.readOptionString(
+ "notification.global.daysToGoBack", "30");
+ int daysToGoBack = 0;
+ try {
+ daysToGoBack = Integer.parseInt(daysToGoBackStr);
+ } catch (NumberFormatException e) {
+ daysToGoBack = 30; // Default to 30 days
+ }
+
+ // Get provider errors and notifications
+ List<org.apache.hop.ui.hopgui.notifications.ProviderErrorInfo> providerErrors =
+ NotificationService.getInstance().getProviderErrors();
+ List<Notification> notifications =
+ NotificationService.getInstance().getNotifications(!showReadNotifications, daysToGoBack);
Review Comment:
**[bug]** The source dialog validates and stores per-source `daysToGoBack` (`NotificationSourceDialog.saveSource`), and the field is labeled "0 = use global". The panel always calls `getNotifications(..., daysToGoBack)` with only `notification.global.daysToGoBack`. Factory/providers never read the per-source property either, so the control is dead: a source set to 7 days still uses the global 30.
**Suggestion:** When listing, apply `max(source.daysToGoBack, 0)` per notification (0 meaning global), or remove the per-source field from the dialog until it is wired through.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/notifications/NotificationPanel.java:
##########
@@ -0,0 +1,1002 @@
+/*
+ * 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 java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.List;
+import org.apache.hop.core.logging.LogChannel;
+import org.apache.hop.core.notifications.Notification;
+import org.apache.hop.i18n.BaseMessages;
+import org.apache.hop.ui.core.PropsUi;
+import org.apache.hop.ui.core.gui.GuiResource;
+import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.hopgui.ISingletonProvider;
+import org.apache.hop.ui.hopgui.ImplementationLoader;
+import org.apache.hop.ui.hopgui.perspective.configuration.ConfigurationPerspective;
+import org.apache.hop.ui.util.EnvironmentUtils;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.SWTException;
+import org.eclipse.swt.custom.CLabel;
+import org.eclipse.swt.custom.ScrolledComposite;
+import org.eclipse.swt.events.SelectionAdapter;
+import org.eclipse.swt.events.SelectionEvent;
+import org.eclipse.swt.graphics.Color;
+import org.eclipse.swt.graphics.Point;
+import org.eclipse.swt.layout.FormAttachment;
+import org.eclipse.swt.layout.FormData;
+import org.eclipse.swt.layout.FormLayout;
+import org.eclipse.swt.widgets.Button;
+import org.eclipse.swt.widgets.Canvas;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Label;
+import org.eclipse.swt.widgets.Listener;
+import org.eclipse.swt.widgets.Shell;
+
+/** Dropdown panel for displaying notifications */
+public class NotificationPanel implements INotificationListener {
+ private static final Class<?> PKG = NotificationPanel.class;
+
+ /** How many notifications the panel draws before it stops and says how many are left. */
+ private static final int MAX_RENDERED_NOTIFICATIONS = 100;
+
+ private static NotificationPanel fallback;
+
+ private static final ISingletonProvider PROVIDER = loadProvider();
+
+ private static ISingletonProvider loadProvider() {
+ try {
+ return (ISingletonProvider) ImplementationLoader.newInstance(NotificationPanel.class);
+ } catch (Throwable e) {
+ // hop-ui unit tests have no rcp/rap *Impl on the classpath. Anywhere else this is a
+ // misconfiguration worth shouting about: one instance would then be shared by every Hop Web
+ // session, which is the very thing the per-session provider exists to prevent.
+ LogChannel.GENERAL.logBasic(
+ "No NotificationPanelImpl found; falling back to a single instance for this process. "
+ + "In Hop Web that means every session shares one.");
+ return () -> {
+ synchronized (NotificationPanel.class) {
+ if (fallback == null) {
+ fallback = new NotificationPanel();
+ }
+ return fallback;
+ }
+ };
+ }
+ }
+
+ private Shell shell;
+ private Shell parentShell;
+ private ScrolledComposite scrolledComposite;
+ private Composite contentComposite;
+ private boolean isVisible = false;
+
+ /** The configured sources, refreshed each time the list is drawn, for the source colours. */
+ private java.util.List<org.apache.hop.ui.hopgui.notifications.config.NotificationSourceConfig>
+ sourcesForRender = new java.util.ArrayList<>();
+
+ /** Use {@link #getInstance()}. Public so RWT can create one per user session in Hop Web. */
+ public NotificationPanel() {
+ this.parentShell = HopGui.getInstance().getShell();
+ NotificationService.getInstance().addNotificationListener(this);
+ }
+
+ /**
+ * @return The notification panel of this process, or of this user's session in Hop Web
+ */
+ public static NotificationPanel getInstance() {
+ return (NotificationPanel) PROVIDER.getInstanceInternal();
+ }
+
+ /** Toggle the panel visibility */
+ public void toggle() {
+ if (isVisible) {
+ hide();
+ } else {
+ show();
+ }
+ }
+
+ /** Show the notification panel */
+ public void show() {
+ if (shell != null && !shell.isDisposed()) {
+ // Panel already exists, refresh notifications and show
+ updateNotifications();
+ shell.setVisible(true);
+ shell.setFocus();
+ // Without this the panel stays "not visible" after the first open and close, which stops
+ // notificationsChanged() from refreshing it and leaves the bell unable to close it again.
+ isVisible = true;
+ return;
+ }
+
+ createPanel();
+ updateNotifications();
+ positionPanel();
+ shell.setVisible(true);
+ isVisible = true;
+ }
+
+ /** Hide the notification panel */
+ public void hide() {
+ if (shell != null && !shell.isDisposed()) {
+ shell.setVisible(false);
+ }
+ isVisible = false;
+ }
+
+ /** Create the panel UI */
+ private void createPanel() {
+ // Use DIALOG_TRIM instead of ON_TOP to keep it attached to parent
+ // Remove ON_TOP so it doesn't stay on top when switching applications
+ shell = new Shell(parentShell, SWT.DIALOG_TRIM | SWT.RESIZE);
+ shell.setLayout(new FormLayout());
+ PropsUi.setLook(shell);
+
+ // Header
+ Composite header = new Composite(shell, SWT.NONE);
+ header.setLayout(new FormLayout());
+ PropsUi.setLook(header);
+ FormData fdHeader = new FormData();
+ fdHeader.left = new FormAttachment(0, 0);
+ fdHeader.right = new FormAttachment(100, 0);
+ fdHeader.top = new FormAttachment(0, 0);
+ header.setLayoutData(fdHeader);
+
+ // Settings button
+ Button settingsButton = new Button(header, SWT.PUSH);
+ settingsButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Settings"));
+ settingsButton.setToolTipText(
+ BaseMessages.getString(PKG, "NotificationPanel.Settings.Tooltip"));
+ PropsUi.setLook(settingsButton);
+ FormData fdSettings = new FormData();
+ fdSettings.right = new FormAttachment(100, -10);
+ fdSettings.top = new FormAttachment(0, 5);
+ fdSettings.bottom = new FormAttachment(100, -5);
+ settingsButton.setLayoutData(fdSettings);
+ settingsButton.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ org.apache.hop.ui.hopgui.perspective.configuration.ConfigurationPerspective
+ configPerspective = HopGui.getConfigurationPerspective();
+ if (configPerspective != null) {
+ HopGui.getInstance().setActivePerspective(configPerspective);
+
+ // Defer tab/tree selection until perspective is fully activated
+ Display.getCurrent()
+ .asyncExec(
+ () -> {
+ ConfigurationPerspective perspective = HopGui.getConfigurationPerspective();
+ if (perspective != null) {
+ perspective.showNotificationsTab();
+ }
+ });
+ }
+ }
+ });
+
+ Button clearAll = new Button(header, SWT.PUSH);
+ clearAll.setText(BaseMessages.getString(PKG, "NotificationPanel.ClearAll"));
+ clearAll.setToolTipText(BaseMessages.getString(PKG, "NotificationPanel.ClearAll.Tooltip"));
+ PropsUi.setLook(clearAll);
+ FormData fdClearAll = new FormData();
+ fdClearAll.right = new FormAttachment(settingsButton, -10);
+ fdClearAll.top = new FormAttachment(0, 5);
+ fdClearAll.bottom = new FormAttachment(100, -5);
+ clearAll.setLayoutData(fdClearAll);
+ clearAll.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ NotificationService.getInstance().clearAll();
+ updateNotifications();
+ }
+ });
+
+ Button markAllRead = new Button(header, SWT.PUSH);
+ markAllRead.setText(BaseMessages.getString(PKG, "NotificationPanel.MarkAllRead"));
+ PropsUi.setLook(markAllRead);
+ FormData fdMarkAll = new FormData();
+ fdMarkAll.right = new FormAttachment(clearAll, -10);
+ fdMarkAll.top = new FormAttachment(0, 5);
+ fdMarkAll.bottom = new FormAttachment(100, -5);
+ markAllRead.setLayoutData(fdMarkAll);
+ markAllRead.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ NotificationService.getInstance().markAllAsRead();
+ updateNotifications();
+ }
+ });
+
+ // Scrolled content area
+ scrolledComposite = new ScrolledComposite(shell, SWT.V_SCROLL | SWT.BORDER);
+ PropsUi.setLook(scrolledComposite);
+ FormData fdScrolled = new FormData();
+ fdScrolled.left = new FormAttachment(0, 0);
+ fdScrolled.right = new FormAttachment(100, 0);
+ fdScrolled.top = new FormAttachment(header, 0);
+ fdScrolled.bottom = new FormAttachment(100, -40);
+ scrolledComposite.setLayoutData(fdScrolled);
+
+ contentComposite = new Composite(scrolledComposite, SWT.NONE);
+ contentComposite.setLayout(new FormLayout());
+ PropsUi.setLook(contentComposite);
+ scrolledComposite.setContent(contentComposite);
+ scrolledComposite.setExpandHorizontal(true);
+ scrolledComposite.setExpandVertical(true);
+
+ // Footer
+ Composite footer = new Composite(shell, SWT.NONE);
+ footer.setLayout(new FormLayout());
+ PropsUi.setLook(footer);
+ FormData fdFooter = new FormData();
+ fdFooter.left = new FormAttachment(0, 0);
+ fdFooter.right = new FormAttachment(100, 0);
+ fdFooter.bottom = new FormAttachment(100, 0);
+ footer.setLayoutData(fdFooter);
+
+ Button closeButton = new Button(footer, SWT.PUSH);
+ closeButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Close"));
+ PropsUi.setLook(closeButton);
+ FormData fdClose = new FormData();
+ fdClose.right = new FormAttachment(100, -10);
+ fdClose.top = new FormAttachment(0, 5);
+ fdClose.bottom = new FormAttachment(100, -5);
+ closeButton.setLayoutData(fdClose);
+ closeButton.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ hide();
+ }
+ });
+
+ // Close when clicking outside
+ shell.addListener(
+ SWT.Deactivate,
+ e -> {
+ if (!shell.isDisposed()) {
+ Display.getCurrent()
+ .asyncExec(
+ () -> {
+ if (!shell.isDisposed() && !shell.isFocusControl()) {
+ hide();
+ }
+ });
+ }
+ });
+
+ // The panel hangs off the bell in the main toolbar, so it has to follow the main window
+ // whenever that moves or is resized, not just when it is resized.
+ if (parentShell != null && !parentShell.isDisposed()) {
+ Listener repositionListener =
+ e -> {
+ if (shell != null && !shell.isDisposed() && isVisible) {
+ Display.getCurrent()
+ .asyncExec(
+ () -> {
+ if (shell != null && !shell.isDisposed() && isVisible) {
+ positionPanel();
+ }
+ });
+ }
+ };
+ parentShell.addListener(SWT.Resize, repositionListener);
+ parentShell.addListener(SWT.Move, repositionListener);
+ }
+
+ shell.setSize(400, 500);
+
+ // Add listener to shell resize to ensure titles truncate properly
+ shell.addListener(
+ SWT.Resize,
+ e -> {
+ if (contentComposite != null && !contentComposite.isDisposed()) {
+ // Force layout update to ensure titles truncate correctly
+ contentComposite.layout(true, true);
+ }
+ });
+ }
+
+ /** Update the notifications display */
+ private void updateNotifications() {
+ if (contentComposite == null || contentComposite.isDisposed()) {
+ return;
+ }
+
+ // Clear existing notifications
+ for (Control control : contentComposite.getChildren()) {
+ control.dispose();
+ }
+
+ // Get configuration options
+ org.apache.hop.core.config.HopConfig hopConfig =
+ org.apache.hop.core.config.HopConfig.getInstance();
+ boolean showReadNotifications =
+ org.apache.hop.core.config.HopConfig.readOptionString(
+ "notification.showReadNotifications", "true")
+ .equalsIgnoreCase("true");
+ String daysToGoBackStr =
+ org.apache.hop.core.config.HopConfig.readOptionString(
+ "notification.global.daysToGoBack", "30");
+ int daysToGoBack = 0;
+ try {
+ daysToGoBack = Integer.parseInt(daysToGoBackStr);
+ } catch (NumberFormatException e) {
+ daysToGoBack = 30; // Default to 30 days
+ }
+
+ // Get provider errors and notifications
+ List<org.apache.hop.ui.hopgui.notifications.ProviderErrorInfo> providerErrors =
+ NotificationService.getInstance().getProviderErrors();
+ List<Notification> notifications =
+ NotificationService.getInstance().getNotifications(!showReadNotifications, daysToGoBack);
+ // Read once per repaint: getSourceColor runs twice for every notification on screen.
+ sourcesForRender = org.apache.hop.ui.hopgui.notifications.config.NotificationSources.load();
+
+ Control lastControl = null;
+
+ // Provider error banner
+ if (!providerErrors.isEmpty()) {
+ Composite errorBanner = createProviderErrorBanner(providerErrors, lastControl);
+ lastControl = errorBanner;
+ }
+
+ if (notifications.isEmpty() && lastControl == null) {
+ Label emptyLabel = new Label(contentComposite, SWT.CENTER | SWT.WRAP);
+ emptyLabel.setText(BaseMessages.getString(PKG, "NotificationPanel.NoNotifications"));
+ PropsUi.setLook(emptyLabel);
+ FormData fdEmpty = new FormData();
+ fdEmpty.left = new FormAttachment(0, 10);
+ fdEmpty.right = new FormAttachment(100, -10);
+ fdEmpty.top = new FormAttachment(0, 20);
+ emptyLabel.setLayoutData(fdEmpty);
+ } else if (!notifications.isEmpty()) {
+ // Every notification becomes a small stack of widgets. Beyond a screenful or two nobody
+ // scrolls anyway, and building hundreds of them is what makes opening the panel feel slow.
+ int shown = Math.min(notifications.size(), MAX_RENDERED_NOTIFICATIONS);
+ for (Notification notification : notifications.subList(0, shown)) {
+ try {
+ Composite notifComposite = createNotificationItem(notification, lastControl);
+ lastControl = notifComposite;
+ } catch (Exception e) {
+ // Log error but continue with other notifications
+ LogChannel.UI.logError("Error creating notification item: " + notification.getTitle(), e);
+ }
+ }
+ if (notifications.size() > shown) {
+ Label moreLabel = new Label(contentComposite, SWT.CENTER | SWT.WRAP);
+ moreLabel.setText(
+ BaseMessages.getString(
+ PKG,
+ "NotificationPanel.MoreNotifications",
+ Integer.toString(notifications.size() - shown)));
+ PropsUi.setLook(moreLabel);
+ FormData fdMore = new FormData();
+ fdMore.left = new FormAttachment(0, 10);
+ fdMore.right = new FormAttachment(100, -10);
+ fdMore.top = new FormAttachment(lastControl, 10);
+ moreLabel.setLayoutData(fdMore);
+ lastControl = moreLabel;
+ }
+ } else if (lastControl != null && notifications.isEmpty()) {
+ // Errors only, no notifications
+ Label emptyLabel = new Label(contentComposite, SWT.CENTER | SWT.WRAP);
+ emptyLabel.setText(BaseMessages.getString(PKG, "NotificationPanel.NoNotifications"));
+ PropsUi.setLook(emptyLabel);
+ FormData fdEmpty = new FormData();
+ fdEmpty.left = new FormAttachment(0, 10);
+ fdEmpty.right = new FormAttachment(100, -10);
+ fdEmpty.top = new FormAttachment(lastControl, 10);
+ emptyLabel.setLayoutData(fdEmpty);
+ }
+
+ // Force layout of content composite and scrolled composite
+ if (contentComposite != null && !contentComposite.isDisposed()) {
+ // Get the scrolled composite width first to ensure proper sizing
+ int availableWidth = SWT.DEFAULT;
+ if (scrolledComposite != null && !scrolledComposite.isDisposed()) {
+ org.eclipse.swt.graphics.Rectangle scrolledBounds = scrolledComposite.getBounds();
+ if (scrolledBounds.width > 0) {
+ availableWidth = scrolledBounds.width - 20; // Account for margins
+ }
+ }
+
+ // Layout scrolled composite first to get its actual width
+ if (scrolledComposite != null && !scrolledComposite.isDisposed()) {
+ scrolledComposite.layout(true, false);
+ org.eclipse.swt.graphics.Rectangle scrolledBounds = scrolledComposite.getBounds();
+ if (scrolledBounds.width > 0) {
+ availableWidth = scrolledBounds.width - 20; // Account for margins
+ } else {
+ // Fallback: use shell width if scrolled composite not sized yet
+ if (shell != null && !shell.isDisposed()) {
+ availableWidth = shell.getSize().x > 0 ? shell.getSize().x - 40 : 380;
+ } else {
+ availableWidth = 380; // Default width
+ }
+ }
+ }
+
+ // Layout content composite with proper width constraint
+ // This is critical for SWT.WRAP labels to calculate their height
+ contentComposite.layout(true, true);
+
+ // Compute size with width constraint for proper wrapping
+ org.eclipse.swt.graphics.Point contentSize =
+ contentComposite.computeSize(availableWidth, SWT.DEFAULT);
+
+ if (scrolledComposite != null && !scrolledComposite.isDisposed()) {
+ scrolledComposite.setMinSize(contentSize);
+ scrolledComposite.layout(true, true);
+ }
+ }
+ }
+
+ /** Create the provider error banner with Retry button */
+ private Composite createProviderErrorBanner(
+ List<org.apache.hop.ui.hopgui.notifications.ProviderErrorInfo> errors, Control above) {
+ Composite banner = new Composite(contentComposite, SWT.BORDER);
+ FormLayout bannerLayout = new FormLayout();
+ // The error text wraps to several lines; without a bottom margin the last one sits on the
+ // border, because nothing attaches the final label to the bottom of the banner.
+ bannerLayout.marginBottom = 10;
+ banner.setLayout(bannerLayout);
+ // Look first, then the banner's own colours: setLook applies the theme's foreground, which on
+ // a dark theme is near white and left this text unreadable on the light background. Both
+ // colours are set explicitly so the banner reads the same whichever theme is in use.
+ PropsUi.setLook(banner);
+ Color bannerBackground = GuiResource.getInstance().getColor(255, 248, 220); // Light yellow
+ Color bannerForeground = GuiResource.getInstance().getColor(60, 50, 20); // Dark brown
+ banner.setBackground(bannerBackground);
+ banner.setForeground(bannerForeground);
+
+ FormData fdBanner = new FormData();
+ fdBanner.left = new FormAttachment(0, 0);
+ fdBanner.right = new FormAttachment(100, 0);
+ fdBanner.top = above != null ? new FormAttachment(above, 10) : new FormAttachment(0, 10);
+ banner.setLayoutData(fdBanner);
+
+ Label headerLabel = new Label(banner, SWT.WRAP);
+ headerLabel.setText(BaseMessages.getString(PKG, "NotificationPanel.ProviderErrors"));
+ PropsUi.setLook(headerLabel);
+ headerLabel.setBackground(bannerBackground);
+ headerLabel.setForeground(bannerForeground);
+ FormData fdHeader = new FormData();
+ fdHeader.left = new FormAttachment(0, 10);
+ fdHeader.right = new FormAttachment(100, -80);
+ fdHeader.top = new FormAttachment(0, 10);
+ headerLabel.setLayoutData(fdHeader);
+
+ Button retryButton = new Button(banner, SWT.PUSH);
+ retryButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Retry"));
+ retryButton.setToolTipText(BaseMessages.getString(PKG, "NotificationPanel.Retry.Tooltip"));
+ PropsUi.setLook(retryButton);
+ FormData fdRetry = new FormData();
+ fdRetry.right = new FormAttachment(100, -10);
+ fdRetry.top = new FormAttachment(0, 5);
+ retryButton.setLayoutData(fdRetry);
+ retryButton.addSelectionListener(
+ new SelectionAdapter() {
+ @Override
+ public void widgetSelected(SelectionEvent e) {
+ // Fetches in the background; the panel refreshes through notificationsChanged().
+ NotificationService.getInstance().retryNow();
+ }
+ });
+
+ Control lastLine = headerLabel;
+ for (org.apache.hop.ui.hopgui.notifications.ProviderErrorInfo err : errors) {
+ String text =
+ BaseMessages.getString(
+ PKG, "NotificationPanel.ProviderErrorItem", err.getProviderName(), err.getMessage());
+ Label line = new Label(banner, SWT.WRAP);
+ line.setText(text);
+ PropsUi.setLook(line);
+ line.setBackground(bannerBackground);
+ line.setForeground(bannerForeground);
+ FormData fdLine = new FormData();
+ fdLine.left = new FormAttachment(0, 10);
+ fdLine.right = new FormAttachment(100, -10);
+ fdLine.top = new FormAttachment(lastLine, 5);
+ line.setLayoutData(fdLine);
+ lastLine = line;
+ }
+
+ return banner;
+ }
+
+ /** Create a notification item UI */
+ private Composite createNotificationItem(Notification notification, Control above) {
+ Composite composite = new Composite(contentComposite, SWT.BORDER);
+ composite.setLayout(new FormLayout());
+ PropsUi.setLook(composite);
+
+ GuiResource guiResource = GuiResource.getInstance();
+
+ // Set background color based on read state and priority
+ // Note: We'll update this dynamically when notification is marked as read
+ updateNotificationBackground(composite, notification, guiResource);
+
+ // Set FormData for positioning in parent
+ FormData fdComposite = new FormData();
+ fdComposite.left = new FormAttachment(0, 0);
+ fdComposite.right = new FormAttachment(100, 0);
+ if (above != null) {
+ fdComposite.top = new FormAttachment(above, 5);
+ } else {
+ fdComposite.top = new FormAttachment(0, 5);
+ }
+ // Don't set bottom - composite will size to its children
+ composite.setLayoutData(fdComposite);
+
+ // Priority indicator (colored bar on the left)
+ Composite priorityBar = new Composite(composite, SWT.NONE);
+ priorityBar.setLayout(null);
+ priorityBar.setData("type", "priorityBar"); // Mark for later updates
+ FormData fdPriorityBar = new FormData();
+ fdPriorityBar.left = new FormAttachment(0, 0);
+ fdPriorityBar.top = new FormAttachment(0, 0);
+ fdPriorityBar.bottom = new FormAttachment(100, 0);
+ fdPriorityBar.width = 4;
+ priorityBar.setLayoutData(fdPriorityBar);
+
+ // Set initial priority bar color (will be updated when read state changes)
+ updatePriorityBar(priorityBar, notification, guiResource);
+
+ // Source color indicator (small colored square) - positioned on the left, after priority bar
+ // A Canvas, not a plain Composite: the border below is drawn in a paint listener, and RAP
+ // only offers one on Canvas. On a Composite this compiles against desktop SWT and fails in
+ // Hop Web with NoSuchMethodError, taking the whole notification list down with it.
+ Canvas sourceIndicator = new Canvas(composite, SWT.NONE);
+ sourceIndicator.setLayout(null);
+ sourceIndicator.setData("type", "sourceIndicator"); // Mark to exclude from click handling
+ PropsUi.setLook(sourceIndicator);
+ // Get color for this source from configuration
+ org.eclipse.swt.graphics.Color sourceColor = getSourceColor(notification, guiResource);
+ sourceIndicator.setBackground(sourceColor);
+ FormData fdSourceIndicator = new FormData();
+ // Position after priority bar, 8px gap
+ fdSourceIndicator.left = new FormAttachment(priorityBar, 8);
+ fdSourceIndicator.top = new FormAttachment(0, 10);
+ fdSourceIndicator.width = 12; // Fixed width
+ fdSourceIndicator.height = 12; // Fixed height
+ sourceIndicator.setLayoutData(fdSourceIndicator);
+
+ // Add a PaintListener to draw a border for better visibility
+ sourceIndicator.addPaintListener(
+ e -> {
+ org.eclipse.swt.graphics.Rectangle bounds = sourceIndicator.getBounds();
+ e.gc.setForeground(guiResource.getColorDarkGray());
+ e.gc.setLineWidth(1);
+ e.gc.drawRectangle(0, 0, bounds.width - 1, bounds.height - 1);
+ });
+
+ // Tooltip with source name and URL (source name shown in tooltip, not as text)
+ String tooltipText = buildSourceTooltip(notification);
+ sourceIndicator.setToolTipText(tooltipText);
+
+ // Title - use CLabel for automatic ellipsis truncation
+ // Title starts after source indicator
+ CLabel titleLabel = new CLabel(composite, SWT.LEFT);
+ String fullTitle = notification.getTitle() != null ? notification.getTitle() : "";
+ titleLabel.setText(fullTitle);
+ titleLabel.setData("type", "title"); // Mark for later updates
+ titleLabel.setData("fullTitle", fullTitle); // Store full title for tooltip
+ PropsUi.setLook(titleLabel);
+ if (!notification.isRead()) {
+ titleLabel.setFont(guiResource.getFontBold());
+ }
+ // Set tooltip to show full title if truncated
+ titleLabel.setToolTipText(fullTitle);
+ FormData fdTitle = new FormData();
+ // Title starts after source indicator with 8px gap
+ fdTitle.left = new FormAttachment(sourceIndicator, 8);
+ // Title extends to right edge with margin
+ fdTitle.right = new FormAttachment(100, -10);
+ fdTitle.top = new FormAttachment(0, 10);
+ // Don't attach bottom - CLabel will size to its preferred height
+ titleLabel.setLayoutData(fdTitle);
+
+ // Timestamp between title and body (always visible)
+ Label timeLabel = new Label(composite, SWT.NONE);
+ String timeText = formatTimestamp(notification.getTimestamp());
+ if (timeText == null || timeText.isEmpty()) {
+ timeText = "Unknown date";
+ }
+ timeLabel.setText(timeText);
+ PropsUi.setLook(timeLabel);
+ timeLabel.setForeground(guiResource.getColorDarkGray());
+ FormData fdTime = new FormData();
+ fdTime.left = new FormAttachment(priorityBar, 10);
+ // Timestamp extends to right edge of composite
+ fdTime.right = new FormAttachment(100, -10);
+ fdTime.top = new FormAttachment(titleLabel, 5);
+ // Don't attach bottom - label will size to its preferred height
+ timeLabel.setLayoutData(fdTime);
+
+ // Message/Description - simplified, truncated to max 3-5 lines
+ Label messageLabel = null;
+ String message = notification.getMessage();
+
+ if (message != null && !message.isEmpty()) {
+ // Limit to approximately 3-5 lines (roughly 200-300 characters)
+ // Simple truncation - just show start of message
+ int maxLength = 250;
+ String displayMessage = message;
+ if (displayMessage.length() > maxLength) {
+ displayMessage = displayMessage.substring(0, maxLength).trim() + "...";
+ }
+ messageLabel = new Label(composite, SWT.WRAP);
+ messageLabel.setText(displayMessage);
+ PropsUi.setLook(messageLabel);
+ FormData fdMessage = new FormData();
+ fdMessage.left = new FormAttachment(priorityBar, 10);
+ // Message extends to right edge of composite (sourceIndicator is on left, so don't constrain
+ // by it)
+ fdMessage.right = new FormAttachment(100, -10);
+ fdMessage.top = new FormAttachment(timeLabel, 5);
+ // Don't attach bottom - label will wrap and size to its content
+ messageLabel.setLayoutData(fdMessage);
+
+ // Set default cursor (not clickable)
+ messageLabel.setCursor(
+ composite.getDisplay().getSystemCursor(org.eclipse.swt.SWT.CURSOR_ARROW));
+ }
+
+ // Force layout of this composite to ensure all children are properly sized
+ composite.layout(true, true);
+
+ // Set cursor to pointer to indicate clickability for entire notification area
+ // Store reference to notification ID and guiResource for updates
+ composite.setData("notificationId", notification.getId());
+ composite.setData("guiResource", guiResource);
+
+ // Set cursor behavior: title and composite are clickable, body and timestamp are not
+ org.eclipse.swt.graphics.Cursor handCursor =
+ composite.getDisplay().getSystemCursor(org.eclipse.swt.SWT.CURSOR_HAND);
+ org.eclipse.swt.graphics.Cursor defaultCursor =
+ composite.getDisplay().getSystemCursor(org.eclipse.swt.SWT.CURSOR_ARROW);
+
+ // Title is clickable - use hand cursor
+ titleLabel.setCursor(handCursor);
+
+ // Body (message) and timestamp are NOT clickable - use default cursor
+ if (messageLabel != null) {
+ messageLabel.setCursor(defaultCursor);
+ }
+ timeLabel.setCursor(defaultCursor);
+
+ // Composite itself is clickable (for clicking outside title but still on notification)
+ composite.setCursor(handCursor);
+
+ // Click handler - attach to composite and all child controls
+ org.eclipse.swt.widgets.Listener clickListener =
+ e -> {
+ // Mark as read first - this updates the notification in the service
+ NotificationService.getInstance().markAsRead(notification.getId());
+
+ // Get fresh notification from service to ensure we have updated state
+ List<Notification> allNotifications =
+ NotificationService.getInstance().getNotifications(false);
+ Notification updatedNotification =
+ allNotifications.stream()
+ .filter(n -> notification.getId().equals(n.getId()))
+ .findFirst()
+ .orElse(notification);
+
+ // Ensure it's marked as read (should already be, but be safe)
+ updatedNotification.setRead(true);
+
+ // Update visual state immediately
+ updateNotificationBackground(composite, updatedNotification, guiResource);
+
+ // Force redraw to ensure visual changes are visible
+ composite.redraw();
+
+ // Open link if available. NotificationService drops links it will not open, but the
+ // link is handed to the operating system here, so it is checked again at the click.
+ String link = updatedNotification.getLink();
+ if (link != null && !link.isEmpty()) {
+ if (NotificationLinks.isSafe(link)) {
+ try {
+ EnvironmentUtils.getInstance().openUrl(link);
+ } catch (Exception ex) {
+ LogChannel.UI.logError("Error opening notification link " + link, ex);
+ }
+ } else {
+ LogChannel.UI.logBasic(
+ "Refusing to open notification link "
+ + link
+ + ": only http and https are opened");
+ }
+ }
+ };
+
+ // Attach click handler to composite
+ composite.addListener(SWT.MouseDown, clickListener);
+
+ // Also attach to all child controls to make entire area clickable
+ attachClickListenerRecursive(composite, clickListener);
+
+ return composite;
+ }
+
+ /** Update notification background based on read state and priority */
+ private void updateNotificationBackground(
+ Composite composite, Notification notification, GuiResource guiResource) {
+ if (notification.isRead()) {
+ // Read notifications have default background
+ composite.setBackground(null);
+ } else {
+ // Unread notifications have colored background based on priority
+ if (notification.getPriority() != null) {
+ switch (notification.getPriority()) {
+ case ERROR:
+ composite.setBackground(guiResource.getColor(255, 240, 240)); // Light red tint
+ break;
+ case WARNING:
+ composite.setBackground(guiResource.getColor(255, 250, 240)); // Light yellow tint
+ break;
+ case INFO:
+ default:
+ composite.setBackground(guiResource.getColorLightGray());
+ break;
+ }
+ } else {
+ composite.setBackground(guiResource.getColorLightGray());
+ }
+ }
+
+ // Update priority bar color based on read state
+ Control[] children = composite.getChildren();
+ for (Control child : children) {
+ if (child instanceof Composite) {
+ Object type = child.getData("type");
+ if ("priorityBar".equals(type)) {
+ updatePriorityBar((Composite) child, notification, guiResource);
+ }
+ }
+ // Check for title label (can be CLabel or Label)
+ Object type = child.getData("type");
+ if ("title".equals(type)) {
+ if (child instanceof CLabel) {
+ CLabel titleLabel = (CLabel) child;
+ if (notification.isRead()) {
+ // Use default font (remove bold)
+ titleLabel.setFont(null);
+ } else {
+ titleLabel.setFont(guiResource.getFontBold());
+ }
+ } else if (child instanceof Label) {
+ Label titleLabel = (Label) child;
+ if (notification.isRead()) {
+ // Use default font (remove bold)
+ titleLabel.setFont(null);
+ } else {
+ titleLabel.setFont(guiResource.getFontBold());
+ }
+ }
+ }
+ }
+ }
+
+ /** Update priority bar color based on read state */
+ private void updatePriorityBar(
+ Composite priorityBar, Notification notification, GuiResource guiResource) {
+ if (notification.isRead()) {
+ // Read notifications have gray priority bar
+ priorityBar.setBackground(guiResource.getColorGray());
+ } else {
+ // Unread notifications use the source color from configuration
+ org.eclipse.swt.graphics.Color sourceColor = getSourceColor(notification, guiResource);
+ priorityBar.setBackground(sourceColor);
+ }
+ }
+
+ /** Recursively attach click listener to composite and all its children */
+ private void attachClickListenerRecursive(
+ Control control, org.eclipse.swt.widgets.Listener listener) {
+ if (control == null || control.isDisposed()) {
+ return;
+ }
+ // Don't attach to the priority bar or source indicator (they're just visual indicators)
+ Object type = control.getData("type");
+ if (!"priorityBar".equals(type) && !"sourceIndicator".equals(type)) {
+ // Only attach to leaf controls (Labels, etc.) to avoid duplicate events
+ // The composite already has the listener, so we don't need to attach to child composites
+ if (!(control instanceof Composite)) {
+ control.addListener(SWT.MouseDown, listener);
+ }
+ }
+ if (control instanceof Composite) {
+ Composite composite = (Composite) control;
+ for (Control child : composite.getChildren()) {
+ attachClickListenerRecursive(child, listener);
+ }
+ }
+ }
+
+ /**
+ * Get color for a notification source. This will be configurable via ConfigOption later. For now,
+ * uses a simple hash-based color scheme.
+ */
+ private org.eclipse.swt.graphics.Color getSourceColor(
+ Notification notification, GuiResource guiResource) {
+ // Try to get color from notification source configuration
+ String sourceId = notification.getSourceId();
+ if (sourceId != null && !sourceId.isEmpty()) {
+ for (org.apache.hop.ui.hopgui.notifications.config.NotificationSourceConfig source :
+ sourcesForRender) {
+ if (sourceId.equals(source.getId())) {
+ String colorHex = source.getColor();
+ if (colorHex != null && !colorHex.isEmpty()) {
+ try {
+ String hex = colorHex.startsWith("#") ? colorHex.substring(1) : colorHex;
+ int colorValue = Integer.parseInt(hex, 16);
+ return guiResource.getColor(
+ (colorValue >> 16) & 0xFF, (colorValue >> 8) & 0xFF, colorValue & 0xFF);
+ } catch (NumberFormatException e) {
+ // Not a colour we can read; fall through to one derived from the source name.
+ }
+ }
+ break;
+ }
+ }
+ }
+
+ // Fallback: hash-based color generation for consistent colors per source
+ String source = notification.getSource();
+ if (source == null || source.isEmpty()) {
+ return guiResource.getColorGray();
+ }
+
+ int hash = source.hashCode();
+ int r = Math.abs(hash % 200) + 50; // 50-250 range
+ int g = Math.abs((hash >> 8) % 200) + 50;
+ int b = Math.abs((hash >> 16) % 200) + 50;
+
+ return guiResource.getColor(r, g, b);
+ }
+
+ /** Build tooltip text for source indicator showing source name and URL */
+ private String buildSourceTooltip(Notification notification) {
+ StringBuilder tooltip = new StringBuilder();
+ if (notification.getSource() != null && !notification.getSource().isEmpty()) {
+ tooltip.append("Source: ").append(notification.getSource());
+ }
+ if (notification.getLink() != null && !notification.getLink().isEmpty()) {
+ if (tooltip.length() > 0) {
+ tooltip.append("\n");
+ }
+ tooltip.append("URL: ").append(notification.getLink());
+ }
+ return tooltip.length() > 0 ? tooltip.toString() : "Unknown source";
+ }
+
+ /** Format timestamp for display */
+ private String formatTimestamp(Date timestamp) {
+ if (timestamp == null) {
+ return "";
+ }
+ long diff = System.currentTimeMillis() - timestamp.getTime();
+ long minutes = diff / 60000;
+ long hours = diff / 3600000;
+ long days = diff / 86400000;
+
+ if (minutes < 1) {
+ return "Just now";
Review Comment:
**[suggestion]** Relative timestamps and source tooltips are hardcoded English (`"Just now"`, `" minute(s) ago"`, `"Source:"`, `"URL:"`) while the rest of the panel is in `messages_en_US.properties`.
**Suggestion:** Move these strings into the notification panel resource bundle (and handle pluralization).
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Hop Gui notification system #6188 (hop)
Posted by "bamaer (via GitHub)" <gi...@apache.org>.
bamaer commented on PR #8270:
URL: https://github.com/apache/hop/pull/8270#issuecomment-5559321062
Review feedback addressed in the three commits above: correctness and security fixes, the source dialog rewritten onto `@GuiWidgetElement`, and the unread bell registration below. Individual replies are on each thread.
Separate from the review points: while testing the above in Hop Web I found that the unread indicator on the notification bell never appears there. It reproduces on this branch before any of the review fixes, so it isn't a regression from them.
The cause isn't in the notification code, which is why reading those files doesn't reveal it. `HopWeb.configure()` registers the image each toolbar item *declares*, from the GUI registry. An icon that replaces it later has to be registered separately, or `RWT.getResourceManager().getLocation()` returns the location of a resource that was never registered: the `img src` 404s and the icon silently doesn't change, with no error on the server or in the browser. There's already a list of such alternates in `HopWeb` for the show/hide toggles, with a comment explaining exactly this — `notification-bell-unread.svg` was just never added to it.
Fixed by adding it to that list, and extended the comment so the next alternate icon doesn't hit the same thing. Verified in Hop Web: the resource now registers at startup and serves the SVG carrying the dot, and the indicator appears.
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org