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

[CONTINT-685] - initial tests for validation of OpenApi request #767

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
b2e9234
initial tests for validation of OpenApi request
azakrzewski-hy Dec 5, 2024
cd4e3ec
updating white spaces
azakrzewski-hy Dec 5, 2024
19e5d98
updating class header
azakrzewski-hy Dec 5, 2024
3964e56
CONTINT-685 Allow specifying request definition in a resource file.
tpage-alfresco Dec 5, 2024
529f282
CONTINT-747 Temporary fix for default version number.
tpage-alfresco Dec 5, 2024
ab08fc3
CONTINT-685 Update existing test file to use new yml file for request.
tpage-alfresco Dec 5, 2024
6afb106
adding update and delete validation tests
azakrzewski-hy Dec 9, 2024
366b993
adding new lines at the end of update and delete files
azakrzewski-hy Dec 9, 2024
0352957
fixing PMD warnings
azakrzewski-hy Dec 10, 2024
182a8f6
fixing constructor and name PMD warnings
azakrzewski-hy Dec 10, 2024
57254c4
adding parse options into interaction validator
azakrzewski-hy Dec 17, 2024
f174dee
adding validation for properties field
azakrzewski-hy Dec 19, 2024
913274f
updating and adding negative tests
azakrzewski-hy Dec 19, 2024
1b1a8ad
adding new line at the end of the file
azakrzewski-hy Dec 19, 2024
576e8eb
updating DeleteRequestIntegrationTest class
azakrzewski-hy Dec 19, 2024
83abcfa
updating testUpdateRequest() in UpdateRequestIntegrationTest class
azakrzewski-hy Dec 19, 2024
5dc73c5
Merge branch 'master' into feature/CONTINT-685/test-open-api-specific…
azakrzewski-hy Dec 19, 2024
49c27b5
implementing changes mentioned in pr comments
azakrzewski-hy Dec 19, 2024
64904cf
align with PMD UnitTestShouldIncludeAssert
azakrzewski-hy Dec 19, 2024
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
2 changes: 1 addition & 1 deletion .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -296,5 +296,5 @@
}
]
},
"generated_at": "2024-10-07T15:02:23Z"
"generated_at": "2024-12-09T11:44:50Z"
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public class DockerTags
private static final String TRANSFORM_ROUTER_TAG_DEFAULT = "4.0.1";
private static final String TRANSFORM_CORE_AIO_TAG_DEFAULT = "5.0.1";
private static final String SFS_TAG_DEFAULT = "4.0.1";
private static final String HXI_CONNECTOR_TAG_DEFAULT = "1.0.0-SNAPSHOT";
private static final String HXI_CONNECTOR_TAG_DEFAULT = "1.0.2-SNAPSHOT";
private static final String PROPERTIES_FILE = "docker-tags.properties";

private static Properties properties;
Expand Down
5 changes: 5 additions & 0 deletions live-ingester/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@
<artifactId>lombok</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
azakrzewski-hy marked this conversation as resolved.
Show resolved Hide resolved
<groupId>com.atlassian.oai</groupId>
<artifactId>swagger-request-validator-core</artifactId>
<version>2.44.1</version>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import org.junit.jupiter.api.Test;

import org.alfresco.hxi_connector.live_ingester.util.E2ETestBase;
import org.alfresco.hxi_connector.live_ingester.util.insight_api.HxInsightRequest;
import org.alfresco.hxi_connector.live_ingester.util.insight_api.RequestLoader;

