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

Making filter immutable, add filter to TableQuery, Rename RawStringFilte... #41

Merged
merged 2 commits into from
Apr 9, 2012
Merged

Making filter immutable, add filter to TableQuery, Rename RawStringFilte... #41

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8dd4620
Making filter immutable, add filter to TableQuery, Rename RawStringFi…
Apr 7, 2012
4d82414
Addressing code review feedback...
Apr 9, 2012
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
54 changes: 31 additions & 23 deletions ...rc/main/java/com/microsoft/windowsazure/services/table/implementation/TableRestProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,12 @@
import com.microsoft.windowsazure.services.table.models.GetServicePropertiesResult;
import com.microsoft.windowsazure.services.table.models.GetTableResult;
import com.microsoft.windowsazure.services.table.models.InsertEntityResult;
import com.microsoft.windowsazure.services.table.models.LitteralFilter;
import com.microsoft.windowsazure.services.table.models.Query;
import com.microsoft.windowsazure.services.table.models.PropertyNameFilter;
import com.microsoft.windowsazure.services.table.models.QueryEntitiesOptions;
import com.microsoft.windowsazure.services.table.models.QueryEntitiesResult;
import com.microsoft.windowsazure.services.table.models.QueryStringFilter;
import com.microsoft.windowsazure.services.table.models.QueryTablesOptions;
import com.microsoft.windowsazure.services.table.models.QueryTablesResult;
import com.microsoft.windowsazure.services.table.models.RawStringFilter;
import com.microsoft.windowsazure.services.table.models.ServiceProperties;
import com.microsoft.windowsazure.services.table.models.TableServiceOptions;
import com.microsoft.windowsazure.services.table.models.UnaryFilter;
Expand Down Expand Up @@ -182,26 +181,29 @@ private WebResource addOptionalQueryParam(WebResource webResource, String key, O
return PipelineHelpers.addOptionalQueryParam(webResource, key, value);
}

