Skip to content

Commit

Permalink
add IllegalArgumentException throws to URIParameters setStart() and s…
Browse files Browse the repository at this point in the history
…etLimit() methods and code simplifications
  • Loading branch information
alexdunnjpl authored and thomas loubrieu committed Mar 2, 2023
1 parent af4f9a9 commit 07e6c30
Show file tree
Hide file tree
Showing 11 changed files with 105 additions and 137 deletions.
14 changes: 6 additions & 8 deletions model/swagger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ paths:
- products
summary: |
returns all product versions with the identifier (lid) selected from all PDS Products. See identifier for details on how it determines what is returned.
note: if given a lidvid, it will be translated to a lid
operationId: select-by-lidvid-all
responses:
Expand Down Expand Up @@ -1107,7 +1107,7 @@ components:
in: path
description: |
syntax: one of the allowable enum values
This is a shortcut for doing a query with ...
required: true
schema:
Expand Down Expand Up @@ -1149,8 +1149,6 @@ components:
syntax: limit=10
behavior: maximum number of matching results returned, for pagination
note: limit=0 returns just the summary
required: false
schema:
type: integer
Expand Down Expand Up @@ -1197,7 +1195,7 @@ components:
in: path
description: |
syntax: one of the allowable enum values
This is a shortcut for doing a query with ...
required: true
schema:
Expand Down Expand Up @@ -1408,15 +1406,15 @@ components:
xml:
prefix: 'pds_api'
namespace: 'http://pds.nasa.gov/api'
observing_system_components:
observing_system_components:
type: array
description: list of instruments or platforms generating the data
items:
$ref: '#/components/schemas/reference'
xml:
prefix: 'pds_api'
namespace: 'http://pds.nasa.gov/api'
targets:
targets:
type: array
description: list of targets or feature of interest the observation.
items:
Expand Down Expand Up @@ -1475,7 +1473,7 @@ components:
items:
$ref: '#/components/schemas/wyriwygProduct'
propertyArrayValues:
type: array
type: array
items:
type: string
xml:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ public interface LidvidsContext
public String getLidVid();
public Integer getLimit();
public Integer getStart();
public boolean isSummaryOnly();
public boolean getReturnSingularDatum();
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,27 +25,26 @@
* a bunch of set calls. Instead of a single line for each set, then can be
* concatenated via the . to make the code more readable by keeping the all
* of the set calls collocated.
* 3. The default values for limit and start work in conjunction later in the
* business logic to indicate a Singular or Plural return type. See swagger.yml
* to understand Singular and Plural return types.
* 3. By default, the context assumes a Singular (single-element-valued) return type for the API route.
* If the route is a Plural (collection-valued) return type, the transmuter will call either/both of setStart() and
* setLimit(), which will mutate returnSingularDatum to its correct false value.
* See swagger.yml for further detail of the Singular and Plural return types.
*/
class URIParameters implements UserContext
{
private static final int SUMMARY_SAMPLE_SIZE = 100;

private boolean verifyClassAndId = false;
private String accept = "application/json";
private List<String> fields = new ArrayList<String>();
private String group = "";
private String identifier = "";
private List<String> keywords = new ArrayList<String>();
private PdsProductIdentifier productIdentifier = null;
private Integer limit = Integer.valueOf(0);
private boolean summaryOnly = true;
private Integer start = 0;
private Integer limit = 0; // Actual default value is passed in from the upstream frames of the call stack, but it's unclear where it comes from. Not swagger.yml, at least.
private Boolean returnSingularDatum = true;
private String query = "";
private ProductVersionSelector selector = ProductVersionSelector.LATEST;
private List<String> sort = new ArrayList<String>();
private Integer start = Integer.valueOf(-1);
private String version = "latest";

@Override
Expand All @@ -61,6 +60,8 @@ class URIParameters implements UserContext
@Override
public Integer getLimit() { return limit; }
@Override
public boolean getReturnSingularDatum() { return returnSingularDatum; }
@Override
public String getLidVid() { return productIdentifier != null ? productIdentifier.toString() : ""; }
@Override
public String getQuery() { return query; }
Expand All @@ -73,8 +74,6 @@ class URIParameters implements UserContext
public boolean getVerifyClassAndId() { return verifyClassAndId; }
@Override
public String getVersion() { return version; }
@Override
public boolean isSummaryOnly() { return summaryOnly; }

public URIParameters setAccept(String accept)
{
Expand Down Expand Up @@ -103,21 +102,15 @@ public URIParameters setKeywords(List<String> keywords)
}
public URIParameters setLimit(Integer limit)
{
/*
* Note: Not too happy w/ having to put behavioral logic in a utility/container class, but
* there are just way too many places where this information is necessary and rather than
* duplicate it everywhere, this is the best place, for now, as it is the common object
* shared across them all.
*/
if (limit != null) {
if(limit == 0) {
summaryOnly = true;
this.limit = Integer.valueOf(SUMMARY_SAMPLE_SIZE);
} else {
this.limit = limit;
summaryOnly = false;
}
if (limit == null) {return this;}

if (limit < 0) {
String errMsg = String.format("start index must be 0 or higher (got '%d'))", start);
throw new IllegalArgumentException(errMsg);
}

this.limit = limit;
this.returnSingularDatum = false;
return this;
}
public URIParameters setProductIdentifier(ControlContext control) throws IOException, LidVidNotFoundException
Expand All @@ -138,7 +131,15 @@ public URIParameters setSort(List<String> sort)
}
public URIParameters setStart(Integer start)
{
if (start != null) this.start = start;
if (start == null) {return this;}

if (start < 0) {
String errMsg = String.format("start index must be 0 or higher (got '%d'))", start);
throw new IllegalArgumentException(errMsg);
}

this.start = start;
this.returnSingularDatum = false;
return this;
}
public URIParameters setVerifyClassAndId (boolean verify)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,22 +83,17 @@ public void setResponse (SearchHit hit, List<String> fields)
{ this.product = Pds4ProductFactory.createProduct(hit.getId(), hit.getSourceAsMap(), this.isJSON); }

@Override
public int setResponse(HitIterator hits, Summary summary, List<String> fields, boolean onlySummary)
{
List<Pds4Product> list = new ArrayList<Pds4Product>();
Pds4Products products = new Pds4Products();
public int setResponse(HitIterator hits, Summary summary, List<String> fields) {
List<Pds4Product> list = new ArrayList<Pds4Product>();
Pds4Products products = new Pds4Products();
Set<String> uniqueProperties = new TreeSet<String>();

for (Map<String,Object> kvp : hits)
{
uniqueProperties.addAll(ProductBusinessObject.getFilteredProperties(kvp, fields, null).keySet());
for (Map<String, Object> kvp : hits) {
uniqueProperties.addAll(ProductBusinessObject.getFilteredProperties(kvp, fields, null).keySet());

if (!onlySummary)
{
Pds4Product prod = Pds4ProductFactory.createProduct(hits.getCurrentId(), kvp, this.isJSON);
list.add(prod);
}
}
Pds4Product prod = Pds4ProductFactory.createProduct(hits.getCurrentId(), kvp, this.isJSON);
list.add(prod);
}

products.setData(list);
products.setSummary(summary);
Expand All @@ -108,25 +103,22 @@ public int setResponse(HitIterator hits, Summary summary, List<String> fields, b
}

@Override
public int setResponse(SearchHits hits, Summary summary, List<String> fields, boolean onlySummary)
public int setResponse(SearchHits hits, Summary summary, List<String> fields)
{
List<Pds4Product> list = new ArrayList<Pds4Product>();
Pds4Products products = new Pds4Products();
Set<String> uniqueProperties = new TreeSet<String>();

// Products
for(SearchHit hit : hits)
for(SearchHit hit : hits)
{
String id = hit.getId();
Map<String, Object> fieldMap = hit.getSourceAsMap();

uniqueProperties.addAll(ProductBusinessObject.getFilteredProperties(fieldMap, fields, null).keySet());

if (!onlySummary)
{
Pds4Product prod = Pds4ProductFactory.createProduct(id, fieldMap, this.isJSON);
list.add(prod);
}
Pds4Product prod = Pds4ProductFactory.createProduct(id, fieldMap, this.isJSON);
list.add(prod);
}
products.setData(list);
products.setSummary(summary);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class PdsProductBusinessObject implements ProductBusinessLogic
private PdsProduct product = null;
private PdsProducts products = null;
private URL baseURL;

@Override
public String[] getMaximallyRequiredFields()
{ return new String[0]; }
Expand Down Expand Up @@ -57,7 +57,7 @@ public void setResponse (SearchHit hit, List<String> fields)

@Override
@SuppressWarnings("unchecked")
public int setResponse(HitIterator hits, Summary summary, List<String> fields, boolean onlySummary)
public int setResponse(HitIterator hits, Summary summary, List<String> fields)
{
int count;
PdsProducts products = new PdsProducts();
Expand All @@ -67,16 +67,11 @@ public int setResponse(HitIterator hits, Summary summary, List<String> fields, b
{
uniqueProperties.addAll(ProductBusinessObject.getFilteredProperties(kvp, fields, null).keySet());

if (!onlySummary)
{
products.addDataItem(SearchUtil.entityProductToAPIProduct(objectMapper.convertValue(kvp, EntityProduct.class), this.baseURL));
products.getData().get(products.getData().size()-1).setProperties((Map<String, PropertyArrayValues>)(Map<String, ?>)ProductBusinessObject.getFilteredProperties(kvp, null, null));
}
products.addDataItem(SearchUtil.entityProductToAPIProduct(objectMapper.convertValue(kvp, EntityProduct.class), this.baseURL));
products.getData().get(products.getData().size()-1).setProperties((Map<String, PropertyArrayValues>)(Map<String, ?>)ProductBusinessObject.getFilteredProperties(kvp, null, null));
}
count = products.getData().size();

if (onlySummary) products.setData(null);

summary.setProperties(new ArrayList<String>(uniqueProperties));
products.setSummary(summary);
this.products = products;
Expand All @@ -85,7 +80,7 @@ public int setResponse(HitIterator hits, Summary summary, List<String> fields, b

@Override
@SuppressWarnings("unchecked")
public int setResponse(SearchHits hits, Summary summary, List<String> fields, boolean onlySummary)
public int setResponse(SearchHits hits, Summary summary, List<String> fields)
{
Map<String,Object> kvp;
PdsProducts products = new PdsProducts();
Expand All @@ -96,14 +91,9 @@ public int setResponse(SearchHits hits, Summary summary, List<String> fields, bo
kvp = hit.getSourceAsMap();
uniqueProperties.addAll(ProductBusinessObject.getFilteredProperties(kvp, fields, null).keySet());

if (!onlySummary)
{
products.addDataItem(SearchUtil.entityProductToAPIProduct(objectMapper.convertValue(kvp, EntityProduct.class), this.baseURL));
products.getData().get(products.getData().size()-1).setProperties((Map<String, PropertyArrayValues>)(Map<String, ?>)ProductBusinessObject.getFilteredProperties(kvp, null, null));
}
}

if (onlySummary) products.setData(null);
products.addDataItem(SearchUtil.entityProductToAPIProduct(objectMapper.convertValue(kvp, EntityProduct.class), this.baseURL));
products.getData().get(products.getData().size()-1).setProperties((Map<String, PropertyArrayValues>)(Map<String, ?>)ProductBusinessObject.getFilteredProperties(kvp, null, null));
}

summary.setProperties(new ArrayList<String>(uniqueProperties));
products.setSummary(summary);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ public interface ProductBusinessLogic
public void setBaseURL (URL baseURL);
public void setObjectMapper (ObjectMapper om);
public void setResponse (SearchHit hit, List<String> fields);
public int setResponse (HitIterator hits, Summary summary, List<String> fields, boolean onlySummary);
public int setResponse (SearchHits hits, Summary summary, List<String> fields, boolean onlySummary);
public int setResponse (HitIterator hits, Summary summary, List<String> fields);
public int setResponse (SearchHits hits, Summary summary, List<String> fields);
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ public class RequestAndResponseContext implements RequestBuildContext, RequestCo
final private List<String> sort;
final private int start;
final private int limit;
final private boolean summaryOnly;

final private boolean returnSingularDatum;
final private GroupConstraint presetCriteria;
final private ProductVersionSelector selector;
final private String format;
Expand Down Expand Up @@ -129,10 +130,10 @@ private RequestAndResponseContext(ControlContext controlContext, // webby criter
this.productIdentifier = LidVidUtils.resolve(parameters.getIdentifier(), versionSelectionScope,
controlContext, RequestBuildContextFactory
.given(parameters.getSelector() == ProductVersionSelector.LATEST, fields, resPreset));
this.summaryOnly = parameters.isSummaryOnly();
this.start = parameters.getStart();
this.limit = parameters.getLimit();
this.returnSingularDatum = parameters.getReturnSingularDatum();
this.sort = parameters.getSort();
this.start = parameters.getStart();
this.presetCriteria = outPreset;
this.selector = parameters.getSelector();
}
Expand Down Expand Up @@ -185,12 +186,10 @@ public ProductVersionSelector getSelector() {
return this.selector;
}

public boolean isSingular() {
return this.getStart() == -1 && this.isSummaryOnly();
} // isSummaryOnly implies original limit value == 0

public boolean isSummaryOnly() {
return summaryOnly;

public boolean isSingular() {
return this.returnSingularDatum;
}

@Override
Expand Down Expand Up @@ -291,22 +290,19 @@ public Object getResponse() throws NothingFoundException {
for (String sort : this.getSort())
log.warn(" " + sort);
log.warn(" start: " + String.valueOf(this.getStart()));
log.warn(" summary: " + String.valueOf(this.isSummaryOnly()));
throw new NothingFoundException();
}
return response;

}

public void setResponse(HitIterator hits, int real_total) {
Summary summary = new Summary();
summary.setQ(this.getQueryString());
summary.setStart(this.getStart());
summary.setLimit(this.getLimit());
if (this.isSummaryOnly())
summary.setLimit(0);
summary.setSort(this.getSort());
summary.setHits(this.formatters.get(this.format).setResponse(hits, summary, this.fields,
this.isSummaryOnly()));
summary.setHits(this.formatters.get(this.format).setResponse(hits, summary, this.fields));
summary.setProperties(new ArrayList<String>());

if (0 < real_total)
Expand All @@ -330,20 +326,18 @@ public void setResponse(SearchHits hits, List<String> uniqueProperties, int tota
summary.setQ(this.getQueryString());
summary.setStart(this.getStart());
summary.setLimit(this.getLimit());
if (this.isSummaryOnly())
summary.setLimit(0);
summary.setSort(this.getSort());
summary.setHits(total_hits);

if (uniqueProperties != null)
summary.setProperties(uniqueProperties);
this.formatters.get(this.format).setResponse(hits, summary, this.fields,
this.isSummaryOnly());
this.formatters.get(this.format).setResponse(hits, summary, this.fields);

summary.setTook((int) (System.currentTimeMillis() - this.begin_processing));
}
}


public void setResponse(RestHighLevelClient client, SearchRequest request) throws IOException {
if (this.isSingular()) {
SearchHits hits;
Expand Down
Loading

0 comments on commit 07e6c30

Please sign in to comment.