Merge pull request #1430 from zan-mateusz/fixes/coercion-error-logbook-class-filtering-dynamic-update
Fixes/coercion error logbook class filtering dynamic update
diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
index 75dcb0b..dd773ea 100644
--- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
+++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
@@ -312,10 +312,10 @@
/**
* Populates the initial catalog, but not via an official code-path.
- *
- * Expected to be called only during tests, where the test has not gone through the same
+ *
+ * Expected to be called only during tests, where the test has not gone through the same
* management-context lifecycle as is done in BasicLauncher.
- *
+ *
* Subsequent calls will fail to things like {@link #populateInitialCatalogOnly()} or
* {@link #populateInitialAndPersistedCatalog(ManagementNodeState, PersistedCatalogState, RebindExceptionHandler, RebindLogger)}.
*/
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java
index e6b89ca..a041b72 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java
@@ -145,7 +145,7 @@
private ManagementContextInternal mgmt() {
return (ManagementContextInternal) osgiManager.getManagementContext();
}
-
+
private synchronized void init() {
if (result!=null) {
if (zipFile!=null || zipIn==null) return;
@@ -836,6 +836,7 @@
throw Exceptions.propagate(e);
}
}
+
}
};
if (deferredStart) {
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/logbook/LogBookQueryParams.java b/core/src/main/java/org/apache/brooklyn/util/core/logbook/LogBookQueryParams.java
index d726b57..704eb48 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/logbook/LogBookQueryParams.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/logbook/LogBookQueryParams.java
@@ -50,6 +50,9 @@
private String entityId;
+ /** The logger/class name prefix to filter log items by, e.g. "o.a.b.SSH" */
+ private String loggerName;
+
public Integer getNumberOfItems() {
return numberOfItems;
}
@@ -121,4 +124,12 @@
public void setEntityId(String entityId) {
this.entityId = entityId;
}
+
+ public String getLoggerName() {
+ return loggerName;
+ }
+
+ public void setLoggerName(String loggerName) {
+ this.loggerName = loggerName;
+ }
}
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/logbook/file/FileLogStore.java b/core/src/main/java/org/apache/brooklyn/util/core/logbook/file/FileLogStore.java
index 375ec07..5b37b77 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/logbook/file/FileLogStore.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/logbook/file/FileLogStore.java
@@ -196,6 +196,11 @@
if (Strings.isBlank(brooklynLogEntry.getMessage()) || !brooklynLogEntry.getMessage().contains(params.getSearchPhrase())) return false;
}
+ // Check logger/class name prefix.
+ if (Strings.isNonBlank(params.getLoggerName())) {
+ if (Strings.isBlank(brooklynLogEntry.getClazz()) || !brooklynLogEntry.getClazz().startsWith(params.getLoggerName())) return false;
+ }
+
return true;
};
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/logbook/opensearch/OpenSearchLogStore.java b/core/src/main/java/org/apache/brooklyn/util/core/logbook/opensearch/OpenSearchLogStore.java
index 4a98893..9c76a66 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/logbook/opensearch/OpenSearchLogStore.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/logbook/opensearch/OpenSearchLogStore.java
@@ -305,6 +305,11 @@
queryBoolMustListBuilder.add(buildMatchPhraseOf("message", params.getSearchPhrase()));
}
+ // Apply logger/class name prefix.
+ if (Strings.isNonBlank(params.getLoggerName())) {
+ queryBoolMustListBuilder.add(ImmutableMap.of("prefix", ImmutableMap.of("class", params.getLoggerName())));
+ }
+
ImmutableList<Object> queryBoolMustList = queryBoolMustListBuilder.build();
if (queryBoolMustList.isEmpty()) {
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java b/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java
index c9468f1..65a40db 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java
@@ -355,11 +355,41 @@
public void testYamlMapsDontGoTooFarWhenWantingListOfString() {
List<?> s = TypeCoercions.coerce("[ a: 1, b: 2 ]", List.class);
assertEquals(s, ImmutableList.of(MutableMap.of("a", 1), MutableMap.of("b", 2)));
-
+
s = TypeCoercions.coerce("[ a: 1, b : 2 ]", new TypeToken<List<String>>() {});
assertEquals(s, ImmutableList.of("a: 1", "b : 2"));
}
+ @SuppressWarnings("serial")
+ @Test
+ public void testYamlBlockListCoercionToStringList() {
+ // YAML block list syntax should be parsed correctly for List<String>
+ List<?> s = TypeCoercions.coerce("- a\n- b", new TypeToken<List<String>>() {});
+ assertEquals(s, ImmutableList.of("a", "b"));
+
+ s = TypeCoercions.coerce("- a\n- b\n- c", new TypeToken<List<String>>() {});
+ assertEquals(s, ImmutableList.of("a", "b", "c"));
+
+ // single item
+ s = TypeCoercions.coerce("- a", new TypeToken<List<String>>() {});
+ assertEquals(s, ImmutableList.of("a"));
+
+ // multi-word items
+ s = TypeCoercions.coerce("- hello world\n- foo bar", new TypeToken<List<String>>() {});
+ assertEquals(s, ImmutableList.of("hello world", "foo bar"));
+
+ // numeric items should coerce to strings
+ s = TypeCoercions.coerce("- 1\n- 2", new TypeToken<List<String>>() {});
+ assertEquals(s, ImmutableList.of("1", "2"));
+
+ // comma-separated and bracket forms still work for List<String>
+ s = TypeCoercions.coerce("a, b, c", new TypeToken<List<String>>() {});
+ assertEquals(s, ImmutableList.of("a", "b", "c"));
+
+ s = TypeCoercions.coerce("[a, b]", new TypeToken<List<String>>() {});
+ assertEquals(s, ImmutableList.of("a", "b"));
+ }
+
@Test
public void testURItoStringCoercion() {
String s = TypeCoercions.coerce(URI.create("http://localhost:1234/"), String.class);
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/logbook/file/FileLogStoreTest.java b/core/src/test/java/org/apache/brooklyn/util/core/logbook/file/FileLogStoreTest.java
index 4b4a7a6..969cf6e 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/logbook/file/FileLogStoreTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/logbook/file/FileLogStoreTest.java
@@ -496,6 +496,58 @@
assertTrue(brooklynLogEntries.stream().anyMatch(Predicates.not(e -> e.getTaskId().equals(logBookQueryParams.getTaskId()))));
}
+ @Test
+ public void testQueryLogSampleWithLoggerName() {
+ File file = new File(Objects.requireNonNull(getClass().getClassLoader().getResource(JAVA_LOG_SAMPLE_PATH)).getFile());
+ mgmt = LocalManagementContextForTests.newInstance();
+ mgmt.getBrooklynProperties().put(LOGBOOK_LOG_STORE_PATH.getName(), file.getAbsolutePath());
+ LogBookQueryParams logBookQueryParams = new LogBookQueryParams();
+ logBookQueryParams.setNumberOfItems(1000);
+ logBookQueryParams.setTail(false);
+ logBookQueryParams.setLevels(ImmutableList.of());
+ logBookQueryParams.setLoggerName("i.c.b"); // matches all AbstractToscaYamlConverter entries
+ FileLogStore fileLogStore = new FileLogStore(mgmt);
+ List<BrooklynLogEntry> brooklynLogEntries = fileLogStore.query(logBookQueryParams);
+
+ assertEquals(4, brooklynLogEntries.size());
+ assertTrue(brooklynLogEntries.stream().allMatch(e -> e.getClazz().startsWith("i.c.b")));
+ }
+
+ @Test
+ public void testQueryLogSampleWithLoggerNameAndPhrase() {
+ File file = new File(Objects.requireNonNull(getClass().getClassLoader().getResource(JAVA_LOG_SAMPLE_PATH)).getFile());
+ mgmt = LocalManagementContextForTests.newInstance();
+ mgmt.getBrooklynProperties().put(LOGBOOK_LOG_STORE_PATH.getName(), file.getAbsolutePath());
+ LogBookQueryParams logBookQueryParams = new LogBookQueryParams();
+ logBookQueryParams.setNumberOfItems(1000);
+ logBookQueryParams.setTail(false);
+ logBookQueryParams.setLevels(ImmutableList.of());
+ logBookQueryParams.setLoggerName("i.c.b");
+ logBookQueryParams.setSearchPhrase("testing");
+ FileLogStore fileLogStore = new FileLogStore(mgmt);
+ List<BrooklynLogEntry> brooklynLogEntries = fileLogStore.query(logBookQueryParams);
+
+ assertEquals(2, brooklynLogEntries.size());
+ assertTrue(brooklynLogEntries.stream().allMatch(e -> e.getClazz().startsWith("i.c.b")));
+ assertTrue(brooklynLogEntries.stream().allMatch(e -> e.getMessage().contains("testing")));
+ }
+
+ @Test
+ public void testQueryLogSampleWithNonMatchingLoggerName() {
+ File file = new File(Objects.requireNonNull(getClass().getClassLoader().getResource(JAVA_LOG_SAMPLE_PATH)).getFile());
+ mgmt = LocalManagementContextForTests.newInstance();
+ mgmt.getBrooklynProperties().put(LOGBOOK_LOG_STORE_PATH.getName(), file.getAbsolutePath());
+ LogBookQueryParams logBookQueryParams = new LogBookQueryParams();
+ logBookQueryParams.setNumberOfItems(1000);
+ logBookQueryParams.setTail(false);
+ logBookQueryParams.setLevels(ImmutableList.of());
+ logBookQueryParams.setLoggerName("o.a.b.SSH"); // no entries with this class prefix
+ FileLogStore fileLogStore = new FileLogStore(mgmt);
+ List<BrooklynLogEntry> brooklynLogEntries = fileLogStore.query(logBookQueryParams);
+
+ assertEquals(0, brooklynLogEntries.size());
+ }
+
private LogBookQueryParams newQueryParams(boolean recursive) {
LogBookQueryParams params = new LogBookQueryParams();
params.setNumberOfItems(5); // Request first five only
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/logbook/opensearch/OpenSearchLogStoreTest.java b/core/src/test/java/org/apache/brooklyn/util/core/logbook/opensearch/OpenSearchLogStoreTest.java
index 5517111..393d0df 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/logbook/opensearch/OpenSearchLogStoreTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/logbook/opensearch/OpenSearchLogStoreTest.java
@@ -171,6 +171,31 @@
}
@Test
+ public void queryWithLoggerName() {
+ OpenSearchLogStore cut = new OpenSearchLogStore();
+ LogBookQueryParams p = new LogBookQueryParams();
+ p.setNumberOfItems(10);
+ p.setTail(false);
+ p.setLevels(ImmutableList.of());
+ p.setLoggerName("o.a.b.SSH");
+ String query = cut.getJsonQuery(p);
+ assertEquals(query, "{\"sort\":{\"timestamp\":\"asc\"},\"size\":10,\"query\":{\"bool\":{\"must\":[{\"prefix\":{\"class\":\"o.a.b.SSH\"}}]}}}");
+ }
+
+ @Test
+ public void queryWithLoggerNameAndPhrase() {
+ OpenSearchLogStore cut = new OpenSearchLogStore();
+ LogBookQueryParams p = new LogBookQueryParams();
+ p.setNumberOfItems(10);
+ p.setTail(false);
+ p.setLevels(ImmutableList.of());
+ p.setLoggerName("o.a.b.SSH");
+ p.setSearchPhrase("some phrase");
+ String query = cut.getJsonQuery(p);
+ assertEquals(query, "{\"sort\":{\"timestamp\":\"asc\"},\"size\":10,\"query\":{\"bool\":{\"must\":[{\"match_phrase\":{\"message\":\"some phrase\"}},{\"prefix\":{\"class\":\"o.a.b.SSH\"}}]}}}");
+ }
+
+ @Test
public void queryWithTaskIdAndPhrase() {
OpenSearchLogStore cut = new OpenSearchLogStore();
LogBookQueryParams p = new LogBookQueryParams();
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/task/AutoFlagsCallbackTest.java b/core/src/test/java/org/apache/brooklyn/util/core/task/AutoFlagsCallbackTest.java
index 37d2361..bf7eb59 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/task/AutoFlagsCallbackTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/task/AutoFlagsCallbackTest.java
@@ -55,8 +55,8 @@
Asserts.assertThat(depth.get(), x -> x>=0);
app.start(null);
- // but by the time start completes it should be back to 0
- Asserts.assertEquals(depth.get(), 0);
+ // sensor tasks triggered during start may complete slightly after start() returns; wait for them
+ Asserts.eventually(() -> depth.get(), x -> x == 0);
Entities.submit(app, Tasks.create("test1", () -> {
log.info("running test 1" + " / " + Tasks.current().getId());
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherUpgradeCatalogOsgiTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherUpgradeCatalogOsgiTest.java
index a3f4c32..97ef1bc 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherUpgradeCatalogOsgiTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherUpgradeCatalogOsgiTest.java
@@ -406,7 +406,7 @@
assertEquals(resultWithoutForceCode, ResultCode.IGNORING_BUNDLE_FORCIBLY_REMOVED);
assertEquals(resultWithoutForce.get().getMetadata().getVersionedName(), bundleV2.getVersionedName());
assertTrue(resultWithoutForceMessage.contains("Bundle "+bundleV1.getVersionedName()+" forcibly removed, upgraded to 2.0.0"), "msg="+resultWithoutForceMessage);
-
+
launcher.terminate();
}
}
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java
index a792c70..563fe12 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java
@@ -441,8 +441,25 @@
Maybe<?> resultM = null;
Collection<?> result = null;
if (parameters.length==1 && TypeTokens.isAssignableFromRaw(CharSequence.class, parameters[0])) {
- // for list of strings, use special parse
- result = JavaStringEscapes.unwrapJsonishListStringIfPossible(inputS);
+ // for list of strings: if input uses YAML block list syntax ("- item" lines), parse
+ // it directly with YAML (wrapping in brackets kills the block sequence markers).
+ // Only adopt the YAML result when all elements are simple values — this preserves
+ // the behaviour that "[ a: 1, b: 2 ]" stays as ["a: 1", "b: 2"] rather than
+ // being coerced from map objects to their toString representations.
+ if (inputS.trim().startsWith("- ")) {
+ try {
+ Object yamlDoc = Iterables.getOnlyElement(Yamls.parseAll(inputS));
+ if (yamlDoc instanceof List &&
+ ((List<?>) yamlDoc).stream().noneMatch(x -> x instanceof Map || x instanceof Collection)) {
+ result = (Collection<?>) yamlDoc;
+ }
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ }
+ }
+ if (result == null) {
+ result = JavaStringEscapes.unwrapJsonishListStringIfPossible(inputS);
+ }
} else {
// any other type, use YAMLish parse
resultM = JavaStringEscapes.tryUnwrapJsonishList(inputS);
