Skip to content

Improve Date Time support and _lastUpdated behavior #255 #258 #437#506

Merged
lmsurpre merged 20 commits intomasterfrom
issue-255-lastupdated
Dec 20, 2019
Merged

Improve Date Time support and _lastUpdated behavior #255 #258 #437#506
lmsurpre merged 20 commits intomasterfrom
issue-255-lastupdated

Conversation

@prb112
Copy link
Copy Markdown
Contributor

@prb112 prb112 commented Dec 15, 2019

  • Update the ParameterValue to use Instants versus DateTime
    • Add Lower and Upper Bound value and the original value
  • Update the Parameter toString to reflect the use of Instant
    and serialize it properly
  • Introduce DateTimeHandler to properly parse and test the parsing of
    dates/times, and add DateTimeHandlerTest
    • Supports yyyy-mm-ddThh:mm:ss[Z|(+|-)hh:mm] defined in spec
    • Supports microseconds
    • Supports TimeZones in Textual Format and +/- HH:MM
    • Also ZonedDateTime / LocalDateTime have granular precision for lower
      and upper bounds
  • Update SearchUtil from DateTime.Builder to DateTimeHandler.Parser to
    use TemporalAccesors
  • Update SearchExceptionUtil to support a DateTime Format Exception
  • Add tests for the various Parsing successes and failures.
  • Introduce DateParmBehaviorUtil to treat date/time ranges based on the
    level of precision in the passed time parameter
    • Add Tests to support the DateParmBehaviorUtil
  • Refactor JDBCQueryBuilder to break out Date Behaviors into the utility
  • Update Precision to 1000 nanos (we're up to 6 places of precision on
    the second)
  • Change DerbyMaster to log values from a static final

Signed-off-by: Paul Bastide pbastide@us.ibm.com

@prb112
Improve Date Time support and _lastUpdated behavior #255 #258 #437
f95956f 
- Update the ParameterValue to use Instants versus DateTime
	- Add Lower and Upper Bound value and the original value
- Update the Parameter toString to reflect the use of Instant
and serialize it properly
- Introduce DateTimeHandler to properly parse and test the parsing of
dates/times, and add DateTimeHandlerTest
	- Supports yyyy-mm-ddThh:mm:ss[Z|(+|-)hh:mm] defined in spec
	- Supports microseconds
	- Supports TimeZones in Textual Format and +/- HH:MM
	- Also ZonedDateTime / LocalDateTime have granular precision for lower
and upper bounds
- Update SearchUtil from DateTime.Builder to DateTimeHandler.Parser to
use TemporalAccesors
- Update SearchExceptionUtil to support a DateTime Format Exception
- Add tests for the various Parsing successes and failures.
- Introduce DateParmBehaviorUtil to treat date/time ranges based on the
level of precision in the passed time parameter
	- Add Tests to support the DateParmBehaviorUtil
- Refactor JDBCQueryBuilder to break out Date Behaviors into the utility
- Update Precision to 1000 nanos (we're up to 6 places of precision on
the second)
- Change DerbyMaster to log values from a static final

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112 prb112 added bug Something isn't working enhancement New feature or request search labels Dec 15, 2019
@prb112 prb112 self-assigned this Dec 15, 2019
@prb112
Improve Date Time support and _lastUpdated behavior #255 #258 #437
ddd5092 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
This was referenced Dec 16, 2019
Closed
Closed
Closed
@prb112
Improve Date Time support and _lastUpdated behavior #255 #258 #437
22840f2 
- refactored _lastUpdated

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Merge branch 'master' into issue-255-lastupdated
6facf2c
@prb112
issue-255-lastupdated
f2dc9cf 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Merge branch 'master' into issue-255-lastupdated
a1ed7af
@prb112
_id and _lastUpdated updates #255
1b67814 
- Merge with master
- Remove unused StringParmVal from ResourceDAO.java
- Update FHIRPersistenceEvent to use non-deprecated API isStandardType
from FHIRUtil

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
lmsurpre
lmsurpre reviewed Dec 17, 2019
View reviewed changes
Comment thread fhir-database-utils/src/main/java/com/ibm/fhir/database/utils/derby/DerbyMaster.java Outdated
@lmsurpre
Copy link
Copy Markdown
Member

lmsurpre commented Dec 17, 2019

Are you still planning to tests in AbstractSearchDateTest? We had listed that as acceptance criteria for #437

@prb112
Copy link
Copy Markdown
Contributor Author

prb112 commented Dec 18, 2019

Are you still planning to tests in AbstractSearchDateTest? We had listed that as acceptance criteria for #437

No, this class is somewhat final.

@prb112 prb112 added this to the Sprint 5 milestone Dec 18, 2019
@prb112
_id and _lastUpdated updates #255
a685735 
- Update the behavior for _lastUpdated treatment
- Change DEBUGGING to DEBUG for DerbyMaster
- Update a href quoting on DateParmBehaviorUtil and
LastUpdatedParmBehaviorUtil
- Warning found in ValueTypesR4Impl, and refactored to support the
special parameter 'near' and the Location.Position
- convert compareTo to equals for review

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
_id and _lastUpdated updates #255
8b5bb31 
- logging
- Query Updates to limit the time it takes to run by using covered
indices.

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112 prb112 changed the title WIP: Improve Date Time support and _lastUpdated behavior #255 #258 #437 Improve Date Time support and _lastUpdated behavior #255 #258 #437 Dec 19, 2019
@prb112
_id and _lastUpdated updates #255
8ad5dda 
- change to use min and max timestamps as defined in the local timezone
and are translatable to UTC

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
_id and _lastUpdated updates #255
8c9a06a 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Merge branch 'master' into issue-255-lastupdated
121c4dc
@prb112
issue 255
81c9334 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Copy link
Copy Markdown
Contributor Author

