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.

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

[OPIK-121] Allow multiline in dataset description #339

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,25 @@

public class ValidationUtils {

public static final String NULL_OR_NOT_BLANK = "^(?!\\s*$).+";
/**
* Regular expression to validate if a string is null or not blank.
*
* <p>It matches any string that is not null and contains at least one non-whitespace character.</p>
* For example:
* <ul>
* <li>"" -> false</li>
* <li>" " -> false</li>
* <li>"\n" -> false</li>
* <li>null -> true</li>
* <li>"a" -> true</li>
* <li>" a " -> true</li>
* <li>"\n a \n" -> true</li>
* </ul>
*
* @see <a href="https://regexper.com/">Visual Explainer</a>
* @see <a href="https://zzzcode.ai/regex/explain">Ai Explainer</a>
*/
public static final String NULL_OR_NOT_BLANK = "(?s)^\\s*(\\S.*\\S|\\S)\\s*$";
andrescrz marked this conversation as resolved.
Show resolved Hide resolved

/**
* Canonical String representation to ensure precision over float or double.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1197,6 +1197,22 @@ void create__success() {
createAndAssert(dataset);
}

@Test
@DisplayName("when description is multiline, then accept the request")
void create__whenDescriptionIsMultiline__thenAcceptTheRequest() {

var dataset = factory.manufacturePojo(Dataset.class).toBuilder()
.id(null)
.description("""
Test
Description
"""
)
.build();

createAndAssert(dataset);
}

@Test
@DisplayName("when creating datasets with same name in different workspaces, then accept the request")
void create__whenCreatingDatasetsWithSameNameInDifferentWorkspaces__thenAcceptTheRequest() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,30 @@ void create__whenWorkspaceNameIsSpecified__thenAcceptTheRequest() {
assertProject(project.toBuilder().id(id).build(), apiKey, workspaceName);
}

@Test
@DisplayName("when workspace description is multiline, then accept the request")
void create__whenDescriptionIsMultiline__thenAcceptTheRequest() {
var project = factory.manufacturePojo(Project.class);

project = project.toBuilder().description("Test Project\n\nMultiline Description").build();

UUID id;
try (var actualResponse = client.target(URL_TEMPLATE.formatted(baseURI)).request()
.accept(MediaType.APPLICATION_JSON_TYPE)
.header(HttpHeaders.AUTHORIZATION, API_KEY)
.header(WORKSPACE_HEADER, TEST_WORKSPACE)
.post(Entity.json(project))) {

assertThat(actualResponse.getStatusInfo().getStatusCode()).isEqualTo(201);
assertThat(actualResponse.hasEntity()).isFalse();
assertThat(actualResponse.getHeaderString("Location")).matches(Pattern.compile(URL_PATTERN));

id = TestUtils.getIdFromLocation(actualResponse.getLocation());
}

assertProject(project.toBuilder().id(id).build());
}

@Test
@DisplayName("when description is null, then accept the request")
void create__whenDescriptionIsNull__thenAcceptNameCreate() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.comet.opik.utils;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.stream.Stream;

import static org.junit.jupiter.api.Assertions.assertEquals;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: better use assertJ assertions instead of the ones from JUnit.

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 will address in following PR


class ValidationUtilsTest {

public static Stream<Arguments> testNullOrNotBlank() {
return Stream.of(
Arguments.of("", false),
Arguments.of(" ", false),
Arguments.of("\n", false),
Arguments.of("a", true),
Arguments.of(" a ", true),
Arguments.of("\n a \n", true)
);
}

@ParameterizedTest
@MethodSource
void testNullOrNotBlank(String input, boolean expected) {
assertEquals(expected, input.matches(ValidationUtils.NULL_OR_NOT_BLANK));
}

}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: new line at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same