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

Add configuration property defaultPageSize #2235

Merged
merged 3 commits into from
Apr 12, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions docs/src/pages/guides/FHIRServerUsersGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -1953,6 +1953,7 @@ This section contains reference information about each of the configuration prop
|`fhirServer/core/capabilityStatementCacheTimeout`|integer|The number of minutes that a tenant's CapabilityStatement is cached for the metadata endpoint. |
|`fhirServer/core/extendedCodeableConceptValidation`|boolean|A boolean flag which indicates whether extended validation is performed by the server during object construction for code, Coding, CodeableConcept, Quantity, Uri, and String elements which have required bindings to value sets.|
|`fhirServer/core/disabledOperations`|string|A comma-separated list of operations which are not allowed to run on the IBM FHIR Server, for example, `validate,import`. Note, do not include the dollar sign `$`|
|`fhirServer/core/defaultPageSize`|integer|Sets the pageSize to use in search when no _count parameter is specified in the request. If a user-specified value exceeds the max page size (1000), then a warning is logged and max page size will be used. If not provided, the default page size (10) is used.|
Copy link
Member

Choose a reason for hiding this comment

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

I assume it applies to history interactions as well

Suggested change
|`fhirServer/core/defaultPageSize`|integer|Sets the pageSize to use in search when no _count parameter is specified in the request. If a user-specified value exceeds the max page size (1000), then a warning is logged and max page size will be used. If not provided, the default page size (10) is used.|
|`fhirServer/core/defaultPageSize`|integer|Sets the pageSize to use in search and history interactions when no _count parameter is specified in the request. If a user-specified value exceeds the max page size (1000), then a warning is logged and max page size will be used. If not provided, the default page size (10) is used.|

Copy link
Member

Choose a reason for hiding this comment

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

Also, its a bit confusing, but we have 3 separate tables in section 5.1. You added this property to the table that has the description. However, 5.1.2 has the default value and 5.1.3 indicates whether the parameter is tenant-specific (different values allowed for different tenants) and dynamic (changes take affect without a server restart).
We used to have all of this in a single table, but it was too wide for just about any documentation format.
For 5.1.2, just add a row and indicate the default is 10.
For 5.1.3, since you're using FHIRConfigHelper, this property will be dynamic and tenant-specific, so thats just Y|Y.

