Close underlying resources

[w3stling]

User description

This PR fixes some cases where the underlying resources are not closed properly.

If you use the iterator() or spliterator() methods, the underlying resources will not close automatically.

---

PR Type

Enhancement, Tests

---

Description

• Introduced AutoCloseStream and related classes to manage stream resources automatically, ensuring proper closure and resource management.
• Added StreamUtil class to facilitate stream operations, such as creating a stream from an iterator.
• Implemented comprehensive tests in CloseTest to verify the automatic closure of streams and resource management.
• Refactored existing tests to improve readability and maintainability, including updates to import paths and stream operations.

---

Changes walkthrough 📝

	Relevant files
Enhancement	7 files AbstractRssReader.java Implement AutoCloseStream for automatic resource management src/main/java/com/apptasticsoftware/rssreader/AbstractRssReader.java Introduced AutoCloseStream for automatic resource management. Replaced manual stream management with AutoCloseStream. Added AtomicBoolean for managing resource closure state. +52/-48  StreamUtil.java Add StreamUtil class for stream operations                              src/main/java/com/apptasticsoftware/rssreader/internal/StreamUtil.java Added utility class StreamUtil for stream operations. Provided method to create a stream from an iterator. +52/-0    AbstractAutoCloseStream.java Introduce AbstractAutoCloseStream for lifecycle management src/main/java/com/apptasticsoftware/rssreader/internal/stream/AbstractAutoCloseStream.java Introduced AbstractAutoCloseStream for managing stream lifecycle. Implemented automatic closure of streams. +80/-0    AutoCloseDoubleStream.java Implement AutoCloseDoubleStream for double stream management src/main/java/com/apptasticsoftware/rssreader/internal/stream/AutoCloseDoubleStream.java Implemented AutoCloseDoubleStream for double stream management. Ensured automatic closure of double streams. +227/-0  AutoCloseIntStream.java Implement AutoCloseIntStream for integer stream management src/main/java/com/apptasticsoftware/rssreader/internal/stream/AutoCloseIntStream.java Implemented AutoCloseIntStream for integer stream management. Ensured automatic closure of integer streams. +234/-0  AutoCloseLongStream.java Implement AutoCloseLongStream for long stream management  src/main/java/com/apptasticsoftware/rssreader/internal/stream/AutoCloseLongStream.java Implemented AutoCloseLongStream for long stream management. Ensured automatic closure of long streams. +230/-0  AutoCloseStream.java Implement AutoCloseStream for generic stream management    src/main/java/com/apptasticsoftware/rssreader/internal/stream/AutoCloseStream.java Implemented AutoCloseStream for generic stream management. Ensured automatic closure of generic streams. +255/-0
Tests	3 files CloseTest.java Add tests for automatic stream closure                                      src/test/java/com/apptasticsoftware/integrationtest/CloseTest.java Added comprehensive tests for stream closure. Verified automatic resource management in streams. +978/-0  SortTest.java Refactor SortTest for improved readability                              src/test/java/com/apptasticsoftware/integrationtest/SortTest.java Refactored test to use list of URLs. Simplified stream operations in tests. +11/-13  rss-feed.xml Add RSS feed XML for testing                                                          src/test/resources/rss-feed.xml Added RSS feed XML for testing purposes. +330/-0
Miscellaneous	2 files ConnectionTest.java Update import path for RssServer                                                  src/test/java/com/apptasticsoftware/integrationtest/ConnectionTest.java Updated import path for RssServer. +1/-1      RssServer.java Move RssServer to internal package                                              src/test/java/com/apptasticsoftware/rssreader/internal/RssServer.java Moved RssServer to internal package. +1/-1

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[github-actions]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Possible Bug The AutoCloseStream.of method is used to automatically close streams, but it's crucial to ensure that all underlying resources are properly closed, especially in error scenarios. The current implementation logs warnings but may not adequately handle resource leaks in all cases. Code Clarity The RssItemIterator class uses an AtomicBoolean to manage state, which is good for thread safety. However, the logic inside the close method could be simplified or better documented to explain why certain checks and operations are necessary. Redundancy The StreamUtil class provides a method to create a stream from an iterator, which is a straightforward utility. However, consider if this functionality is already provided by another library or Java itself, to avoid redundancy.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add null-check filtering to prevent potential NullPointerExceptions Consider handling the case where null values are added to extendedUrlList before processing it in the stream. This can prevent potential NullPointerException when methods like Item::getPubDateZonedDateTime are called on null items. src/test/java/com/apptasticsoftware/integrationtest/SortTest.java [49-54] List<String> extendedUrlList = new ArrayList<>(urlList); extendedUrlList.add(null); ... +.filter(Objects::nonNull) .map(Item::getPubDateZonedDateTime) Suggestion importance[1-10]: 9 Why: This suggestion addresses a potential bug by adding a null-check filter, which can prevent NullPointerExceptions when processing the list. This is a significant improvement for code robustness.	9
	Add exception handling to close the stream on errors To prevent potential resource leaks, it's crucial to ensure that the close() method is called on the underlying stream if an exception occurs during the execution of terminal operations. Implement try-catch blocks around stream operations that might throw exceptions. src/main/java/com/apptasticsoftware/rssreader/internal/stream/AutoCloseStream.java [131-135] public void forEach(Consumer<? super T> action) { autoClose(stream -> { - stream.forEach(action); + try { + stream.forEach(action); + } catch (Exception e) { + stream.close(); + throw e; + } return null; }); } Suggestion importance[1-10]: 8 Why: This suggestion addresses a potential resource leak by ensuring the stream is closed if an exception occurs during terminal operations, which is crucial for robust resource management.	8
