Skip to content
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.

Sign up for GitHub

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

Log entry group property not editable #191

Merged
merged 1 commit into from
Mar 25, 2024
Merged

Log entry group property not editable #191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/main/java/org/phoebus/olog/LogResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,16 @@ public Log updateLog(@PathVariable String logId,
Log persistedLog = foundLog.get();
logRepository.archive(persistedLog);

// log entry group property should not be editable but remain if it exists
Property logEntryGroupProperty = LogEntryGroupHelper.getLogEntryGroupProperty(log);
if (logEntryGroupProperty != null) {
log.getProperties().remove(logEntryGroupProperty);
}
logEntryGroupProperty = LogEntryGroupHelper.getLogEntryGroupProperty(persistedLog);
if (logEntryGroupProperty != null) {
log.getProperties().add(logEntryGroupProperty);
}

persistedLog.setOwner(principal.getName());
persistedLog.setLevel(log.getLevel());
persistedLog.setProperties(log.getProperties());
Expand Down
123 changes: 94 additions & 29 deletions src/test/java/org/phoebus/olog/docker/ITUtilLogs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import java.io.IOException;
import java.net.HttpURLConnection;
import java.util.List;

import org.phoebus.olog.docker.ITUtil.AuthorizationChoice;
import org.phoebus.olog.docker.ITUtil.EndpointChoice;
Expand Down Expand Up @@ -91,14 +92,8 @@ static String object2Json(Log[] value) {
/**
* @see ITUtilLogs#assertRetrieveLog(String, int, Log)
*/
public static Log assertRetrieveLog(String path, int expectedResponseCode) {
return assertRetrieveLog(path, expectedResponseCode, LOG_NULL);
}
/**
* @see ITUtilLogs#assertRetrieveLog(String, int, Log)
*/
public static Log assertRetrieveLog(String path, Log expected) {
return assertRetrieveLog(path, HttpURLConnection.HTTP_OK, expected);
public static Log assertRetrieveLog(String path) {
return assertRetrieveLog(path, HttpURLConnection.HTTP_OK, LOG_NULL);
}
/**
* Utility method to return the log with the given name.
Expand All @@ -108,27 +103,27 @@ public static Log assertRetrieveLog(String path, Log expected) {
* @param expected expected response log
*/
public static Log assertRetrieveLog(String path, int expectedResponseCode, Log expected) {
try {
String[] response = null;
Log actual = null;

response = ITUtil.doGetJson(ITUtil.HTTP_IP_PORT_OLOG_LOGS + path);
ITUtil.assertResponseLength2Code(response, expectedResponseCode);
if (HttpURLConnection.HTTP_OK == expectedResponseCode) {
actual = mapper.readValue(response[1], Log.class);
}

if (expected != null) {
assertEquals(expected, actual);
}

return actual;
} catch (IOException e) {
fail();
} catch (Exception e) {
fail();
}
return null;
try {
String[] response = null;
Log actual = null;

response = ITUtil.doGetJson(ITUtil.HTTP_IP_PORT_OLOG_LOGS + path);
ITUtil.assertResponseLength2Code(response, expectedResponseCode);
if (HttpURLConnection.HTTP_OK == expectedResponseCode) {
actual = mapper.readValue(response[1], Log.class);
}

if (expected != null) {
assertEquals(expected, actual);
}

return actual;
} catch (IOException e) {
fail();
} catch (Exception e) {
fail();
}
return null;
}

// ----------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -310,6 +305,76 @@ public static Log assertCreateLog(AuthorizationChoice authorizationChoice, Strin

// ----------------------------------------------------------------------------------------------------

/**
* @see ITUtilLogs#assertUpdateLog(AuthorizationChoice, String, String, int, Log)
*/
public static Log assertUpdateLog(Log value) {
return assertUpdateLog(AuthorizationChoice.ADMIN, "/" + value.getId(), object2Json(value), HttpURLConnection.HTTP_OK, LOG_NULL);
}
/**
* Utility method to update an existing log with the payload data.
*
* @param authorizationChoice authorization choice (none, user, admin)
* @param path path
* @param json json
* @param expectedResponseCode expected response code
* @param expected expected response log
* @return
*/
public static Log assertUpdateLog(AuthorizationChoice authorizationChoice, String path, String json, int expectedResponseCode, Log expected) {
try {
String[] response = null;
Log actual = null;

response = ITUtil.runShellCommand(ITUtil.curlMethodAuthEndpointPathJson(MethodChoice.POST, authorizationChoice, EndpointChoice.LOGS, path, json));
ITUtil.assertResponseLength2Code(response, expectedResponseCode);
if (HttpURLConnection.HTTP_OK == expectedResponseCode) {
actual = mapper.readValue(response[1], Log.class);
}

if (expected != null) {
assertEquals(expected, actual);
}

return actual;
} catch (IOException e) {
fail();
} catch (Exception e) {
fail();
}
return null;
}

// ----------------------------------------------------------------------------------------------------

/**
* @see ITUtilLogs#assertGroupLogs(AuthorizationChoice, List, int)
*/
public static void assertGroupLogs(List<Long> logEntryIds) {
assertGroupLogs(AuthorizationChoice.ADMIN, logEntryIds, HttpURLConnection.HTTP_OK);
}
/**
* Utility method to group log entries, with given id values, together with a joint log entry group property.
*
* @param authorizationChoice authorization choice (none, user, admin)
* @param logEntryIds log entry id values
* @param expectedResponseCode expected response code
*/
public static void assertGroupLogs(AuthorizationChoice authorizationChoice, List<Long> logEntryIds, int expectedResponseCode) {
try {
String[] response = null;

response = ITUtil.runShellCommand(ITUtil.curlMethodAuthEndpointPathJson(MethodChoice.POST, authorizationChoice, EndpointChoice.LOGS, "/group", mapper.writeValueAsString(logEntryIds)));
ITUtil.assertResponseLength2Code(response, expectedResponseCode);
} catch (IOException e) {
fail();
} catch (Exception e) {
fail();
}
}

// ----------------------------------------------------------------------------------------------------

/**
* Utility method to return curl to create log for regular user.
*
Expand Down
59 changes: 51 additions & 8 deletions src/test/java/org/phoebus/olog/docker/OlogLogsIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,17 @@
import org.junit.jupiter.api.Test;
import org.phoebus.olog.docker.ITUtil.AuthorizationChoice;
import org.phoebus.olog.entity.Log;
import org.phoebus.olog.entity.Logbook;
import org.phoebus.olog.entity.State;
import org.phoebus.olog.entity.Log.LogBuilder;
import org.testcontainers.containers.ComposeContainer;
import org.testcontainers.junit.jupiter.Container;
import org.testcontainers.junit.jupiter.Testcontainers;

import java.io.IOException;
import java.net.HttpURLConnection;
import java.util.Arrays;
import java.util.Set;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.fail;
Expand Down Expand Up @@ -118,7 +123,7 @@ void handleLogRetrieveCheck() {
// Upload attachment
// Upload multiple attachments

ITUtilTags.assertRetrieveTag("/l11", HttpURLConnection.HTTP_NOT_FOUND);
ITUtilTags.assertRetrieveTag("/l11", HttpURLConnection.HTTP_NOT_FOUND);
}

/**
Expand Down Expand Up @@ -153,7 +158,7 @@ void handleLogCreateCheckJson() {
String json_incomplete8 = "}";
String json_incomplete9 = "\"";

ITUtilLogs.assertListLogs(0);
int length = ITUtilLogs.assertListLogs(-1).length;

ITUtilLogs.assertCreateLog(AuthorizationChoice.ADMIN, "", json_incomplete1, HttpURLConnection.HTTP_BAD_REQUEST);
ITUtilLogs.assertCreateLog(AuthorizationChoice.ADMIN, "", json_incomplete2, HttpURLConnection.HTTP_BAD_REQUEST);
Expand All @@ -165,7 +170,7 @@ void handleLogCreateCheckJson() {
ITUtilLogs.assertCreateLog(AuthorizationChoice.ADMIN, "", json_incomplete8, HttpURLConnection.HTTP_BAD_REQUEST);
ITUtilLogs.assertCreateLog(AuthorizationChoice.ADMIN, "", json_incomplete9, HttpURLConnection.HTTP_BAD_REQUEST);

ITUtilLogs.assertListLogs(0);
assertEquals(length, ITUtilLogs.assertListLogs(-1).length);
}

/**
Expand All @@ -190,14 +195,52 @@ void handleLogCreateCheck() {
// Upload attachment
// Upload multiple attachments

Log log_check = new Log.LogBuilder().build();
Log log_check = new Log.LogBuilder().build();

ITUtilLogs.assertListLogs(0);
int length = ITUtilLogs.assertListLogs(-1).length;

ITUtilLogs.assertCreateLog(AuthorizationChoice.ADMIN, "", log_check, HttpURLConnection.HTTP_BAD_REQUEST);
ITUtilLogs.assertCreateLog(AuthorizationChoice.ADMIN, "", log_check, HttpURLConnection.HTTP_BAD_REQUEST);
ITUtilLogs.assertCreateLog(AuthorizationChoice.ADMIN, "", log_check, HttpURLConnection.HTTP_BAD_REQUEST);
ITUtilLogs.assertCreateLog(AuthorizationChoice.ADMIN, "", log_check, HttpURLConnection.HTTP_BAD_REQUEST);

ITUtilLogs.assertListLogs(0);
assertEquals(length, ITUtilLogs.assertListLogs(-1).length);
}

@Test
void handleLogGroup() {
// create logbook and prepare logs
Logbook logbook1 = new Logbook("name1", "user", State.Active);
ITUtilLogbooks.assertCreateLogbook("/" + logbook1.getName(), logbook1);

Log log1 = LogBuilder.createLog()
.owner("owner")
.title("title1")
.withLogbooks(Set.of(logbook1))
.build();
Log log2 = LogBuilder.createLog()
.owner("user")
.title("title2")
.withLogbooks(Set.of(logbook1))
.build();

// create logs
log1 = ITUtilLogs.assertCreateLog("", log1);
log2 = ITUtilLogs.assertCreateLog("", log2);
assertEquals(0, log1.getProperties().size());
assertEquals(0, log2.getProperties().size());

// group logs
ITUtilLogs.assertGroupLogs(Arrays.asList(log1.getId(), log2.getId()));
log1 = ITUtilLogs.assertRetrieveLog("/" + log1.getId());
log2 = ITUtilLogs.assertRetrieveLog("/" + log2.getId());
assertEquals(1, log1.getProperties().size());
assertEquals(1, log2.getProperties().size());

// edit and update logs but log group to remain
log1.getProperties().clear();
log1 = ITUtilLogs.assertUpdateLog(log1);
log2 = ITUtilLogs.assertRetrieveLog("/" + log2.getId());
assertEquals(1, log1.getProperties().size());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good we check the logbook group property remains, however it may be more effective to also verify that other properties are operated on as expected? E.g. if the other properties are left alone but we attempt to change or delete the log group?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the other properties may be part of the update which makes it not known which changes to keep and which to stop.

assertEquals(1, log2.getProperties().size());
}

}
2 changes: 1 addition & 1 deletion src/test/resources/INTEGRATIONTEST_DOCKER_RUN.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

* Docker - engine 18.06.0+ or later, compose 2.21.0 or later, compose file version 3.7 to be supported

##### Build ChannelFinder service
##### Build Olog service

```
mvn clean install -Pdeployable-jar
Expand Down
Loading