private WebResource addOptionalQuery(WebResource webResource, Query query) {
if (query == null)
private WebResource addOptionalQueryEntitiesOptions(WebResource webResource,
QueryEntitiesOptions queryEntitiesOptions) {
if (queryEntitiesOptions == null)
return webResource;

if (query.getSelectFields() != null && query.getSelectFields().size() > 0) {
if (queryEntitiesOptions.getSelectFields() != null && queryEntitiesOptions.getSelectFields().size() > 0) {
webResource = addOptionalQueryParam(webResource, "$select",
CommaStringBuilder.join(encodeODataURIValues(query.getSelectFields())));
CommaStringBuilder.join(encodeODataURIValues(queryEntitiesOptions.getSelectFields())));
}

if (query.getTop() != null) {
webResource = addOptionalQueryParam(webResource, "$top", encodeODataURIValue(query.getTop().toString()));
if (queryEntitiesOptions.getTop() != null) {
webResource = addOptionalQueryParam(webResource, "$top", encodeODataURIValue(queryEntitiesOptions.getTop()
.toString()));
}

if (query.getFilter() != null) {
webResource = addOptionalQueryParam(webResource, "$filter", buildFilterExpression(query.getFilter()));
if (queryEntitiesOptions.getFilter() != null) {
webResource = addOptionalQueryParam(webResource, "$filter",
buildFilterExpression(queryEntitiesOptions.getFilter()));
}

if (query.getOrderByFields() != null) {
if (queryEntitiesOptions.getOrderByFields() != null) {
webResource = addOptionalQueryParam(webResource, "$orderby",
CommaStringBuilder.join(encodeODataURIValues(query.getOrderByFields())));
CommaStringBuilder.join(encodeODataURIValues(queryEntitiesOptions.getOrderByFields())));
}

return webResource;
Expand All @@ -217,8 +219,8 @@ private void buildFilterExpression(Filter filter, StringBuilder sb) {
if (filter == null)
return;

if (filter instanceof LitteralFilter) {
sb.append(((LitteralFilter) filter).getLitteral());
if (filter instanceof PropertyNameFilter) {
sb.append(((PropertyNameFilter) filter).getPropertyName());
}
else if (filter instanceof ConstantFilter) {
Object value = ((ConstantFilter) filter).getValue();
Expand Down Expand Up @@ -282,8 +284,8 @@ else if (filter instanceof BinaryFilter) {
buildFilterExpression(((BinaryFilter) filter).getRight(), sb);
sb.append(")");
}
else if (filter instanceof RawStringFilter) {
sb.append(((RawStringFilter) filter).getRawString());
else if (filter instanceof QueryStringFilter) {
sb.append(((QueryStringFilter) filter).getQueryString());
}
}

Expand Down Expand Up @@ -380,19 +382,25 @@ public QueryTablesResult queryTables() throws ServiceException {

@Override
public QueryTablesResult queryTables(QueryTablesOptions options) throws ServiceException {
Query query = new Query();
Filter queryFilter = options.getFilter();
Copy link
Contributor

Choose a reason for hiding this comment

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

By only supporting Filter and removing Query you removed the support for $top query option. I know that $select is not working on tables but $top does work. Also there could be extra query options to be added in the future which will be used for tables so why make the code here very dependent on the case?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. Top should be added.

Copy link
Author

Choose a reason for hiding this comment

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

issure 43 is filed to track this feature request.

String nextTableName = options.getNextTableName();
String prefix = options.getPrefix();

if (prefix != null) {
// Append Max char to end '{' is 1 + 'z' in AsciiTable ==> upperBound is prefix + '{'
Filter prefixFilter = Filter.and(Filter.ge(Filter.litteral("TableName"), Filter.constant(prefix)),
Filter.le(Filter.litteral("TableName"), Filter.constant(prefix + "{")));
query.setFilter(prefixFilter);
Filter prefixFilter = Filter.and(Filter.ge(Filter.propertyName("TableName"), Filter.constant(prefix)),
Filter.le(Filter.propertyName("TableName"), Filter.constant(prefix + "{")));

if (queryFilter == null) {
queryFilter = prefixFilter;
}
else {
queryFilter = Filter.and(queryFilter, prefixFilter);
}
}

WebResource webResource = getResource(options).path("Tables");
webResource = addOptionalQuery(webResource, query);
webResource = addOptionalQueryParam(webResource, "$filter", buildFilterExpression(queryFilter));
Copy link
Contributor

Choose a reason for hiding this comment

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

The queryFilter should only be added if it is not null. See the corresponding line in addOptionalQueryEntitiesOptions

webResource = addOptionalQueryParam(webResource, "NextTableName", nextTableName);

WebResource.Builder builder = webResource.getRequestBuilder();
Expand Down Expand Up @@ -609,7 +617,7 @@ public QueryEntitiesResult queryEntities(String table, QueryEntitiesOptions opti
options = new QueryEntitiesOptions();

WebResource webResource = getResource(options).path(table);
webResource = addOptionalQuery(webResource, options.getQuery());
webResource = addOptionalQueryEntitiesOptions(webResource, options);
webResource = addOptionalQueryParam(webResource, "NextPartitionKey",
encodeODataURIValue(options.getNextPartitionKey()));
webResource = addOptionalQueryParam(webResource, "NextRowKey", encodeODataURIValue(options.getNextRowKey()));
Expand Down
26 changes: 9 additions & 17 deletions ...zure-api/src/main/java/com/microsoft/windowsazure/services/table/models/BinaryFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,34 +15,26 @@
package com.microsoft.windowsazure.services.table.models;

public class BinaryFilter extends Filter {
private String operator;
private Filter left;
private Filter right;
private final String operator;
private final Filter left;
private final Filter right;

public String getOperator() {
return operator;
public BinaryFilter(Filter left, String operator, Filter right) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why you've changed the way to instantiate Model objects? The SDKs Model use the form of having default constructor and you set the values via properties.

Copy link
Author

Choose a reason for hiding this comment

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

We believe that all the filters should be immutable, as a result, we are removing the setters.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha!

this.left = left;
this.operator = operator;
this.right = right;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why you've removed the setters for the class?

Copy link
Contributor

Choose a reason for hiding this comment

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

To make them immutable.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jcookems can you add new issue for tables to align with that change?

Copy link
Contributor

Choose a reason for hiding this comment

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

Filed issues 160-162 in the other fork.


public BinaryFilter setOperator(String operator) {
this.operator = operator;
return this;
public String getOperator() {
return operator;
}

public Filter getLeft() {
return left;
}

public BinaryFilter setLeft(Filter left) {
this.left = left;
return this;
}

public Filter getRight() {
return right;
}

public BinaryFilter setRight(Filter right) {
this.right = right;
return this;
}
}
11 changes: 5 additions & 6 deletions ...re-api/src/main/java/com/microsoft/windowsazure/services/table/models/ConstantFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@
package com.microsoft.windowsazure.services.table.models;

public class ConstantFilter extends Filter {
private Object value;
private final Object value;

public Object getValue() {
return value;
public ConstantFilter(Object value) {
this.value = value;
}

public ConstantFilter setValue(Object value) {
this.value = value;
return this;
public Object getValue() {
return value;
}
}
28 changes: 14 additions & 14 deletions ...soft-azure-api/src/main/java/com/microsoft/windowsazure/services/table/models/Filter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,50 +16,50 @@

public class Filter {
public static UnaryFilter not(Filter operand) {
return new UnaryFilter().setOperator("not").setOperand(operand);
return new UnaryFilter("not", operand);
}

public static BinaryFilter and(Filter left, Filter right) {
return new BinaryFilter().setOperator("and").setLeft(left).setRight(right);
return new BinaryFilter(left, "and", right);
}

public static BinaryFilter or(Filter left, Filter right) {
return new BinaryFilter().setOperator("or").setLeft(left).setRight(right);
return new BinaryFilter(left, "or", right);
}

public static BinaryFilter eq(Filter left, Filter right) {
return new BinaryFilter().setOperator("eq").setLeft(left).setRight(right);
return new BinaryFilter(left, "eq", right);
}

public static BinaryFilter ne(Filter left, Filter right) {
return new BinaryFilter().setOperator("ne").setLeft(left).setRight(right);
return new BinaryFilter(left, "ne", right);
}

public static BinaryFilter ge(Filter left, Filter right) {
return new BinaryFilter().setOperator("ge").setLeft(left).setRight(right);
return new BinaryFilter(left, "ge", right);
}

public static BinaryFilter gt(Filter left, Filter right) {
return new BinaryFilter().setOperator("gt").setLeft(left).setRight(right);
return new BinaryFilter(left, "gt", right);
}

public static BinaryFilter lt(Filter left, Filter right) {
return new BinaryFilter().setOperator("lt").setLeft(left).setRight(right);
return new BinaryFilter(left, "lt", right);
}

public static BinaryFilter le(Filter left, Filter right) {
return new BinaryFilter().setOperator("le").setLeft(left).setRight(right);
return new BinaryFilter(left, "le", right);
}

public static ConstantFilter constant(Object value) {
return new ConstantFilter().setValue(value);
return new ConstantFilter(value);
}

public static LitteralFilter litteral(String value) {
return new LitteralFilter().setLitteral(value);
public static PropertyNameFilter propertyName(String value) {
return new PropertyNameFilter(value);
}

public static RawStringFilter rawString(String value) {
return new RawStringFilter().setRawString(value);
public static QueryStringFilter QueryString(String value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It is very important to adheir to the Java nameing guidelines, and make this a lower-case "queryString".

Copy link
Author

Choose a reason for hiding this comment

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

updated.

return new QueryStringFilter(value);
}
}
14 changes: 7 additions & 7 deletions ...ervices/table/models/RawStringFilter.java → ...ices/table/models/PropertyNameFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
*/
package com.microsoft.windowsazure.services.table.models;

public class RawStringFilter extends Filter {
private String rawString;
public class PropertyNameFilter extends Filter {
private final String propertyName;

public String getRawString() {
return rawString;
public PropertyNameFilter(String propertyName) {
this.propertyName = propertyName;
}

public RawStringFilter setRawString(String rawString) {
this.rawString = rawString;
return this;
public String getPropertyName() {
return propertyName;
}

}
81 changes: 0 additions & 81 deletions ...osoft-azure-api/src/main/java/com/microsoft/windowsazure/services/table/models/Query.java
View file
Open in desktop

This file was deleted.

Loading