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.

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

Jump to bottom

Remove org.json dependency #416

Merged
merged 5 commits into from
Jul 19, 2017
Merged

Remove org.json dependency #416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
617327f
Remove org.json dependency, udpate changelog
kevinhinterlong Jun 28, 2017
3f497ac
Update javadoc comments
kevinhinterlong Jun 28, 2017
809b3f5
Address comments
kevinhinterlong Jul 7, 2017
60e766c
Fix weird merge error
kevinhinterlong Jul 11, 2017
7acdf67
Merge branch 'master' into removeOrgJsonDependency
garyluoex Jul 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ Current

### Removed:

- [Remove dependency on org.json](https://github.com/yahoo/fili/pull/416)
* Replace uses of org.json with the jackson equivalent


v0.8.69 - 2017/06/06
Expand Down
5 changes: 0 additions & 5 deletions fili-core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,6 @@
<groupId>com.fasterxml.jackson.dataformat</groupId>
<artifactId>jackson-dataformat-csv</artifactId>
</dependency>
<dependency>
<!--TODO: switch back to jackson parser-->
<groupId>org.json</groupId>
<artifactId>json</artifactId>
</dependency>

<!-- Redis -->
<dependency>
Expand Down
32 changes: 14 additions & 18 deletions fili-core/src/main/java/com/yahoo/bard/webservice/web/DataApiRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@
import com.yahoo.bard.webservice.web.util.BardConfigResources;
import com.yahoo.bard.webservice.web.util.PaginationParameters;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;

import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.Duration;
import org.joda.time.Interval;
import org.joda.time.Period;
import org.joda.time.format.DateTimeFormatter;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -802,7 +802,7 @@ protected LinkedHashSet<LogicalMetric> generateLogicalMetrics(
//If INTERSECTION_REPORTING_ENABLED flag is true, convert the aggregators into FilteredAggregators and
//replace old PostAggs with new postAggs in order to generate a new Filtered Logical Metric
if (BardFeatureFlag.INTERSECTION_REPORTING.isOn()) {
JSONArray metricsJsonArray;
ArrayNode metricsJsonArray;
try {
//For a given metricString, returns an array of json objects contains metric name and associated
// filters
Expand All @@ -811,33 +811,29 @@ protected LinkedHashSet<LogicalMetric> generateLogicalMetrics(
} catch (IllegalArgumentException e) {
LOG.debug(INCORRECT_METRIC_FILTER_FORMAT.logFormat(e.getMessage()));
throw new BadApiRequestException(INCORRECT_METRIC_FILTER_FORMAT.format(apiMetricQuery));
} catch (JSONException e) {
// This needs to stay here due to a bytecode issue where Java 8 flags JSONException as invalid
LOG.debug(INCORRECT_METRIC_FILTER_FORMAT.logFormat(e.getMessage()));
throw new BadApiRequestException(INCORRECT_METRIC_FILTER_FORMAT.format(apiMetricQuery));
}
//check for the duplicate occurrence of metrics in an API
FieldConverterSupplier.metricsFilterSetBuilder.validateDuplicateMetrics(metricsJsonArray);
for (int i = 0; i < metricsJsonArray.length(); i++) {
JSONObject jsonObject;
for (int i = 0; i < metricsJsonArray.size(); i++) {
JsonNode jsonObject;
try {
jsonObject = metricsJsonArray.getJSONObject(i);
} catch (JSONException e) {
jsonObject = metricsJsonArray.get(i);
} catch (IndexOutOfBoundsException e) {
LOG.debug(INCORRECT_METRIC_FILTER_FORMAT.logFormat(e.getMessage()));
throw new BadApiRequestException(INCORRECT_METRIC_FILTER_FORMAT.format(apiMetricQuery));
}
String metricName = jsonObject.getString("name");
String metricName = jsonObject.get("name").asText();
LogicalMetric logicalMetric = metricDictionary.get(metricName);

// If metric dictionary returns a null, it means the requested metric is not found.
if (logicalMetric == null) {
invalidMetricNames.add(metricName);
} else {
//metricFilterObject contains all the filters for a given metric
JSONObject metricFilterObject = jsonObject.getJSONObject("filter");
JsonNode metricFilterObject = jsonObject.get("filter");

//Currently supporting AND operation for metric filters.
if (!metricFilterObject.isNull("AND")) {
if (metricFilterObject.has("AND") && !metricFilterObject.get("AND").isNull()) {

//We currently do not support ratio metrics
if (logicalMetric.getCategory().equals(RATIO_METRIC_CATEGORY)) {
Expand Down Expand Up @@ -868,10 +864,10 @@ protected LinkedHashSet<LogicalMetric> generateLogicalMetrics(
}

//If metric filter isn't empty or it has anything other then 'AND' then throw an exception
} else if (!(metricFilterObject.toString().equals("{}"))) {
LOG.debug(INVALID_METRIC_FILTER_CONDITION.logFormat(metricFilterObject.keySet()));
} else if (!metricFilterObject.asText().isEmpty()) {
LOG.debug(INVALID_METRIC_FILTER_CONDITION.logFormat(metricFilterObject.asText()));
throw new BadApiRequestException(
INVALID_METRIC_FILTER_CONDITION.format(metricFilterObject.keySet())
INVALID_METRIC_FILTER_CONDITION.format(metricFilterObject.asText())
);
}
generated.add(logicalMetric);
Expand Down
21 changes: 11 additions & 10 deletions fili-core/src/main/java/com/yahoo/bard/webservice/web/FilteredSketchMetricsHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@
import com.yahoo.bard.webservice.druid.model.postaggregation.WithFields;
import com.yahoo.bard.webservice.table.LogicalTable;

import org.json.JSONArray;
import org.json.JSONObject;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -50,12 +51,12 @@ public class FilteredSketchMetricsHelper implements MetricsFilterSetBuilder {
private static final String ALPHANUMERIC_REGEX = "[^a-zA-Z0-9]";

@Override
public void validateDuplicateMetrics(JSONArray metricsJsonArray) {
public void validateDuplicateMetrics(ArrayNode metricsJsonArray) {
Set<String> metricsList = new HashSet<>();
List<String> duplicateMetrics = new ArrayList<>();

for (int i = 0; i < metricsJsonArray.length(); i++) {
String metricName = metricsJsonArray.getJSONObject(i).getString("name");
for (int i = 0; i < metricsJsonArray.size(); i++) {
String metricName = metricsJsonArray.get(i).get("name").asText();
boolean status = metricsList.add(metricName);
if (!status) {
duplicateMetrics.add(metricName);
Expand All @@ -70,7 +71,7 @@ public void validateDuplicateMetrics(JSONArray metricsJsonArray) {
@Override
public LogicalMetric getFilteredLogicalMetric(
LogicalMetric logicalMetric,
JSONObject metricFilterObject,
JsonNode metricFilterObject,
DimensionDictionary dimensionDictionary,
LogicalTable table,
DataApiRequest apiRequest
Expand All @@ -88,7 +89,7 @@ public LogicalMetric getFilteredLogicalMetric(
apiRequest
);

String filterSuffix = "-" + metricFilterObject.get("AND").toString().replaceAll(ALPHANUMERIC_REGEX, "");
String filterSuffix = "-" + metricFilterObject.get("AND").asText().replaceAll(ALPHANUMERIC_REGEX, "");

//innerPostAggToOuterAggMap stores the mapping of old postAgg to new postAgg name. In filtering of metrics,
//the name of postAggs(all postAggs other than CONSTANT type) in the nested query is appended with a filter
Expand Down Expand Up @@ -239,7 +240,7 @@ public Collection<PostAggregation> updateNestedQueryPostAggs(
@Override
public TemplateDruidQuery updateTemplateDruidQuery(
TemplateDruidQuery query,
JSONObject metricFilterObject,
JsonNode metricFilterObject,
DimensionDictionary dimensionDictionary,
LogicalTable table,
DataApiRequest apiRequest
Expand Down Expand Up @@ -414,14 +415,14 @@ public PostAggregation replacePostAggWithPostAggFromMap(

@Override
public Set<FilteredAggregation> getFilteredAggregation(
JSONObject filter,
JsonNode filter,
Aggregation aggregation,
DimensionDictionary dimensionDictionary,
LogicalTable table,
DataApiRequest apiRequest
) throws DimensionRowNotFoundException {
//Converting json filter string to a plain filter string to prepare the Filter out of it
String filterString = filter.get("AND").toString().replace("],", "]],");
String filterString = filter.get("AND").asText().replace("],", "]],");
String[] filterList = filterString.split("],");
Set<FilteredAggregation> filteredAggregationSet = new HashSet<>();
Map<String, Filter> filterHashMap = new HashMap<>();
Expand Down
21 changes: 11 additions & 10 deletions fili-core/src/main/java/com/yahoo/bard/webservice/web/FilteredThetaSketchMetricsHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@
import com.yahoo.bard.webservice.druid.model.postaggregation.WithFields;
import com.yahoo.bard.webservice.table.LogicalTable;

import org.json.JSONArray;
import org.json.JSONObject;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -47,12 +48,12 @@ public class FilteredThetaSketchMetricsHelper implements MetricsFilterSetBuilder


@Override
public void validateDuplicateMetrics(JSONArray metricsJsonArray) {
public void validateDuplicateMetrics(ArrayNode metricsJsonArray) {
Set<String> metricsList = new HashSet<>();
List<String> duplicateMetrics = new ArrayList<>();

for (int i = 0; i < metricsJsonArray.length(); i++) {
String metricName = metricsJsonArray.getJSONObject(i).getString("name");
for (int i = 0; i < metricsJsonArray.size(); i++) {
String metricName = metricsJsonArray.get(i).get("name").asText();
boolean status = metricsList.add(metricName);
if (!status) {
duplicateMetrics.add(metricName);
Expand All @@ -67,7 +68,7 @@ public void validateDuplicateMetrics(JSONArray metricsJsonArray) {
@Override
public LogicalMetric getFilteredLogicalMetric(
LogicalMetric logicalMetric,
JSONObject metricFilterObject,
JsonNode metricFilterObject,
DimensionDictionary dimensionDictionary,
LogicalTable table,
DataApiRequest apiRequest
Expand All @@ -85,7 +86,7 @@ public LogicalMetric getFilteredLogicalMetric(
apiRequest
);

String filterSuffix = "-" + metricFilterObject.get("AND").toString().replaceAll(ALPHANUMERIC_REGEX, "");
String filterSuffix = "-" + metricFilterObject.get("AND").asText().replaceAll(ALPHANUMERIC_REGEX, "");

//innerPostAggToOuterAggMap stores the mapping of old postAgg to new postAgg name. In filtering of metrics,
//the name of postAggs(all postAggs other than CONSTANT type) in the nested query is appended with a filter
Expand Down Expand Up @@ -207,7 +208,7 @@ public Collection<PostAggregation> updateNestedQueryPostAggs(
@Override
public TemplateDruidQuery updateTemplateDruidQuery(
TemplateDruidQuery query,
JSONObject metricFilterObject,
JsonNode metricFilterObject,
DimensionDictionary dimensionDictionary,
LogicalTable table,
DataApiRequest apiRequest
Expand Down Expand Up @@ -382,14 +383,14 @@ public PostAggregation replacePostAggWithPostAggFromMap(

@Override
public Set<FilteredAggregation> getFilteredAggregation(
JSONObject filter,
JsonNode filter,
Aggregation aggregation,
DimensionDictionary dimensionDictionary,
LogicalTable table,
DataApiRequest apiRequest
) throws DimensionRowNotFoundException {
//Converting json filter string to a plain filter string to prepare the Filter out of it
String filterString = filter.get("AND").toString().replace("],", "]],");
String filterString = filter.get("AND").asText().replace("],", "]],");
String[] filterList = filterString.split("],");
Set<FilteredAggregation> filteredAggregationSet = new HashSet<>();
Map<String, Filter> filterHashMap = new HashMap<>();
Expand Down
51 changes: 24 additions & 27 deletions fili-core/src/main/java/com/yahoo/bard/webservice/web/MetricParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
// Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms.
package com.yahoo.bard.webservice.web;

import org.json.JSONArray;
import org.json.JSONException;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
import com.fasterxml.jackson.databind.node.ObjectNode;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Stack;

Expand All @@ -21,7 +21,7 @@ public class MetricParser {
private static final Logger LOG = LoggerFactory.getLogger(MetricParser.class);

/**
* This function converts the string of metrics extracted from the url into JSONArray.
* This function converts the string of metrics extracted from the url into ArrayNode.
*
* @param metricString An Api metric string eg:
* <pre>{@code metricString = metric1(AND(dim1|id-in[a,b],dim2|id-in[c,d])),metric2 }</pre>
Expand All @@ -30,10 +30,9 @@ public class MetricParser {
* <pre>{@code {[{"filter":{"AND":{"dim2|id-in":["abc","xyz"],"dim3|id-in":["mobile","tablet"]}}, "name":"metric1"},
* {"filter":{},"name":"metric2"}]}}</pre>

* @throws JSONException if a JSON cannot be constructed successfully from the given metricString
* @throws IllegalArgumentException if metricString is empty or the metricString has unbalanced brackets
*/
public static JSONArray generateMetricFilterJsonArray(String metricString) throws JSONException {
public static ArrayNode generateMetricFilterJsonArray(String metricString) {
if (metricString.length() == 0 || !(isBracketsBalanced(metricString))) {
LOG.error("Metrics parameter values are invalid. The string is: " + metricString);
throw new IllegalArgumentException("Metrics parameter values are invalid. The string is: " + metricString);
Expand All @@ -45,42 +44,40 @@ public static JSONArray generateMetricFilterJsonArray(String metricString) throw
String[] metrics = encodeMetricFilters(metricString, mapper).split(",");

//looping each metric in a metrics array
List<String> metricList = new ArrayList<>();
ArrayNode arrayNode = JsonNodeFactory.instance.arrayNode();
for (String metric : metrics) {
//Separating metric and '-' separated filter encode metricFilterArray[0] will be metric name
//and metricFilterArray[1] will be key of the mapper contains respective filter string of the metric
String[] metricFilterArray = metric.split("-");
//Retrieving the filter string from the mapper and creating a metric set with metric as key
//and filter as value
String metricFilter = metricFilterArray.length > 1 ? mapper.get("-" + metricFilterArray[1]) : "";
metricList.add(getJsonString(metricFilterArray[0], metricFilter));
}
StringBuilder finalJson = new StringBuilder();
//preparing complete json string by appending brackets to metric name and metric filter
for (String jsonToken : metricList) {
finalJson.append("{" + jsonToken + "},");

ObjectNode objectNode = arrayNode.objectNode();
addJsonFilter(objectNode, metricFilterArray[0], metricFilter);
arrayNode.add(objectNode);
}
return new JSONArray("[" + finalJson.toString().substring(0, finalJson.toString().length() - 1) + "]");

return arrayNode;
}

/**
* Returns a String that is formatted to a JSON string.
* Add the name and filter of a metric to an ObjectNode.
* For eg: {"filter":{"AND":"property|id-in:[14,125],country|id-in:[US,IN]},"name":"foo"}
*
* @param metricNode the object node to fill with the name and filter on a metric
* @param metricName the name of the metric
* @param metricFilter the filter associated with the metric. Could be empty or have a value
*
* @return A String in JSON format for a given metric and filter.
* For eg: "filter":{"AND":"property|id-in:[14,125],country|id-in:[US,IN]},"name":"foo"
*/
private static String getJsonString(String metricName, String metricFilter) {
if (metricFilter.isEmpty()) {
metricFilter = "\"filter\": {}";
} else {
metricFilter = "\"filter\":{\"AND\":\"" +
metricFilter.replace("AND", "").replace("(", "").replace(")", "") + "\" }";

private static void addJsonFilter(ObjectNode metricNode, String metricName, String metricFilter) {
metricNode.put("name", metricName);
ObjectNode filterNode = metricNode.putObject("filter");
if (!metricFilter.isEmpty()) {
metricFilter = metricFilter.replace("AND", "")
.replace("(", "")
.replace(")", "");
filterNode.put("AND", metricFilter);
}
return "\"name\": \"" + metricName + "\"," + metricFilter;
}

/**
Expand Down
12 changes: 6 additions & 6 deletions fili-core/src/main/java/com/yahoo/bard/webservice/web/MetricsFilterSetBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
import com.yahoo.bard.webservice.druid.model.postaggregation.SketchSetOperationPostAggFunction;
import com.yahoo.bard.webservice.table.LogicalTable;

import org.json.JSONArray;
import org.json.JSONObject;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;

import java.util.Collection;
import java.util.List;
Expand All @@ -38,7 +38,7 @@ public interface MetricsFilterSetBuilder {
* @throws BadApiRequestException Invalid metric query if the metric query has
* duplicate metrics
*/
void validateDuplicateMetrics(JSONArray metricsJsonArray) throws BadApiRequestException;
void validateDuplicateMetrics(ArrayNode metricsJsonArray) throws BadApiRequestException;

/**
* Provides filter wrapped logical metric for the given logical metric.
Expand All @@ -56,7 +56,7 @@ public interface MetricsFilterSetBuilder {
*/
LogicalMetric getFilteredLogicalMetric(
LogicalMetric logicalMetric,
JSONObject metricFilterObject,
JsonNode metricFilterObject,
DimensionDictionary dimensionDictionary,
LogicalTable table,
DataApiRequest apiRequest
Expand Down Expand Up @@ -125,7 +125,7 @@ Collection<PostAggregation> updateNestedQueryPostAggs(
*/
TemplateDruidQuery updateTemplateDruidQuery(
TemplateDruidQuery query,
JSONObject metricFilterObject,
JsonNode metricFilterObject,
DimensionDictionary dimensionDictionary,
LogicalTable table,
DataApiRequest apiRequest
Expand Down Expand Up @@ -177,7 +177,7 @@ PostAggregation replacePostAggWithPostAggFromMap(
* filter is not found.
*/
Set<FilteredAggregation> getFilteredAggregation(
JSONObject filter,
JsonNode filter,
Aggregation aggregation,
DimensionDictionary dimensionDictionary,
LogicalTable table,
Expand Down