public class CreateRequestIntegrationTest extends E2ETestBase
{
Expand Down Expand Up @@ -87,41 +89,8 @@ void testCreateRequest()
containerSupport.raiseRepoEvent(repoEvent);

// then
String expectedBody = """
[
{
"objectId": "d71dd823-82c7-477c-8490-04cb0e826e65",
"sourceId" : "alfresco-dummy-source-id-0a63de491876",
"eventType": "create",
"sourceTimestamp": 1611227656423,
"properties": {
"cm:autoVersion": {"value": true},
"createdAt": {"value": 1611227655695},
"modifiedAt": {"value" : 1611227655695},
"cm:versionType": {"value": "MAJOR"},
"aspectsNames": {"value": ["cm:versionable", "cm:auditable"]},
"cm:name": {
"value": "purchase-order-scan.doc",
"annotation" : "name"
},
"type": {"value": "cm:content"},
"createdBy": {"value": "admin"},
"modifiedBy": {"value": "admin"},
"cm:content": {
"file": {
"content-metadata": {
"size": 531152,
"name": "purchase-order-scan.doc",
"content-type": "application/msword"
}
}
},
"ALLOW_ACCESS": {"value": ["GROUP_EVERYONE"]},
"DENY_ACCESS": {"value": []}
}
}
]""";
containerSupport.expectHxIngestMessageReceived(expectedBody);
HxInsightRequest request = RequestLoader.load("/expected-hxinsight-requests/create-document-request.yml");
containerSupport.expectHxIngestMessageReceived(request.body());

String expectedATSRequest = """
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
/*
* #%L
* Alfresco HX Insight Connector
* %%
* Copyright (C) 2023 - 2024 Alfresco Software Limited
* %%
* This file is part of the Alfresco software.
* If the software was purchased under a paid Alfresco license, the terms of
* the paid license agreement will prevail. Otherwise, the software is
* provided under the following open source license terms:
*
* Alfresco is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Alfresco is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
* #L%
*/
package org.alfresco.hxi_connector.live_ingester.domain.usecase.e2e.repository;

import static com.atlassian.oai.validator.schema.SchemaValidator.ADDITIONAL_PROPERTIES_KEY;
import static org.assertj.core.api.Assertions.assertThat;

import java.util.List;

import com.atlassian.oai.validator.OpenApiInteractionValidator;
import com.atlassian.oai.validator.model.Request;
import com.atlassian.oai.validator.model.SimpleRequest;
import com.atlassian.oai.validator.report.LevelResolver;
import com.atlassian.oai.validator.report.MessageResolver;
import com.atlassian.oai.validator.report.ValidationReport;
import com.atlassian.oai.validator.schema.SchemaValidator;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import io.swagger.parser.OpenAPIParser;
import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.parser.core.models.ParseOptions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import org.alfresco.hxi_connector.live_ingester.util.insight_api.HxInsightRequest;
import org.alfresco.hxi_connector.live_ingester.util.insight_api.RequestLoader;

public class OpenApiRequestValidationTest
{

private static final String OPEN_API_SPECIFICATION_URL = "http://hxai-data-platform-dev-swagger-ui.s3-website-us-east-1.amazonaws.com/docs/insight-ingestion-api-swagger.json";
private static OpenApiInteractionValidator classUnderTest;
private static Schema propertiesSchema;
private static SchemaValidator schemaValidator;

@BeforeAll
static void setUp()
{

classUnderTest = OpenApiInteractionValidator
azakrzewski-hy marked this conversation as resolved.
Show resolved Hide resolved
.createForSpecificationUrl(OPEN_API_SPECIFICATION_URL)
.withLevelResolver(LevelResolver.create().withLevel(ADDITIONAL_PROPERTIES_KEY, ValidationReport.Level.IGNORE).build())
.build();

schemaValidator = validatorWithAdditionalPropertiesIgnored(OPEN_API_SPECIFICATION_URL);
propertiesSchema = new Schema().additionalProperties(
azakrzewski-hy marked this conversation as resolved.
Show resolved Hide resolved
new Schema().oneOf(List.of(
new Schema().$ref("#/components/schemas/File"),
new Schema().$ref("#/components/schemas/Value"))));
}

@Test
void testRequestToPresignedUrls()
Fixed Show fixed Hide fixed
{
HxInsightRequest hxInsightRequest = RequestLoader.load("/expected-hxinsight-requests/get-presigned-url-request.yml");

Request request = makeRequest(hxInsightRequest);

assertThat(classUnderTest.validateRequest(request).getMessages()).isEmpty();
}

@Test
void testCreateRequestToIngestionEvents() throws Exception
{
HxInsightRequest hxInsightRequest = RequestLoader.load("/expected-hxinsight-requests/create-document-request.yml");

Request request = makeRequest(hxInsightRequest);

assertThat(classUnderTest.validateRequest(request).getMessages()).isEmpty();
validatePropertiesField(hxInsightRequest.body(), propertiesSchema);
}

@Test
void testUpdateRequestToIngestionEvents() throws Exception
{
HxInsightRequest hxInsightRequest = RequestLoader.load("/expected-hxinsight-requests/update-document-request.yml");

Request request = makeRequest(hxInsightRequest);

assertThat(classUnderTest.validateRequest(request).getMessages()).isEmpty();
validatePropertiesField(hxInsightRequest.body(), propertiesSchema);
Copy link
Member

@tpage-alfresco tpage-alfresco Dec 19, 2024

Choose a reason for hiding this comment

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

We are very close to a generic test here. If we add in a check based on whether the request contains a properties field then we can do something like:

for (requestFile : expected-hxinsight-requests folder)
{
    HxInsightRequest hxInsightRequest = RequestLoader.load(requestFile);

    Request request = makeRequest(hxInsightRequest);

    assertThat(classUnderTest.validateRequest(request).getMessages()).isEmpty();
    if (hxInsightRequest.hasField("properties")
    {
        validatePropertiesField(hxInsightRequest.body(), propertiesSchema);
    }
}

Then we never need to update this file and we automatically check any new requests stored in the expected requests folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could be done in separate pr

}

@Test
void testDeleteRequestToIngestionEvents()
Fixed Show fixed Hide fixed
{
HxInsightRequest hxInsightRequest = RequestLoader.load("/expected-hxinsight-requests/delete-document-request.yml");

Request request = makeRequest(hxInsightRequest);

assertThat(classUnderTest.validateRequest(request).getMessages()).isEmpty();
}

private static SchemaValidator validatorWithAdditionalPropertiesIgnored(final String api)
{
final ParseOptions parseOptions = new ParseOptions();
parseOptions.setResolve(true);
return new SchemaValidator(
new OpenAPIParser().readLocation(api, null, parseOptions).getOpenAPI(),
new MessageResolver(
LevelResolver
.create()
.withLevel(ADDITIONAL_PROPERTIES_KEY, ValidationReport.Level.IGNORE)
.build()));
}

private static Request makeRequest(HxInsightRequest hxInsightRequest)
{
SimpleRequest.Builder builder = SimpleRequest.Builder.post(hxInsightRequest.url());
hxInsightRequest.headers().forEach(builder::withHeader);
return builder.withBody(hxInsightRequest.body()).build();
}

private void validatePropertiesField(String propertiesBody, Schema propertiesSchema) throws Exception
Fixed Show fixed Hide fixed
{
ObjectMapper objectMapper = new ObjectMapper();
JsonNode requestBodyNode = objectMapper.readTree(propertiesBody);
JsonNode propertiesNode = requestBodyNode.get(0).get("properties");

if (propertiesNode != null)
{
ValidationReport validationReport = schemaValidator.validate(propertiesNode.toString(), propertiesSchema, null);
assertThat(validationReport.getMessages()).isEmpty();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* #%L
* Alfresco HX Insight Connector
* %%
* Copyright (C) 2023 - 2024 Alfresco Software Limited
* %%
* This file is part of the Alfresco software.
* If the software was purchased under a paid Alfresco license, the terms of
* the paid license agreement will prevail. Otherwise, the software is
* provided under the following open source license terms:
*
* Alfresco is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Alfresco is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
* #L%
*/
package org.alfresco.hxi_connector.live_ingester.util.insight_api;

import java.util.Map;

public record HxInsightRequest(String url, Map<String, String> headers, String body)
{}
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* #%L
* Alfresco HX Insight Connector
* %%
* Copyright (C) 2023 - 2024 Alfresco Software Limited
* %%
* This file is part of the Alfresco software.
* If the software was purchased under a paid Alfresco license, the terms of
* the paid license agreement will prevail. Otherwise, the software is
* provided under the following open source license terms:
*
* Alfresco is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Alfresco is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
* #L%
*/
package org.alfresco.hxi_connector.live_ingester.util.insight_api;

import java.io.IOException;
import java.io.InputStream;
import java.io.UncheckedIOException;
import java.util.HashMap;
import java.util.Map;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
import lombok.AccessLevel;
import lombok.NoArgsConstructor;

import org.alfresco.hxi_connector.live_ingester.domain.exception.LiveIngesterRuntimeException;

@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class RequestLoader
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
{
private final static ObjectMapper JSON_MAPPER = new ObjectMapper();
private final static ObjectMapper YAML_MAPPER = new ObjectMapper(new YAMLFactory());

public static HxInsightRequest load(String path)
{
Map<String, Object> data;
try (InputStream stream = HxInsightRequest.class.getResourceAsStream(path))
{
data = YAML_MAPPER.readValue(stream.readAllBytes(), HashMap.class);
}
catch (IOException e)
{
throw new UncheckedIOException(e);
}
Object body = data.get("body");
String bodyAsString = null;
if (body != null)
{
try
{
bodyAsString = JSON_MAPPER.writeValueAsString(body);
}
catch (JsonProcessingException e)
{
throw new LiveIngesterRuntimeException(e);
}
}
return new HxInsightRequest((String) data.get("url"), (Map) data.get("headers"), bodyAsString);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
url: /v1/ingestion-events
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if we should go this path - I'm not in favor of adding another static (json or yaml) requests - I think we should introduce some Java DTOs and use them. With introduction of these DTOs and new Mappers we could get rid of UpdateNodeEventSerializer, DeleteNodeEventSerializer and some static requests from our tests - WDYT @tpage-alfresco and @Pawel-608 ?

Copy link
Member

Choose a reason for hiding this comment

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

The idea here is just to extract the static json from e.g. CreateRequestIntegrationTest and put it in a file where two tests can access it. I think what you're suggesting is to combine the tests so that we validate the output from the Connector directly against the remote schema, but I'm concerned that if the test starts failing then we won't know if it's due to our code changes or changes in the remote schema.

Copy link
Member

Choose a reason for hiding this comment

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

Additionally I think we want to validate the exact output of the Connector (rather than just its validity), because otherwise we would pass tests by e.g. only producing delete requests.

Copy link
Contributor

@krdabrowski krdabrowski Dec 19, 2024

Choose a reason for hiding this comment

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

Ok, I get your point - the idea is to perform validation in two steps:

  1. verify if connector output meats "our requirements",
  2. verify if "our requirements" comply with OpenAPI spec

One downside what I see here is: maintain complexity - if someone will forget to update this static yaml request we might end up in situation where Connector will be sending not valid requests and we might notice it quite late

Copy link
Member

Choose a reason for hiding this comment

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

Yes - I agree. It's important that all static requests are also included in e2e tests so that we know they can be produced by the application. I can't think of a neat way to verify that.

Copy link
Member

Choose a reason for hiding this comment

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

I made a suggestion of how we could ensure we test all requests in the folder against the schema here: #767 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think, that we could leave CreateRequestIntegrationTest and others usecase.e2e tests as they are and introduce new test verifying if our code complies with OpenAPI spec. If only this new one test would fail, it would mean, that the spec has changed. While when more tests will start to fail, then someone messed something up in the code.
Our Endpoint-to-Endpoint tests are very handy and bring a lot of value presenting in a simple way what will be the outcome for different repo events, so we shouldn't change them.

headers:
authorization: string
content-type: application/json
hxp-environment: string
user-agent: string
body: [
{
"objectId": "d71dd823-82c7-477c-8490-04cb0e826e65",
"sourceId" : "alfresco-dummy-source-id-0a63de491876",
"eventType": "create",
"sourceTimestamp": 1611227656423,
"properties": {
"cm:autoVersion": {"value": true},
"createdAt": {"value": 1611227655695},
"modifiedAt": {"value" : 1611227655695},
"cm:versionType": {"value": "MAJOR"},
"aspectsNames": {"value": ["cm:versionable", "cm:auditable"]},
"cm:name": {
"value": "purchase-order-scan.doc",
"annotation" : "name"
},
"type": {"value": "cm:content"},
"createdBy": {"value": "admin"},
"modifiedBy": {"value": "admin"},
"cm:content": {
"file": {
"content-metadata": {
"size": 531152,
"name": "purchase-order-scan.doc",
"content-type": "application/msword"
}
}
},
"ALLOW_ACCESS": {"value": ["GROUP_EVERYONE"]},
"DENY_ACCESS": {"value": []}
}
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
url: /v1/ingestion-events
Copy link
Member

Choose a reason for hiding this comment

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

For this method of testing to be valid we need to ensure that we're also checking the application can output these requests. We are doing this for create-document-request.yml in CreateRequestIntegrationTest.java, but I don't think we're doing it for the other three yml files.

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've updated testUpdateRequest() in UpdateRequestIntegrationTest.java class with "/expected-hxinsight-requests/update-document-request.yml". In UpdateRequestIntegrationTest.java there are test cases which use different request body - we do not use those in OpenApi tests. DeleteRequestIntegrationTest.java class was updated with "/expected-hxinsight-requests/delete-document-request.yml". There is no body for PresignedUrls case so no need for update there.

headers:
authorization: string
content-type: application/json
hxp-environment: string
user-agent: string
body: [
{
"objectId": "d71dd823-82c7-477c-8490-04cb0e826e65",
"sourceId" : "alfresco-dummy-source-id-0a63de491876",
"eventType": "delete",
"sourceTimestamp": 1611656982995
}
]
Loading
Loading