Skip to content

feat(cts): add custom request tests #331

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

Merged
merged 12 commits into from
Apr 14, 2022
Merged

feat(cts): add custom request tests #331

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a56fbab
feat(cts): add custom request tests
shortcuts Mar 31, 2022
f954028
Merge branch 'main' into feat/add-customRequest-tests
shortcuts Apr 12, 2022
7cb03cb
lint
shortcuts Apr 12, 2022
a874de4
Merge branch 'main' into feat/add-customRequest-tests
shortcuts Apr 12, 2022
cf915d5
remove `requestBody` from `DELETE` operations
shortcuts Apr 13, 2022
0daa016
Merge branch 'main' into feat/add-customRequest-tests
shortcuts Apr 13, 2022
b0cf8eb
fix php path
shortcuts Apr 13, 2022
e3a9a36
remove echo, remove model cache
shortcuts Apr 13, 2022
86d1cdf
cache was already used??
shortcuts Apr 13, 2022
19b0aaa
Merge branch 'main' into feat/add-customRequest-tests
shortcuts Apr 14, 2022
4efdcd8
fix wrong path
shortcuts Apr 14, 2022
42ed0a5
remove Api from createClientName
shortcuts Apr 14, 2022
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
16 changes: 16 additions & 0 deletions generators/src/main/java/com/algolia/codegen/cts/AlgoliaCtsGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,9 @@ public Map<String, Object> postProcessSupportingFileData(
System.out.println(e.getMessage());
System.exit(0);
}

System.out.println(e.getMessage());
Copy link
Collaborator

Choose a reason for hiding this comment

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

huge fix !