Enhancement	Use a comparator for sorting by publication date Ensure that the sorted() method is called with a comparator when sorting items by their publication date. This is necessary because the default sorting may not work as expected without a proper comparator. src/test/java/com/apptasticsoftware/integrationtest/SortTest.java [52-57] var timestamps = new RssReader().read(extendedUrlList) - .sorted() + .sorted(Comparator.comparing(item -> item.getPubDateZonedDateTime().orElse(null))) .map(Item::getPubDateZonedDateTime) .flatMap(Optional::stream) .map(t -> t.toInstant().toEpochMilli()) .collect(Collectors.toList()); Suggestion importance[1-10]: 8 Why: The suggestion to use a comparator for sorting by publication date enhances the correctness of the sorting operation, ensuring that the items are sorted as intended. This is an important enhancement for functionality.	8
	Add checks for null and empty conditions before size assertion Consider verifying the non-null and non-empty state of timestamps before asserting its size to enhance test robustness. src/test/java/com/apptasticsoftware/integrationtest/SortTest.java [59] +assertNotNull(timestamps); +assertFalse(timestamps.isEmpty()); assertTrue(timestamps.size() > 200); Suggestion importance[1-10]: 7 Why: Adding checks for null and empty conditions before asserting the size enhances test robustness and reliability, ensuring that the test does not fail unexpectedly due to null or empty lists.	7
	Enhance onClose() to support chaining of close handlers The onClose() method should ensure that multiple close handlers can be registered and invoked in a chain. This is crucial for proper resource management, especially when streams are composed of multiple operations that each may require clean-up. src/main/java/com/apptasticsoftware/rssreader/internal/stream/AutoCloseStream.java [252-253] public Stream<T> onClose(Runnable closeHandler) { - return asAutoCloseStream(stream().onClose(closeHandler)); + return asAutoCloseStream(stream().onClose(() -> { + try { + closeHandler.run(); + } finally { + super.onClose(closeHandler); + } + })); } Suggestion importance[1-10]: 6 Why: The suggestion to support chaining of close handlers in onClose() improves resource management by allowing multiple handlers, although the current implementation may already suffice for many use cases.	6
	Use Optional for null checks and conditional logging Replace the manual null check with Optional to streamline the null handling and improve readability. src/main/java/com/apptasticsoftware/rssreader/AbstractRssReader.java [449-451] -if (LOGGER.isLoggable(Level.WARNING)) { - LOGGER.log(Level.WARNING, () -> String.format("Failed read URL %s. Message: %s", url, e.getMessage())); -} +Optional.ofNullable(LOGGER) + .filter(logger -> logger.isLoggable(Level.WARNING)) + .ifPresent(logger -> logger.log(Level.WARNING, () -> String.format("Failed read URL %s. Message: %s", url, e.getMessage()))); Suggestion importance[1-10]: 5 Why: Using Optional for null checks can improve readability, but the existing code does not perform a null check on LOGGER, making the suggestion partially relevant.	5
	Override parallel() and sequential() to maintain stream state Consider overriding the BaseStream.parallel() and BaseStream.sequential() methods to ensure that the state of the stream (parallel or sequential) is preserved when wrapping the stream. This will maintain consistency in the behavior of the stream when these methods are used. src/main/java/com/apptasticsoftware/rssreader/internal/stream/AutoCloseStream.java [242-243] +@Override public Stream<T> parallel() { + super.parallel(); return asAutoCloseStream(stream().parallel()); } +@Override public Stream<T> sequential() { + super.sequential(); return asAutoCloseStream(stream().sequential()); Suggestion importance[1-10]: 3 Why: The suggestion to override parallel() and sequential() methods with calls to super is unnecessary because the current implementation already correctly wraps the stream while maintaining its state. The suggestion does not provide a significant improvement.	3
Best practice	Use specific catch blocks for different exceptions in the close method Use a more specific exception handler for the XMLStreamException and IOException in the close method to provide more targeted error handling. src/main/java/com/apptasticsoftware/rssreader/AbstractRssReader.java [607-611] -} catch (XMLStreamException | IOException e) { - if (LOGGER.isLoggable(Level.WARNING)) { - LOGGER.log(Level.WARNING, "Failed to close XML stream. ", e); - } +} catch (XMLStreamException e) { + LOGGER.log(Level.SEVERE, "Error closing XML stream", e); +} catch (IOException e) { + LOGGER.log(Level.SEVERE, "IO error when closing stream", e); } Suggestion importance[1-10]: 8 Why: Using specific catch blocks for different exceptions allows for more targeted error handling and logging, improving code clarity and maintainability.	8
Resource management	Ensure proper resource management in AutoCloseStream Ensure that the AutoCloseStream properly handles potential exceptions during the stream operations to prevent resource leaks. src/main/java/com/apptasticsoftware/rssreader/AbstractRssReader.java [443-465] return AutoCloseStream.of(urls.stream() ... return Stream.empty(); -})); +}).onClose(() -> { + // Ensure all resources are closed properly here +}); Suggestion importance[1-10]: 7 Why: Adding an onClose handler to ensure resources are closed properly is a good practice for resource management, enhancing the robustness of the code.	7
Possible issue	Modify iterator() and spliterator() to prevent resource leaks The iterator() and spliterator() methods currently return the underlying stream's iterators without ensuring they are auto-closed. This could lead to resource leaks if not handled properly. Consider implementing these methods to support auto-closing features or throw an UnsupportedOperationException if they should not be used. src/main/java/com/apptasticsoftware/rssreader/internal/stream/AutoCloseStream.java [222-228] public Iterator<T> iterator() { - return stream().iterator(); + throw new UnsupportedOperationException("Auto-closing iterator is not supported."); } public Spliterator<T> spliterator() { - return stream().spliterator(); + throw new UnsupportedOperationException("Auto-closing spliterator is not supported."); } Suggestion importance[1-10]: 7 Why: The suggestion to throw UnsupportedOperationException for iterator() and spliterator() is valid as it prevents potential resource leaks by discouraging their use, aligning with the class's design to auto-close streams.	7
Maintainability	Use explicit type definitions instead of 'var' for clarity Replace the use of var with explicit type definitions for better code readability and maintainability, especially in a testing environment where clarity is crucial. src/test/java/com/apptasticsoftware/integrationtest/SortTest.java [52-57] -var timestamps = new RssReader().read(extendedUrlList) +List<Long> timestamps = new RssReader().read(extendedUrlList) ... .collect(Collectors.toList()); Suggestion importance[1-10]: 6 Why: Replacing 'var' with explicit type definitions improves code readability and maintainability, which is beneficial in a testing environment. However, it is a minor improvement compared to functional changes.	6

[github-actions]

Test Results

 10 files   10 suites   33s ⏱️
273 tests 271 ✅ 2 💤 0 ❌
281 runs  279 ✅ 2 💤 0 ❌

Results for commit 297a4ad.

♻️ This comment has been updated with latest results.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
98.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud