-
Notifications
You must be signed in to change notification settings - Fork 23
Text properties substitution #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| package com.confighub.core.utils; | ||
|
|
||
| import com.confighub.core.repository.Property; | ||
| import com.confighub.core.repository.PropertyKey; | ||
| import com.confighub.core.security.SecurityProfile; | ||
| import org.apache.logging.log4j.LogManager; | ||
| import org.apache.logging.log4j.Logger; | ||
|
|
||
| import java.util.*; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| public class PropertiesSubstitutor { | ||
|
|
||
| private static final Logger log = LogManager.getLogger(PropertiesSubstitutor.class); | ||
|
|
||
| private static String replaceKey(String key, String value, String text) | ||
| { | ||
| return text.replaceAll("\\$\\{" + Pattern.quote(key) + "}", value); | ||
| } | ||
|
|
||
| private static boolean containsKey(String key, String text) | ||
| { | ||
| return text.contains("${" + key + "}"); | ||
| } | ||
|
|
||
| public static Map<PropertyKey, String> resolveTextSubstitutions(final Map<PropertyKey, Property> resolved, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am only concerned with the amount of extra work this is doing to handle things that refer to eachother. We've been dealing with performance issues and would like to potentially limit the amount of work necessary to resolve a file. This seems like it is adding a significant amount of work to solve a case that can be solved by not having configurations bidirectional. I'm curious on Edin's point of view here. |
||
| final Map<String, String> passwords) { | ||
| Map<PropertyKey, String> textPropertiesValues = new HashMap<>(); | ||
| for (PropertyKey key : resolved.keySet()) | ||
| { | ||
| //We are only interested in text properties | ||
| if(key.getValueDataType().equals(PropertyKey.ValueDataType.Text)) | ||
| { | ||
| Property property = resolved.get(key); | ||
| if(key.isEncrypted()) | ||
| { | ||
| SecurityProfile sp = key.getSecurityProfile(); | ||
| String password = passwords.get(sp.getName()); | ||
| if(password != null) { | ||
| property.decryptValue(password); | ||
| textPropertiesValues.put(key, property.getValue()); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| textPropertiesValues.put(key, property.getValue()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Map<PropertyKey, Set<PropertyKey>> inboundDependencies = new HashMap<>(); | ||
| Map<PropertyKey, Set<PropertyKey>> outboundDependencies = new HashMap<>(); | ||
| Queue<PropertyKey> queue = new ArrayDeque<>(); | ||
|
|
||
| for (PropertyKey key : textPropertiesValues.keySet()) | ||
| { | ||
| inboundDependencies.put(key, new HashSet<>()); | ||
| outboundDependencies.put(key, new HashSet<>()); | ||
| } | ||
|
|
||
| for (PropertyKey keyA : textPropertiesValues.keySet()) | ||
| { | ||
| String text = textPropertiesValues.get(keyA); | ||
| for (PropertyKey keyB : textPropertiesValues.keySet()) | ||
| { | ||
| if(containsKey(keyB.getKey(), text)) | ||
| { | ||
| outboundDependencies.get(keyA).add(keyB); | ||
| inboundDependencies.get(keyB).add(keyA); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| for (PropertyKey key : textPropertiesValues.keySet()) | ||
| { | ||
| if(outboundDependencies.get(key).isEmpty()) | ||
| { | ||
| queue.offer(key); | ||
| } | ||
| } | ||
|
|
||
| while (!queue.isEmpty()) { | ||
| PropertyKey key = queue.poll(); | ||
| for(PropertyKey dependantKey : inboundDependencies.get(key)) { | ||
| textPropertiesValues.put( | ||
| dependantKey, | ||
| replaceKey( | ||
| key.getKey(), | ||
| textPropertiesValues.get(key), | ||
| textPropertiesValues.get(dependantKey))); | ||
| Set<PropertyKey> dependencies = outboundDependencies.get(dependantKey); | ||
| dependencies.remove(key); | ||
| if(dependencies.isEmpty()) { | ||
| queue.offer(dependantKey); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return textPropertiesValues; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| package com.confighub.core.utils; | ||
|
|
||
| import com.confighub.core.repository.Depth; | ||
| import com.confighub.core.repository.Property; | ||
| import com.confighub.core.repository.PropertyKey; | ||
| import com.confighub.core.repository.Repository; | ||
| import com.confighub.core.user.Account; | ||
| import org.junit.Test; | ||
|
|
||
| import java.lang.reflect.Field; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| import static org.junit.Assert.assertEquals; | ||
|
|
||
| public class PropertiesSubstitutorTest { | ||
|
|
||
| private static final Repository testRepository = | ||
| new Repository("TestRepository", Depth.D0, false, new Account()); | ||
|
|
||
| private void addProperty(String key, String value, Map<PropertyKey, Property> properties, Map<String, PropertyKey> keys) { | ||
| PropertyKey propertyKey = new PropertyKey(testRepository, key, PropertyKey.ValueDataType.Text); | ||
| Property property = new Property(testRepository); | ||
| try { | ||
| Field valueField = Property.class.getDeclaredField("value"); | ||
| valueField.setAccessible(true); | ||
| valueField.set(property, value); | ||
| } catch (NoSuchFieldException | IllegalAccessException e) { | ||
| e.printStackTrace(); | ||
| } | ||
| properties.put(propertyKey, property); | ||
| keys.put(key, propertyKey); | ||
| } | ||
|
|
||
| @Test | ||
| public void substitutionBasicTest() | ||
| { | ||
| Map<PropertyKey, Property> properties = new HashMap<>(); | ||
| Map<String, PropertyKey> keys = new HashMap<>(); | ||
|
|
||
| addProperty("key0", "value0:${key1}", properties, keys); | ||
| addProperty("key1", "value1:${key0}:${key2}", properties, keys); | ||
| addProperty("key2", "value2", properties, keys); | ||
| addProperty("key3", "value3:${key2}", properties, keys); | ||
| addProperty("key4", "value4:${key3}", properties, keys); | ||
|
|
||
| Map<PropertyKey, String> result = PropertiesSubstitutor.resolveTextSubstitutions(properties, new HashMap<>()); | ||
|
|
||
| assertEquals(properties.size(), result.size()); | ||
| assertEquals("value0:${key1}", result.get(keys.get("key0"))); | ||
| assertEquals("value1:${key0}:value2", result.get(keys.get("key1"))); | ||
| assertEquals("value2", result.get(keys.get("key2"))); | ||
| assertEquals("value3:value2", result.get(keys.get("key3"))); | ||
| assertEquals("value4:value3:value2", result.get(keys.get("key4"))); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ | |
|
|
||
| <groupId>ConfigHub</groupId> | ||
| <artifactId>ConfigHub-Rest</artifactId> | ||
| <version>1.8.4</version> | ||
| <version>1.8.5-alpha-1</version> | ||
| <name>ConfigHub REST</name> | ||
| <packaging>war</packaging> | ||
|
|
||
|
|
@@ -14,7 +14,7 @@ | |
| <dependency> | ||
| <groupId>ConfigHub</groupId> | ||
| <artifactId>ConfigHub-Core</artifactId> | ||
| <version>1.8.4</version> | ||
| <version>1.8.5-alpha-1</version> | ||
| </dependency> | ||
|
|
||
| <dependency> | ||
|
|
@@ -27,7 +27,7 @@ | |
| <dependency> | ||
| <groupId>org.projectlombok</groupId> | ||
| <artifactId>lombok</artifactId> | ||
| <version>1.16.20</version> | ||
| <version>1.18.4</version> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i agree with updating things, but it is subtle and not described in this commit message. |
||
| </dependency> | ||
|
|
||
| <dependency> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably don't want to change the base version master to the alpha. I get you probably did this to get a build for yourself.