System.exit(1);
} catch (Exception e) {
e.printStackTrace();
System.exit(1);
Expand All @@ -181,15 +184,28 @@ private Map<String, Request[]> loadCTS()
throws JsonParseException, JsonMappingException, IOException, CTSException {
TreeMap<String, Request[]> cts = new TreeMap<>();
File dir = new File("tests/CTS/methods/requests/" + client);
File commonTestDir = new File("tests/CTS/methods/requests/common");
if (!dir.exists()) {
throw new CTSException("CTS not found at " + dir.getAbsolutePath(), true);
}
if (!commonTestDir.exists()) {
throw new CTSException(
"CTS not found at " + commonTestDir.getAbsolutePath(),
true
);
}
for (File f : dir.listFiles()) {
cts.put(
f.getName().replace(".json", ""),
Json.mapper().readValue(f, Request[].class)
);
}
for (File f : commonTestDir.listFiles()) {
cts.put(
f.getName().replace(".json", ""),
Json.mapper().readValue(f, Request[].class)
);
}
return cts;
}

Expand Down
43 changes: 36 additions & 7 deletions generators/src/main/java/com/algolia/codegen/cts/ParametersWithDataType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,13 @@ private void handleModel(
String parent,
int suffix
) throws CTSException {
assert (spec.getHasVars());
assert (spec.getItems() == null);
if (!spec.getHasVars()) {
throw new CTSException("Spec has no vars.");
}

if (spec.getItems() != null) {
throw new CTSException("Spec has items.");
}

if (
spec instanceof CodegenModel && ((CodegenModel) spec).oneOf.size() > 0
Expand Down Expand Up @@ -269,8 +274,13 @@ private void handleObject(
IJsonSchemaValidationProperties spec,
int suffix
) throws CTSException {
assert (!spec.getHasVars());
assert (spec.getItems() == null);
if (spec.getHasVars()) {
throw new CTSException("Spec has vars.");
}

if (spec.getItems() != null) {
throw new CTSException("Spec has items.");
}

Map<String, Object> vars = (Map<String, Object>) param;

Expand Down Expand Up @@ -300,18 +310,37 @@ private void handleMap(
IJsonSchemaValidationProperties spec,
int suffix
) throws CTSException {
assert (!spec.getHasVars());
assert (spec.getItems() != null);
if (spec.getHasVars()) {
throw new CTSException("Spec has vars.");
}

Map<String, Object> vars = (Map<String, Object>) param;

List<Object> values = new ArrayList<>();

CodegenProperty items = spec.getItems();

for (Entry<String, Object> entry : vars.entrySet()) {
IJsonSchemaValidationProperties itemType = items;

// The generator consider a free form object type as an `object`, which
// is wrong in our case, so we infer it.
if (
items == null ||
(items.openApiType.equals("object") && items.isFreeFormObject)
) {
CodegenParameter maybeMatch = new CodegenParameter();
String paramType = inferDataType(entry.getValue(), maybeMatch, null);

maybeMatch.dataType = paramType;
itemType = maybeMatch;
}

values.add(
traverseParams(
entry.getKey(),
entry.getValue(),
spec.getItems(),
itemType,
paramName,
suffix + 1
)
Expand Down
36 changes: 36 additions & 0 deletions tests/CTS/methods/requests/common/del.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
[
{
"method": "del",
"testName": "allow del method for a custom path with minimal parameters",
"parameters": {
"path": "/test/minimal"
},
"request": {
"path": "/1/test/minimal",
"method": "DELETE"
}
},
{
"method": "del",
"testName": "allow del method for a custom path with all parameters",
"parameters": {
"path": "/test/all",
"parameters": {
"query": "parameters"
},
"body": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

DELETE method should not have body, from the http spec:

A payload within a DELETE request message has no defined semantics;
sending a payload body on a DELETE request might cause some existing
implementations to reject the request.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! It's not the case for the current clients, so I think our API handles it. We can still remove it if we think it shouldn't be the case

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, other DELETE operations we have does not have a request body, so I'll remove it 👍🏼

Copy link
Collaborator

Choose a reason for hiding this comment

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

I know that algolia API can be non-standard sometimes but I hope they don't break the http specification at least

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe we just missed that part in the custom request implementation of the current clients

Copy link
Member Author

Choose a reason for hiding this comment

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

Done here: cf915d5

"body": "parameters"
}
},
"request": {
"path": "/1/test/all",
"method": "DELETE",
"data": {
"body": "parameters"
},
"searchParams": {
"query": "parameters"
}
}
}
]
30 changes: 30 additions & 0 deletions tests/CTS/methods/requests/common/get.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
[
{
"method": "get",
"testName": "allow get method for a custom path with minimal parameters",
"parameters": {
"path": "/test/minimal"
},
"request": {
"path": "/1/test/minimal",
"method": "GET"
}
},
{
"method": "get",
"testName": "allow get method for a custom path with all parameters",
"parameters": {
"path": "/test/all",
"parameters": {
"query": "parameters"
}
},
"request": {
"path": "/1/test/all",
"method": "GET",
"searchParams": {
"query": "parameters"
}
}
}
]
36 changes: 36 additions & 0 deletions tests/CTS/methods/requests/common/post.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
[
{
"method": "post",
"testName": "allow post method for a custom path with minimal parameters",
"parameters": {
"path": "/test/minimal"
},
"request": {
"path": "/1/test/minimal",
"method": "POST"
}
},
{
"method": "post",
"testName": "allow post method for a custom path with all parameters",
"parameters": {
"path": "/test/all",
"parameters": {
"query": "parameters"
},
"body": {
"body": "parameters"
}
},
"request": {
"path": "/1/test/all",
"method": "POST",
"data": {
"body": "parameters"
},
"searchParams": {
"query": "parameters"
}
}
}
]
36 changes: 36 additions & 0 deletions tests/CTS/methods/requests/common/put.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
[
{
"method": "put",
"testName": "allow put method for a custom path with minimal parameters",
"parameters": {
"path": "/test/minimal"
},
"request": {
"path": "/1/test/minimal",
"method": "PUT"
}
},
{
"method": "put",
"testName": "allow put method for a custom path with all parameters",
"parameters": {
"path": "/test/all",
"parameters": {
"query": "parameters"
},
"body": {
"body": "parameters"
}
},
"request": {
"path": "/1/test/all",
"method": "PUT",
"data": {
"body": "parameters"
},
"searchParams": {
"query": "parameters"
}
}
}
]
4 changes: 2 additions & 2 deletions tests/CTS/methods/requests/templates/java/requests.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ class {{client}}Tests {

{{#request.searchParams}}
HashMap<String, String> expectedQuery = JSON.deserialize("{{#lambda.escapequotes}}{{{request.searchParams}}}{{/lambda.escapequotes}}", new TypeToken<HashMap<String, String>>() {}.getType());
List<Pair> acutalQuery = req.getQueryParams();
for (Pair p : acutalQuery) {
List<Pair> actualQuery = req.getQueryParams();
Copy link
Collaborator

Choose a reason for hiding this comment

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

oops I have the same code here, you will merge first unfortunately :(

Copy link
Member Author

Choose a reason for hiding this comment

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

D:

for (Pair p : actualQuery) {
assertEquals(expectedQuery.get(p.getName()), p.getValue());
}
{{/request.searchParams}}
Expand Down
Loading