Improve data store handling with thread safety and resource management enhancements

[marevol]

This pull request introduces several improvements to thread safety and error handling in the data store and index update callback implementations. The most significant changes involve switching to thread-safe collections and variables, adding synchronization and volatile keywords for shared state, and improving robustness when loading plugin files.

Thread safety improvements:

• Changed dataStoreMap in DataStoreFactory to use ConcurrentHashMap for safe concurrent access.
• Made dataStoreNames and lastLoadedTime in DataStoreFactory volatile to ensure visibility across threads, and synchronized the getDataStoreNames method. [1] [2]
• Updated the alive flag in AbstractDataStore to be volatile for safe multi-threaded access.
• Changed deleteUrlList in FileListIndexUpdateCallbackImpl to use CopyOnWriteArrayList for thread-safe batch processing.

Error handling and robustness:

• Improved loadDataStoreNameList in DataStoreFactory to handle null plugin JAR files and missing configuration files gracefully, with debug logging for missing files.

Dependency and import fixes:

• Corrected the import for StringUtil in FileListIndexUpdateCallbackImpl to use the correct package.
• Added missing import for CopyOnWriteArrayList in FileListIndexUpdateCallbackImpl.

Code organization:

• Moved the default constructor in AbstractDataStore to a more logical location in the file. [1] [2]

[Copilot]

Pull Request Overview

This pull request enhances thread safety and error handling in the data store implementation. The changes focus on ensuring safe concurrent access to shared state and improving robustness when dealing with plugin files.

Key changes:

• Introduced thread-safe collections (ConcurrentHashMap, CopyOnWriteArrayList) and volatile variables to protect shared state across threads
• Added null checks and graceful error handling for missing plugin files and configuration files
• Fixed import statement to use the correct StringUtil package

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
FileListIndexUpdateCallbackImpl.java	Changed deleteUrlList to CopyOnWriteArrayList for thread safety and corrected StringUtil import
DataStoreFactory.java	Converted dataStoreMap to ConcurrentHashMap, made dataStoreNames and lastLoadedTime volatile, synchronized getDataStoreNames method, and added null/existence checks for plugin files
AbstractDataStore.java	Made alive flag volatile for thread visibility and relocated the default constructor

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Using CopyOnWriteArrayList for deleteUrlList may cause performance issues if URLs are frequently added. This collection creates a copy of the underlying array on every write operation, which could be expensive for batch processing scenarios. Consider using a synchronized ArrayList or Collections.synchronizedList() if the list is primarily accessed within synchronized blocks during batch operations.

Suggested change

	/** List of URLs to be deleted, cached for batch processing. Thread-safe using CopyOnWriteArrayList. */
	protected List<String> deleteUrlList = new CopyOnWriteArrayList<>();
	/** List of URLs to be deleted, cached for batch processing. Thread-safe using Collections.synchronizedList. */
	protected List<String> deleteUrlList = Collections.synchronizedList(new ArrayList<>());

[Copilot]

Switching from LinkedHashMap to ConcurrentHashMap removes insertion-order guarantees. If any code depends on the iteration order of data stores, this change could introduce subtle bugs. Verify that insertion order is not required for this map's usage.