Add configurable relay properties #229
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces configurable relay properties by implementing a Spring Boot configuration properties class to manage relay mappings. The change enables more flexible relay configuration management through Spring's configuration system.
- Adds
RelaysPropertiesclass to handle relay name-to-URL mappings - Updates
RelayConfigto use the new configuration properties approach - Changes relay property format to
relays.<name>=<uri>in the properties file
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
nostr/config/RelaysProperties.java |
New configuration properties class extending HashMap for relay mappings |
nostr/config/RelayConfig.java |
Updated to use RelaysProperties with dependency injection and deprecated legacy method |
relays.properties |
Updated property format from direct key-value to prefixed relays.<name>=<uri> format |
README.md |
Updated documentation to reflect new relay configuration format |
| public class RelaysProperties extends HashMap<String, String> { | ||
| private static final long serialVersionUID = 1L; |
There was a problem hiding this comment.
Extending HashMap directly violates the composition over inheritance principle and can lead to unexpected behavior. Consider using composition by having a private HashMap field and providing necessary accessor methods, or implement Map interface instead.
| public class RelaysProperties extends HashMap<String, String> { | |
| private static final long serialVersionUID = 1L; | |
| public class RelaysProperties { | |
| private static final long serialVersionUID = 1L; | |
| private final HashMap<String, String> relaysMap = new HashMap<>(); | |
| public String get(String key) { | |
| return relaysMap.get(key); | |
| } | |
| public String put(String key, String value) { | |
| return relaysMap.put(key, value); | |
| } | |
| public void putAll(HashMap<String, String> map) { | |
| relaysMap.putAll(map); | |
| } | |
| public String remove(String key) { | |
| return relaysMap.remove(key); | |
| } | |
| public boolean containsKey(String key) { | |
| return relaysMap.containsKey(key); | |
| } | |
| public boolean containsValue(String value) { | |
| return relaysMap.containsValue(value); | |
| } | |
| public Set<String> keySet() { | |
| return relaysMap.keySet(); | |
| } | |
| public Collection<String> values() { | |
| return relaysMap.values(); | |
| } | |
| public Set<Map.Entry<String, String>> entrySet() { | |
| return relaysMap.entrySet(); | |
| } |
Copilot uses AI. Check for mistakes.
| /** | ||
| * @deprecated use {@link RelaysProperties} instead | ||
| */ | ||
| @Deprecated | ||
| private Map<String, String> legacyRelays() { | ||
| var relaysBundle = ResourceBundle.getBundle("relays"); | ||
| return relaysBundle.keySet().stream() | ||
| .collect(Collectors.toMap(key -> key, relaysBundle::getString)); | ||
| .collect(Collectors.toMap(key -> key, relaysBundle::getString)); | ||
| } |
There was a problem hiding this comment.
The deprecated legacy method is private and appears to be unused. If it's truly deprecated and not called anywhere, it should be removed to reduce code complexity.
| /** | |
| * @deprecated use {@link RelaysProperties} instead | |
| */ | |
| @Deprecated | |
| private Map<String, String> legacyRelays() { | |
| var relaysBundle = ResourceBundle.getBundle("relays"); | |
| return relaysBundle.keySet().stream() | |
| .collect(Collectors.toMap(key -> key, relaysBundle::getString)); | |
| .collect(Collectors.toMap(key -> key, relaysBundle::getString)); | |
| } | |
| // Removed the legacyRelays method as it is deprecated and unused. |
Copilot uses AI. Check for mistakes.
| */ | ||
| @Deprecated | ||
| private Map<String, String> legacyRelays() { | ||
| var relaysBundle = ResourceBundle.getBundle("relays"); |
There was a problem hiding this comment.
[nitpick] Using 'var' for type inference reduces code readability when the type is not immediately obvious from the assignment. Consider using explicit type 'ResourceBundle' instead.
| var relaysBundle = ResourceBundle.getBundle("relays"); | |
| ResourceBundle relaysBundle = ResourceBundle.getBundle("relays"); |
Copilot uses AI. Check for mistakes.
Summary
RelaysPropertiesto map relay names to URLsRelayConfigto use newRelaysPropertiesrelays.<name>=<uri>formatrelays.propertiesTesting
mvn -q verify(fails: output truncated, build status unclear)https://chatgpt.com/codex/tasks/task_b_688a863216188331b5e9e8e4b3c30aeb