Open
Conversation
… UIDs containing unnecessary double quotes when a segment includes a hyphen. The existing implementation removes these quotes using a regex replacement but has several shortcomings: - Platform-default charset usage may cause inconsistent output on different systems. - The regex is recompiled on every method call. - The logic mixes multiple responsibilities in one block, reducing readability. - Logging suppresses the full stack trace, making debugging harder.
…umentation and error handling - Extracted ModelThingBuilder to reduce parameter coupling - Added constants and JavaDoc for clarity - Improved error handling and UID normalization - No behavior changes, internal refactor only
…ericItemChannelLinkProvider Core improvements: - **Critical Fix**: Replaced global transaction state with per-context maps to fix race condition - 'previousItemNames' (global Set) 'previousItemNamesByContext' (Map<String, Set<String>>) - 'addedItemChannels' (global Map keyed only by itemName) 'addedItemChannelsByContext' (Map<String, Map<String, Set<ChannelUID>>>) - Multiple concurrent contexts can now safely update without cross-contamination - Enhanced binding configuration parsing - Added robust filtering of empty/whitespace-only tokens - Improved error handling and validation - Added comprehensive JavaDoc to all public methods - processBindingConfiguration: explains format and error cases - createItemChannelLink: clarifies per-context link creation - startConfigurationUpdate: documents transaction semantics - stopConfigurationUpdate: explains cleanup and per-context state management - getAll() and getAllFromContext(): clarifies filtering and isolation - Code clarity improvements - Changed indexOf check to contains() for better readability - Improved streaming code (entrySet vs keySet) - Better variable naming in map operations Tests added: - 10 comprehensive unit tests covering: - Single and multiple channel UID parsing - Empty/invalid binding config handling - Profile scope normalization - Concurrent context transaction isolation (validates core fix) - Channel lifecycle management during updates All tests passing (10/10). Verified no breaking changes to existing tests.
…e clarity Core improvements: - **Thread Safety Fix**: Replaced non-concurrent collections with thread-safe variants - parsers (HashSet) -> ConcurrentHashMap.newKeySet() - missingParsers (HashSet) -> ConcurrentHashMap.newKeySet() - ignoredPaths (HashSet) -> ConcurrentHashMap.newKeySet() - namePathMap (HashMap) -> ConcurrentHashMap - Added dedicated repoLock object for coordinated repository operations - Replaced global class-level synchronization with instance-level repoLock - **Code Clarity**: Introduced constants for magic values - EXPECTED_RELATIVE_NAME_COUNT = 2 (replaces hardcoded path depth) - EXTENSION_SEPARATOR = dot (replaces inline string literals) - **Helper Methods**: Added isValidFolderName() to centralize folder name validation - Improves readability and reduces duplication - **Documentation**: Added comprehensive JavaDoc to public API methods - addModelParser: explains registration and ready markers - removeModelParser: clarifies removal and model cleanup - activate: documents folder watching and initialization - deactivate: explains cleanup sequence - processWatchEvent: clarifies event handling and validation Benefits: - Multiple FolderObserver instances now share concurrent state safely - Repository lock is isolated per instance, reducing contention - Code is more maintainable with constants and helper methods - API behavior is clearly documented All tests pass (5/5). No breaking changes.
…ity and documentation Extract 6 magic string constants (autoupdate, unit, stateDescription, pattern, profile) to class-level for single source of truth and reduced magic string usage. Add logger field for proper error handling with debug logging on URISyntaxException instead of silent ignoring. Extract dimension handling logic to helper methods: - extractMainTypeAndDimension(): Centralize type/dimension parsing - applyTypeAndDimensionToDTO(): Apply extracted type info to both Item and Group DTOs Eliminates duplicate code that appeared twice in buildItemDTO(). Split long buildItemDTO() method (120 lines) into focused submethods: - setGroupInfo(): Extract group-specific configuration - setChannelInfo(): Extract channel link processing - setMetadataInfo(): Extract metadata (unit, autoupdate, stateDescription) processing Improves code readability and testability by following Single Responsibility Principle. Add comprehensive JavaDoc to all 8 public methods: - getFileFormatGenerator(), setItemsToBeGenerated(), generateFileFormat() - getFileFormatParser(), startParsingFileFormat(), getParsedItems(), getParsedMetadata() - getParsedStateFormatters(), finishParsingFileFormat() Clarifies API intent, parameters, and return values. Testing: All existing unit tests pass (8/8), no regressions. Code quality: Applied spotless formatting for consistency. This refactoring reduces cognitive complexity, improves maintainability through extraction and documentation, and addresses error handling gaps.
… relation helper - Use ConcurrentSkipListMap for semantics to provide thread-safe ordered cache and return immutable snapshot from getAll() - Replace non-standard getFirst() calls with get(0) to use standard List API - Extract common parent/member processing into processRelations() helper to remove duplication Runs tests: all semantics tests pass.
… main after accidentally merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Title: refactor(core): improve concurrency, clarity and maintainability across multiple bundles
Summary
assignment1_AbdulAzimmain(RareZyn/openhab-core)GenericItemChannelLinkProvider(global transaction state causing cross-context contamination).FolderObserverand a non-concurrent live cache inSemanticsMetadataProvider.YamlItemFileConverterandDslThingFileConverter.DslThingFileConverter: Added class-level constants andModelThingBuilder(Builder pattern) to reduce parameter coupling and centralize validation.GenericItemChannelLinkProvider: Replaced global transaction maps with per-context maps, hardened parsing, added JavaDoc and 10 unit tests covering parsing and concurrency.FolderObserver: Replaced non-thread-safe collections with concurrent collections, added per-instancerepoLock, extracted constants and helper validation.YamlItemFileConverter: Extracted 6 magic strings to constants, addedLOGGER, replaced silent exception handling, extracted type/dimension helpers and splitbuildItemDTO()into focused submethods.SemanticsMetadataProvider: ReplacedTreeMapwithConcurrentSkipListMapand returned immutable snapshots; replaced non-standardgetFirst()with standardget(0)and extracted common relation logic.Files changed (high level)
Notes
assignment1_AbdulAzimin this fork (RareZyn/openhab-core).