prb112 commented Dec 19, 2019

The latest commits as of 7:47 bring the code up to the level that is in master, and further enhance the support for lower bound and upperbound timestamps.

@prb112 prb112 requested a review from JohnTimm December 19, 2019 15:59
@prb112
improve handling of sums/counts and _lastUpdated
569b3ce 
- refactor the _lastUpdated
- SUM/COUNT

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
improve handling of sums/counts and _lastUpdated
6aed41a 
- turn off logging in the PR

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Implements _lastUpdated
63b3c09 
- lastUpdated is implemented
- Remove bad formatting (spaces at end of lines)

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
prb112
prb112 commented Dec 19, 2019
View reviewed changes
Comment thread fhir-core/src/main/java/com/ibm/fhir/core/FHIRUtilities.java Outdated
@prb112
Issue 255 (PR Comments/Issues)
fe2f966 
- Remove AbstractJDBCQueryBuilder and JDBCQueryBuilder getPrefix from
Interface and Impl
- Refactor NumberParmBehaviorUtil per JDBCOperator removal
- Refactor DateParmBehaviorUtil per JDBCOperator removal
- Refactor DateParmBehaviorUtilTest to remove UTC/Z formatting issues in
the tests
- Refactor DateTimeHandlerTest to remove UTC/Z formatting issues in
tests
- Refactor DateTimeHandler to remove unused unit and simplify code
- Add test for LastUpdatedParmBehaviorUtilTest
- Refactor LastUpdatedParmBehaviorUtil to remove references to
JDBCOperator
- Refactor SortedQuerySegmentAggregator to simplify Constants and
JDBCOperators
- Refactor UriModifierUtil / UriModifierUtilTest to simplify constants
and JDBCConstants and JDBCOperators
- Refactor JDBCConstants remove JDBCOperators and the prefix map
- Refactor QuantityParmBehaviorUtil and LocationParmBehaviorUtil to
simplify SQL operators.
- Refactor out JDBCOperator from AbstractQueryBuilder / JDBCQueryBuilder
- Refactor InclusionQuerySegmentAggregator / QuerySegmentAggregator
query segments
- Refactor CacheUtilTest to keep from printing useless output (added
assertNotNull)
- Tune down logging to FINE only (and reset minimum level to FINE)
- Update JDBCParameterBuildingVisitor to reflect UTC
- Update AbstractSearchDateTest to reflect UTC changes and more accurate
precision
- Resolve DateTimeHandler with Z test issues

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Issue 255 (PR Comments/Issues)
0ae0482 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Update Issue 255
2ecf0a4 
- Add missing tests in testng.xml
- Remove references to Timezone
- Update DateTimeHandler
- Update logging properties

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112 prb112 mentioned this pull request Dec 20, 2019
Closed
lmsurpre
lmsurpre reviewed Dec 20, 2019
View reviewed changes
Comment thread fhir-core/src/main/java/com/ibm/fhir/core/FHIRUtilities.java
*/
public static Timestamp convertToTimestamp(java.time.ZonedDateTime zdt) {
return new Timestamp(zdt.toInstant().toEpochMilli());
return Timestamp.from(zdt.withZoneSameInstant(ZoneId.of("UTC")).toInstant());
Copy link
Copy Markdown
Member

@lmsurpre lmsurpre Dec 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This one still confuses me. An Instant doesn't have a timezone, so why do we need to convert the ZonedDateTime to UTC before calling toInstant?

Copy link
Copy Markdown
Contributor Author

@prb112 prb112 Dec 20, 2019

Choose a reason for hiding this comment

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

this step ensures it is UTC. when it's converted to an instant

Copy link
Copy Markdown
Contributor Author

@prb112 prb112 Dec 20, 2019

Choose a reason for hiding this comment

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

Instant = 1576849840146
Converted to Timestamp
Fri Dec 20 2019 13:51:08
however that instant is actually the time EST
so we need to adjust UP
Instant = 1576859840146
Fri Dec 20 2019 16:37:20
Now, it's actually in UTC (edited)

@prb112
issue 255
8c5a9ac 
- remove unused utility and corresponding test

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
lmsurpre
lmsurpre approved these changes Dec 20, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@lmsurpre lmsurpre left a comment

Choose a reason for hiding this comment

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

LGTM

@lmsurpre lmsurpre merged commit c53bc36 into master Dec 20, 2019
@lmsurpre lmsurpre deleted the issue-255-lastupdated branch December 20, 2019 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lmsurpre lmsurpre lmsurpre approved these changes

@JohnTimm JohnTimm Awaiting requested review from JohnTimm

Assignees

@prb112 prb112

Labels

bug Something isn't working enhancement New feature or request search

Projects

None yet

Milestone

Sprint 5

Development

Successfully merging this pull request may close these issues.

2 participants

@prb112 @lmsurpre