-
Notifications
You must be signed in to change notification settings - Fork 32
feat: Add ODP GraphQL Api Interface #481
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
Changes from all commits
6034617
899fa88
b14562e
a7c66fc
889957c
f793afc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
/** | ||
* Copyright 2022, Optimizely Inc. and contributors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package com.optimizely.ab.internal; | ||
|
||
import com.optimizely.ab.config.parser.MissingJsonParserException; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public enum JsonParserProvider { | ||
GSON_CONFIG_PARSER("com.google.gson.Gson"), | ||
JACKSON_CONFIG_PARSER("com.fasterxml.jackson.databind.ObjectMapper" ), | ||
JSON_CONFIG_PARSER("org.json.JSONObject"), | ||
JSON_SIMPLE_CONFIG_PARSER("org.json.simple.JSONObject"); | ||
|
||
private static final Logger logger = LoggerFactory.getLogger(JsonParserProvider.class); | ||
|
||
private final String className; | ||
|
||
JsonParserProvider(String className) { | ||
this.className = className; | ||
} | ||
|
||
private boolean isAvailable() { | ||
try { | ||
Class.forName(className); | ||
return true; | ||
} catch (ClassNotFoundException e) { | ||
return false; | ||
} | ||
} | ||
|
||
public static JsonParserProvider getDefaultParser() { | ||
String defaultParserName = PropertyUtils.get("default_parser"); | ||
|
||
if (defaultParserName != null) { | ||
try { | ||
JsonParserProvider parser = JsonParserProvider.valueOf(defaultParserName); | ||
if (parser.isAvailable()) { | ||
logger.debug("using json parser: {}, based on override config", parser.className); | ||
return parser; | ||
} | ||
|
||
logger.warn("configured parser {} is not available in the classpath", defaultParserName); | ||
} catch (IllegalArgumentException e) { | ||
logger.warn("configured parser {} is not a valid value", defaultParserName); | ||
} | ||
} | ||
|
||
for (JsonParserProvider parser: JsonParserProvider.values()) { | ||
if (!parser.isAvailable()) { | ||
continue; | ||
} | ||
|
||
logger.info("using json parser: {}", parser.className); | ||
return parser; | ||
} | ||
|
||
throw new MissingJsonParserException("unable to locate a JSON parser. " | ||
+ "Please see <link> for more information"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
/** | ||
* Copyright 2022, Optimizely Inc. and contributors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package com.optimizely.ab.odp; | ||
|
||
import java.util.List; | ||
|
||
public interface ODPApiManager { | ||
String fetchQualifiedSegments(String apiKey, String apiEndpoint, String userKey, String userValue, List<String> segmentsToCheck); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Name already "reserved" for the method in ODPSegmentManager? This one should be fetchSegments, no? (consistency across SDKs) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not 100% sure, maybe I'm wrong... you have ODPAPIManager interface that defines fetchQualifiedSegments method, but there is also method fetchSegments in Swift PR that is located in the GraphQL API Manager. Not sure how you are overriding, but it seems smth not quite right...unless it's my weak java knowledge There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Design of java is a little different than other sdks. For all the external calls, we need to create an interface and pass it in to the core SDK from the outside. That is why i created ODPAPIManager which is an interface that will contain all the external calls such as graphQL and ODPEvents. Then i am providing a DefaultODPApiManager as a default implementation. user will use this by default but can pass in their own implementation. To me There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like you plan to open all ODP APIs here to public for customization support. It looks good for now and we can continue discussion the final apis. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this will contain all the methods that make an external call. ODPEvents calls will also go here |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
/** | ||
* Copyright 2022, Optimizely Inc. and contributors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package com.optimizely.ab.odp.parser; | ||
|
||
import java.util.List; | ||
|
||
public interface ResponseJsonParser { | ||
public List<String> parseQualifiedSegments(String responseJson); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/** | ||
* Copyright 2022, Optimizely Inc. and contributors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package com.optimizely.ab.odp.parser; | ||
|
||
import com.optimizely.ab.internal.JsonParserProvider; | ||
import com.optimizely.ab.odp.parser.impl.GsonParser; | ||
import com.optimizely.ab.odp.parser.impl.JacksonParser; | ||
import com.optimizely.ab.odp.parser.impl.JsonParser; | ||
import com.optimizely.ab.odp.parser.impl.JsonSimpleParser; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public class ResponseJsonParserFactory { | ||
private static final Logger logger = LoggerFactory.getLogger(ResponseJsonParserFactory.class); | ||
|
||
public static ResponseJsonParser getParser() { | ||
JsonParserProvider parserProvider = JsonParserProvider.getDefaultParser(); | ||
ResponseJsonParser jsonParser = null; | ||
switch (parserProvider) { | ||
case GSON_CONFIG_PARSER: | ||
jsonParser = new GsonParser(); | ||
break; | ||
case JACKSON_CONFIG_PARSER: | ||
jsonParser = new JacksonParser(); | ||
break; | ||
case JSON_CONFIG_PARSER: | ||
jsonParser = new JsonParser(); | ||
break; | ||
case JSON_SIMPLE_CONFIG_PARSER: | ||
jsonParser = new JsonSimpleParser(); | ||
break; | ||
} | ||
logger.info("Using " + parserProvider.toString() + " parser"); | ||
return jsonParser; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
/** | ||
* Copyright 2022, Optimizely Inc. and contributors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package com.optimizely.ab.odp.parser.impl; | ||
|
||
import com.google.gson.*; | ||
import com.google.gson.JsonParser; | ||
import com.optimizely.ab.odp.parser.ResponseJsonParser; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
public class GsonParser implements ResponseJsonParser { | ||
private static final Logger logger = LoggerFactory.getLogger(GsonParser.class); | ||
|
||
@Override | ||
public List<String> parseQualifiedSegments(String responseJson) { | < 10000 /tr>||
List<String> parsedSegments = new ArrayList<>(); | ||
try { | ||
JsonObject root = JsonParser.parseString(responseJson).getAsJsonObject(); | ||
|
||
if (root.has("errors")) { | ||
JsonArray errors = root.getAsJsonArray("errors"); | ||
StringBuilder logMessage = new StringBuilder(); | ||
for (int i = 0; i < errors.size(); i++) { | ||
if (i > 0) { | ||
logMessage.append(", "); | ||
} | ||
logMessage.append(errors.get(i).getAsJsonObject().get("message").getAsString()); | ||
} | ||
logger.error(logMessage.toString()); | ||
return null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Collecting error messages looks good. For other SDKs (swift and go), extracting "InvalidIdentifierException" from error "classification" and passing back to clients is important. Not sure how we can pass back this info to callers in Java. We can log in at minimum, but this may be redundant to your error log message already collecting all error messages. |
||
} | ||
|
||
JsonArray edges = root.getAsJsonObject("data").getAsJsonObject("customer").getAsJsonObject("audiences").getAsJsonArray("edges"); | ||
for (int i = 0; i < edges.size(); i++) { | ||
JsonObject node = edges.get(i).getAsJsonObject().getAsJsonObject("node"); | ||
if (node.has("state") && node.get("state").getAsString().equals("qualified")) { | ||
parsedSegments.add(node.get("name").getAsString()); | ||
} | ||
} | ||
return parsedSegments; | ||
} catch (JsonSyntaxException e) { | ||
logger.error("Error parsing qualified segments from response", e); | ||
return null; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
/** | ||
* Copyright 2022, Optimizely Inc. and contributors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package com.optimizely.ab.odp.parser.impl; | ||
|
||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import com.fasterxml.jackson.databind.JsonNode; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.optimizely.ab.odp.parser.ResponseJsonParser; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import java.util.ArrayList; | ||
|
||
import java.util.List; | ||
|
||
public class JacksonParser implements ResponseJsonParser { | ||
private static final Logger logger = LoggerFactory.getLogger(JacksonParser.class); | ||
|
||
@Override | ||
public List<String> parseQualifiedSegments(String responseJson) { | ||
ObjectMapper objectMapper = new ObjectMapper(); | ||
List<String> parsedSegments = new ArrayList<>(); | ||
JsonNode root; | ||
try { | ||
root = objectMapper.readTree(responseJson); | ||
|
||
if (root.has("errors")) { | ||
JsonNode errors = root.path("errors"); | ||
StringBuilder logMessage = new StringBuilder(); | ||
for (int i = 0; i < errors.size(); i++) { | ||
if (i > 0) { | ||
logMessage.append(", "); | ||
} | ||
logMessage.append(errors.get(i).path("message")); | ||
} | ||
logger.error(logMessage.toString()); | ||
return null; | ||
} | ||
|
||
JsonNode edges = root.path("data").path("customer").path("audiences").path("edges"); | ||
for (JsonNode edgeNode : edges) { | ||
String state = edgeNode.path("node").path("state").asText(); | ||
if (state.equals("qualified")) { | ||
parsedSegments.add(edgeNode.path("node").path("name").asText()); | ||
} | ||
} | ||
return parsedSegments; | ||
} catch (JsonProcessingException e) { | ||
logger.error("Error parsing qualified segments from response", e); | ||
return null; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
/** | ||
* Copyright 2022, Optimizely Inc. and contributors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package com.optimizely.ab.odp.parser.impl; | ||
|
||
import com.optimizely.ab.odp.parser.ResponseJsonParser; | ||
import org.json.JSONArray; | ||
import org.json.JSONException; | ||
import org.json.JSONObject; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
public class JsonParser implements ResponseJsonParser { | ||
private static final Logger logger = LoggerFactory.getLogger(JsonParser.class); | ||
|
||
@Override | ||
public List<String> parseQualifiedSegments(String responseJson) { | ||
List<String> parsedSegments = new ArrayList<>(); | ||
try { | ||
JSONObject root = new JSONObject(responseJson); | ||
|
||
if (root.has("errors")) { | ||
JSONArray errors = root.getJSONArray("errors"); | ||
StringBuilder logMessage = new StringBuilder(); | ||
for (int i = 0; i < errors.length(); i++) { | ||
if (i > 0) { | ||
logMessage.append(", "); | ||
} | ||
logMessage.append(errors.getJSONObject(i).getString("message")); | ||
} | ||
logger.error(logMessage.toString()); | ||
return null; | ||
} | ||
|
||
JSONArray edges = root.getJSONObject("data").getJSONObject("customer").getJSONObject("audiences").getJSONArray("edges"); | ||
for (int i = 0; i < edges.length(); i++) { | ||
JSONObject node = edges.getJSONObject(i).getJSONObject("node"); | ||
if (node.has("state") && node.getString("state").equals("qualified")) { | ||
parsedSegments.add(node.getString("name")); | ||
} | ||
} | ||
return parsedSegments; | ||
} catch (JSONException e) { | ||
logger.error("Error parsing qualified segments from response", e); | ||
return null; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just wondering if there is a smart way to reuse the same code for DefaultConfigParser. It is quite complicated currently supporting multiple parsers and this repeated code will make it worse :)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to reuse code from there but it was tightly coupled. I wanted to refactor DefaultConfigParser to use this one but i thought i will add a comment and do that later as a separate ticket. Right now it will become too much work. what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to make DefaultConfigParser and whatever else needs a parser use this code eventually. i am just too afraid to touch DafaultConfigParser in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I leave it to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! I will add a comment in the DefaultConfigParser mentioning the same. I will also add a ticket to refactor this and put it in the tech debt backlog.