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

Refactor ApiDefinitionParser to fix issue #23 #25

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
90 changes: 51 additions & 39 deletions src/main/java/se/ams/dcatprocessor/parser/ApiDefinitionParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,57 +38,69 @@ public class ApiDefinitionParser {

private static Logger logger = LoggerFactory.getLogger(ApiDefinitionParser.class);

public enum ApiSpecSyntax {
Json,
YamlRaml,
Unknown
};

public static JSONObject getApiJsonString(String fileString) throws IOException, ParseException {
JSONParser parser = new JSONParser();
Stream<String> lines;
String apiJsonString = "";
JSONObject fullObj = parseToJsonObject(fileString);
JSONObject inner = getIn(fullObj, new String[]{"info", "x-dcat"});
return inner == null? fullObj : inner;
}

private static JSONObject getIn(JSONObject obj, String[] path) {
for (String k: path) {
obj = (JSONObject)obj.get(k);
if (obj == null) {
return null;
}
}
return obj;
}

public static ApiSpecSyntax guessSyntax(String fileString) {
String trimmedString = fileString.trim();
if (trimmedString.startsWith("{") && trimmedString.endsWith("}")) {
return ApiSpecSyntax.Json;
}

Stream<String> lines;
lines = fileString.lines();
String apiLine1 = lines.limit(1).collect(Collectors.joining("\n"));

if (apiLine1.contains("openapi")) {
apiJsonString = getFileApiYamlRaml(fileString);
} else if (apiLine1.contains("RAML")) {
apiJsonString = getFileApiYamlRaml(fileString);
} else if (apiLine1.contains("{")){
apiJsonString = fileString;
}
else {
throw new DcatException("not supported api definition");
if (apiLine1.contains("RAML") || apiLine1.contains("openapi")) {
return ApiSpecSyntax.YamlRaml;
}
JSONObject jsonObjectFile = (JSONObject) parser.parse(apiJsonString);
if (jsonObjectFile.containsKey("info")) {
jsonObjectFile = (org.json.simple.JSONObject) (jsonObjectFile.get("info"));
jsonObjectFile = (org.json.simple.JSONObject) (jsonObjectFile.get("x-dcat"));

return ApiSpecSyntax.Unknown;
}

private static JSONObject parseToJsonObject(String fileString) throws IOException, ParseException {
switch (guessSyntax(fileString)) {
case Json: return (JSONObject)((new JSONParser()).parse(fileString));
case YamlRaml: return convertYamlRamlToJsonObject(fileString);
case Unknown: {throw new DcatException("not supported api definition");}
}
return jsonObjectFile;
return null;
}

private static String getFileApiYamlRaml(String apiSpec) throws IOException {
FileOutputStream output = new FileOutputStream("output.json");
private static JSONObject convertYamlRamlToJsonObject(String apiSpec) throws IOException, ParseException {
FileOutputStream output = new FileOutputStream("output.json"); // TODO: Could we use a ByteArrayOutputStream for this, instead?
JsonFactory factory = new JsonFactory();
JsonGenerator generator = factory.createGenerator(output, JsonEncoding.UTF8);

try {
Copy link
Author

@jonasseglare jonasseglare May 25, 2023

Choose a reason for hiding this comment

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

Do I need to keep the try-catch here or can I throw it away? Cathing it and printing the stack trace potentially hides potential bugs in my opinion.

Maybe I should keep it in this PR in order to make the changes focused.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good and clear PR. I think it should be accepted immediately. We are trying to find a good working method for contributions. I just need to get the team to verify that the bug fixes and improvements are working for everyone. Is it Ok if it takes a few more days? Agree, don't hide exceptions. Remove the silent swallowing of the exception.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, no problem, these changes are not very urgent so take your time :-) I ran across the issues that these PRs address while using the DCAT-AP-SE-Processor from another code base and though I should fix them.

Yaml yamlParser = new Yaml();
yamlParser.addImplicitResolver(Tag.YAML, Pattern.compile("^(!)$"), "!");
Node compose = yamlParser.compose(new StringReader(apiSpec));
build(compose, generator);
generator.close();
output.close();
Yaml yamlParser = new Yaml();
yamlParser.addImplicitResolver(Tag.YAML, Pattern.compile("^(!)$"), "!");
Node compose = yamlParser.compose(new StringReader(apiSpec));
build(compose, generator);
generator.close();
output.close();

JSONParser jsonParser = new JSONParser();
FileReader reader = new FileReader("output.json");

//Read JSON file
Object obj = jsonParser.parse(reader);
return obj.toString();
//logger.debug("JSON string from file:\n"+obj.toString());

} catch (IOException | ParseException e) {
e.printStackTrace();
}
return "";
JSONParser jsonParser = new JSONParser();
FileReader reader = new FileReader("output.json");
return (JSONObject)jsonParser.parse(reader);
}

private static void build(Node yaml, JsonGenerator generator) throws IOException {
Expand Down Expand Up @@ -126,4 +138,4 @@ private static void build(Node yaml, JsonGenerator generator) throws IOException
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -195,24 +195,24 @@ void setup() {
parser = new ApiDefinitionParser();
}

//TODO: Fixa så att testet funkar
// Helper method
void checkParse(String input, ApiDefinitionParser.ApiSpecSyntax expectedSyntax, String expectedOutput) throws Exception {
assertEquals(expectedSyntax, parser.guessSyntax(input));
assertEquals(expectedOutput, parser.getApiJsonString(input).toString());
}

@Test
void testMandatoryRamlApi() throws Exception {
try {
String result = parser.getApiJsonString(ramlApi).toString();
assertEquals(result, ramlResult);
} catch (DcatException e) {

}
checkParse(ramlApi, ApiDefinitionParser.ApiSpecSyntax.YamlRaml, ramlResult);
}

@Test
void testMandatoryJsonApi() throws Exception {
try {
String result = parser.getApiJsonString(jsonApi).toString();
assertEquals(result, jsonresult);
} catch (DcatException e) {
checkParse(jsonApi, ApiDefinitionParser.ApiSpecSyntax.Json, jsonresult);
}

}
@Test
void testMandatoryJsonApiCompactFormatting() throws Exception {
checkParse(jsonApi.replace("\n", ""), ApiDefinitionParser.ApiSpecSyntax.Json, jsonresult);
}
}