|`fhirServer/term/graphTermServiceProvider/enabled`|boolean|Indicates whether the graph term service provider should be used by the FHIR term service to access code system content|
|`fhirServer/term/graphTermServiceProvider/timeLimit`|integer|Graph traversal time limit (in milliseconds)|
|`fhirServer/term/graphTermServiceProvider/configuration`|object (name/value pairs)|A JSON object that contains the name/value pairs used to configure the graph database behind the graph term service provider see: [https://docs.janusgraph.org/basics/configuration-reference/](https://docs.janusgraph.org/basics/configuration-reference/)|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public class FHIRConfiguration {
public static final String PROPERTY_CAPABILITY_STATEMENT_CACHE = "fhirServer/core/capabilityStatementCacheTimeout";
public static final String PROPERTY_EXTENDED_CODEABLE_CONCEPT_VALIDATION = "fhirServer/core/extendedCodeableConceptValidation";
public static final String PROPERTY_DISABLED_OPERATIONS = "fhirServer/core/disabledOperations";
public static final String PROPERTY_DEFAULT_PAGE_SIZE = "fhirServer/core/defaultPageSize";

// Terminology service properties
public static final String PROPERTY_GRAPH_TERM_SERVICE_PROVIDER_ENABLED = "fhirServer/term/graphTermServiceProvider/enabled";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,23 @@

package com.ibm.fhir.persistence.context;

import java.util.logging.Level;
import java.util.logging.Logger;

import com.ibm.fhir.config.FHIRConfigHelper;
import com.ibm.fhir.config.FHIRConfiguration;
import com.ibm.fhir.core.FHIRConstants;
import com.ibm.fhir.persistence.context.impl.FHIRHistoryContextImpl;
import com.ibm.fhir.persistence.context.impl.FHIRPersistenceContextImpl;
import com.ibm.fhir.persistence.interceptor.FHIRPersistenceEvent;
import com.ibm.fhir.search.SearchConstants;
import com.ibm.fhir.search.context.FHIRSearchContext;

/**
* This is a factory used to create instances of the FHIRPersistenceContext interface.
*/
public class FHIRPersistenceContextFactory {
private static Logger log = Logger.getLogger(FHIRPersistenceContextFactory.class.getName());

private FHIRPersistenceContextFactory() {
// No Operation
Expand Down Expand Up @@ -59,7 +67,18 @@ public static FHIRPersistenceContext createPersistenceContext(FHIRPersistenceEve
* Returns a FHIRHistoryContext instance with default values.
*/
public static FHIRHistoryContext createHistoryContext() {
return new FHIRHistoryContextImpl();
int pageSize = FHIRConfigHelper.getIntProperty(FHIRConfiguration.PROPERTY_DEFAULT_PAGE_SIZE, FHIRConstants.FHIR_PAGE_SIZE_DEFAULT);
if (pageSize > SearchConstants.MAX_PAGE_SIZE) {
if (log.isLoggable(Level.WARNING)) {
log.warning(String.format("Server configuration %s = %d exceeds maximum allowed page size %d; using %d",
FHIRConfiguration.PROPERTY_DEFAULT_PAGE_SIZE, pageSize, SearchConstants.MAX_PAGE_SIZE, SearchConstants.MAX_PAGE_SIZE));
}
pageSize = SearchConstants.MAX_PAGE_SIZE;
}

FHIRHistoryContext ctx = new FHIRHistoryContextImpl();
ctx.setPageSize(pageSize);
return ctx;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* (C) Copyright IBM Corp. 2021, 2021
*
* SPDX-License-Identifier: Apache-2.0
*/

package com.ibm.fhir.persistence;

import static org.testng.Assert.assertEquals;

import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import com.ibm.fhir.config.FHIRConfiguration;
import com.ibm.fhir.config.FHIRRequestContext;
import com.ibm.fhir.core.FHIRConstants;
import com.ibm.fhir.exception.FHIRException;
import com.ibm.fhir.persistence.context.FHIRHistoryContext;
import com.ibm.fhir.persistence.context.FHIRPersistenceContextFactory;
import com.ibm.fhir.search.SearchConstants;

public class FHIRPersistenceFactoryTest {
@BeforeClass
public void setUpBeforeClass() {
FHIRConfiguration.setConfigHome("target/test-classes");
}

@BeforeMethod
@AfterMethod
public void clearThreadLocal() {
FHIRRequestContext.remove();
}

@Test
public void testCreateWithDefaultPageSize() throws Exception {
runCreateTest("default", FHIRConstants.FHIR_PAGE_SIZE_DEFAULT);
}

@Test
public void testCreateWithUserConfiguredPageSize() throws Exception {
runCreateTest("pagesize-valid", 500);
}

@Test
public void testCreateWithUserConfiguredPageSizeBeyondMaxium() throws Exception {
runCreateTest("pagesize-invalid", SearchConstants.MAX_PAGE_SIZE);
}

private void runCreateTest(String tenantId, int expectedPageSize) throws FHIRException {
FHIRRequestContext.set(new FHIRRequestContext(tenantId));
FHIRHistoryContext ctx = FHIRPersistenceContextFactory.createHistoryContext();
assertEquals (ctx.getPageSize(), expectedPageSize);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"fhirServer": {
"core": {
"defaultPageSize": 5000
},
"persistence": {
"factoryClassname": "com.ibm.fhir.persistence.test.MockPersistenceFactory"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"fhirServer": {
"core": {
"defaultPageSize": 500
},
"persistence": {
"factoryClassname": "com.ibm.fhir.persistence.test.MockPersistenceFactory"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,20 @@

package com.ibm.fhir.search.context;

import java.util.logging.Level;
lmsurpre marked this conversation as resolved.
Show resolved Hide resolved
import java.util.logging.Logger;

import com.ibm.fhir.config.FHIRConfigHelper;
import com.ibm.fhir.config.FHIRConfiguration;
import com.ibm.fhir.core.FHIRConstants;
import com.ibm.fhir.search.SearchConstants;
import com.ibm.fhir.search.context.impl.FHIRSearchContextImpl;

/**
* This factory class can be used to create instances of the FHIRSearchContext interface.
*/
public class FHIRSearchContextFactory {
private static Logger log = Logger.getLogger(FHIRSearchContextFactory.class.getName());

/**
* Hide the default ctor.
Expand All @@ -23,6 +31,17 @@ private FHIRSearchContextFactory() {
* Returns a new instance of the FHIRSearchContext interface.
*/
public static FHIRSearchContext createSearchContext() {
return new FHIRSearchContextImpl();
int pageSize = FHIRConfigHelper.getIntProperty(FHIRConfiguration.PROPERTY_DEFAULT_PAGE_SIZE, FHIRConstants.FHIR_PAGE_SIZE_DEFAULT);
if (pageSize > SearchConstants.MAX_PAGE_SIZE) {
lmsurpre marked this conversation as resolved.
Show resolved Hide resolved
if (log.isLoggable(Level.WARNING)) {
log.warning(String.format("Server configuration %s = %d exceeds maximum allowed page size %d; using %d",
FHIRConfiguration.PROPERTY_DEFAULT_PAGE_SIZE, pageSize, SearchConstants.MAX_PAGE_SIZE, SearchConstants.MAX_PAGE_SIZE));
}
pageSize = SearchConstants.MAX_PAGE_SIZE;
}

FHIRSearchContext ctx = new FHIRSearchContextImpl();
ctx.setPageSize(pageSize);
return ctx;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* (C) Copyright IBM Corp. 2021, 2021
lmsurpre marked this conversation as resolved.
Show resolved Hide resolved
*
* SPDX-License-Identifier: Apache-2.0
*/

package com.ibm.fhir.search.context;

import static org.testng.Assert.assertEquals;

import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import com.ibm.fhir.config.FHIRConfiguration;
import com.ibm.fhir.config.FHIRRequestContext;
import com.ibm.fhir.core.FHIRConstants;
import com.ibm.fhir.exception.FHIRException;
import com.ibm.fhir.search.SearchConstants;

public class FHIRSearchContextFactoryTest {
@BeforeClass
public void setUpBeforeClass() {
FHIRConfiguration.setConfigHome("target/test-classes");
}

@BeforeMethod
@AfterMethod
public void clearThreadLocal() {
FHIRRequestContext.remove();
}

@Test
public void testCreateWithDefaultPageSize() throws Exception {
runCreateTest("default", FHIRConstants.FHIR_PAGE_SIZE_DEFAULT);
}

@Test
public void testCreateWithUserConfiguredPageSize() throws Exception {
runCreateTest("tenant1", 500);
}

@Test
public void testCreateWithUserConfiguredPageSizeBeyondMaxium() throws Exception {
runCreateTest("tenant2", SearchConstants.MAX_PAGE_SIZE);
}

private void runCreateTest(String tenantId, int expectedPageSize) throws FHIRException {
FHIRRequestContext.set(new FHIRRequestContext(tenantId));
FHIRSearchContext ctx = FHIRSearchContextFactory.createSearchContext();
assertEquals (ctx.getPageSize(), expectedPageSize);
}
}
Original file line number Diff line number Diff line change
@@ -1,28 +1,31 @@
{
"__comment": "FHIR Server configuration",
"fhirServer": {
"resources": {
"open": true,
"Device": {
"searchParameters": {
"patient": "http://hl7.org/fhir/SearchParameter/Device-patient",
"organization": "http://hl7.org/fhir/SearchParameter/Device-organization"
}
},
"Observation": {
"searchParameters": {
"code": "http://hl7.org/fhir/SearchParameter/clinical-code",
"value-range": "http://hl7.org/fhir/SearchParameter/Observation-value-range"
}
},
"Patient": {
"searchParameters": {
"active": "http://hl7.org/fhir/SearchParameter/Patient-active",
"address": "http://hl7.org/fhir/SearchParameter/individual-address",
"birthdate": "http://hl7.org/fhir/SearchParameter/individual-birthdate",
"name": "http://hl7.org/fhir/SearchParameter/Patient-name"
}
}
}
}
}
{
"__comment": "FHIR Server configuration",
"fhirServer": {
"resources": {
"open": true,
"Device": {
"searchParameters": {
"patient": "http://hl7.org/fhir/SearchParameter/Device-patient",
"organization": "http://hl7.org/fhir/SearchParameter/Device-organization"
}
},
"Observation": {
"searchParameters": {
"code": "http://hl7.org/fhir/SearchParameter/clinical-code",
"value-range": "http://hl7.org/fhir/SearchParameter/Observation-value-range"
}
},
"Patient": {
"searchParameters": {
"active": "http://hl7.org/fhir/SearchParameter/Patient-active",
"address": "http://hl7.org/fhir/SearchParameter/individual-address",
"birthdate": "http://hl7.org/fhir/SearchParameter/individual-birthdate",
"name": "http://hl7.org/fhir/SearchParameter/Patient-name"
}
}
},
"core": {
"defaultPageSize": 500
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
{
"__comment": "FHIR Server configuration",
"fhirServer": {
"search": {
"useStoredCompartmentParam": false
}
}
}
{
"__comment": "FHIR Server configuration",
"fhirServer": {
"search": {
"useStoredCompartmentParam": false
},
"core": {
"defaultPageSize": 5000
}
}
}