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

[dynamodb] Fix CallItem IT, ZonedDateTime into wide use #7598

Merged
merged 1 commit into from
May 13, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@

import java.math.BigDecimal;
import java.text.DateFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Calendar;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeParseException;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
import java.util.TimeZone;

import org.openhab.core.items.Item;
import org.openhab.core.library.items.CallItem;
Expand Down Expand Up @@ -58,11 +58,8 @@
*/
public abstract class AbstractDynamoDBItem<T> implements DynamoDBItem<T> {

public static final SimpleDateFormat DATEFORMATTER = new SimpleDateFormat(DATE_FORMAT);

static {
DATEFORMATTER.setTimeZone(TimeZone.getTimeZone("UTC"));
}
public static final DateTimeFormatter DATEFORMATTER = DateTimeFormatter.ofPattern(DATE_FORMAT)
.withZone(ZoneId.of("UTC"));

private static final String UNDEFINED_PLACEHOLDER = "<org.openhab.core.types.UnDefType.UNDEF>";

Expand Down Expand Up @@ -117,15 +114,10 @@ public static DynamoDBItem<?> fromState(String name, State state, Date time) {
return new DynamoDBBigDecimalItem(name,
((UpDownType) state) == UpDownType.UP ? BigDecimal.ONE : BigDecimal.ZERO, time);
} else if (state instanceof DateTimeType) {
synchronized (DATEFORMATTER) {
return new DynamoDBStringItem(name,
DATEFORMATTER.format(((DateTimeType) state).getCalendar().getTime()), time);
}
return new DynamoDBStringItem(name, ((DateTimeType) state).getZonedDateTime().format(DATEFORMATTER), time);
} else if (state instanceof UnDefType) {
return new DynamoDBStringItem(name, UNDEFINED_PLACEHOLDER, time);
} else if (state instanceof StringListType) {
// StringListType.format method instead of toString since that matches the format expected by the String
// constructor
return new DynamoDBStringItem(name, state.toFullString(), time);
} else {
// HSBType, PointType and StringType
Expand All @@ -145,17 +137,17 @@ public void visit(DynamoDBStringItem dynamoStringItem) {
} else if (item instanceof LocationItem) {
state[0] = new PointType(dynamoStringItem.getState());
} else if (item instanceof DateTimeItem) {
Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
try {
synchronized (DATEFORMATTER) {
cal.setTime(DATEFORMATTER.parse(dynamoStringItem.getState()));
}
} catch (ParseException e) {
// Parse ZoneDateTime from string. DATEFORMATTER assumes UTC in case it is not clear
// from the string (should be).
// We convert to default/local timezone for user convenience (e.g. display)
state[0] = new DateTimeType(ZonedDateTime.parse(dynamoStringItem.getState(), DATEFORMATTER)
.withZoneSameInstant(ZoneId.systemDefault()));
} catch (DateTimeParseException e) {
logger.warn("Failed to parse {} as date. Outputting UNDEF instead",
dynamoStringItem.getState());
state[0] = UnDefType.UNDEF;
}
state[0] = new DateTimeType(cal);
} else if (dynamoStringItem.getState().equals(UNDEFINED_PLACEHOLDER)) {
state[0] = UnDefType.UNDEF;
} else if (item instanceof CallItem) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -436,9 +436,8 @@ private void flushBatch(DynamoDBMapper mapper, Deque<DynamoDBItem<?>> batch) {
List<FailedBatch> failed = mapper.batchSave(batch);
for (FailedBatch failedBatch : failed) {
if (failedBatch.getException() instanceof ResourceNotFoundException) {
// Table did not exist. Try writing everything again.
// Table did not exist. Try again after creating table
retryFlushAfterCreatingTable(mapper, batch, failedBatch);
break;
} else {
logger.debug("Batch failed with {}. Retrying next with exponential back-off",
failedBatch.getException().getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,16 +106,16 @@ public void visit(DynamoDBBigDecimalItem dynamoBigDecimalItem) {
final Condition timeCondition;
if (!hasBegin && !hasEnd) {
timeCondition = null;
} else if (!hasBegin && hasEnd) {
timeCondition = new Condition().withComparisonOperator(ComparisonOperator.LE).withAttributeValueList(
new AttributeValue().withS(AbstractDynamoDBItem.DATEFORMATTER.format(filter.getEndDate())));
} else if (hasBegin && !hasEnd) {
timeCondition = new Condition().withComparisonOperator(ComparisonOperator.GE).withAttributeValueList(
new AttributeValue().withS(AbstractDynamoDBItem.DATEFORMATTER.format(filter.getBeginDate())));
new AttributeValue().withS(filter.getBeginDateZoned().format(AbstractDynamoDBItem.DATEFORMATTER)));
} else if (!hasBegin && hasEnd) {
timeCondition = new Condition().withComparisonOperator(ComparisonOperator.LE).withAttributeValueList(
new AttributeValue().withS(filter.getEndDateZoned().format(AbstractDynamoDBItem.DATEFORMATTER)));
} else {
timeCondition = new Condition().withComparisonOperator(ComparisonOperator.BETWEEN).withAttributeValueList(
new AttributeValue().withS(AbstractDynamoDBItem.DATEFORMATTER.format(filter.getBeginDate())),
new AttributeValue().withS(AbstractDynamoDBItem.DATEFORMATTER.format(filter.getEndDate())));
new AttributeValue().withS(filter.getBeginDateZoned().format(AbstractDynamoDBItem.DATEFORMATTER)),
new AttributeValue().withS(filter.getEndDateZoned().format(AbstractDynamoDBItem.DATEFORMATTER)));
}
return timeCondition;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import static org.junit.Assert.*;

import java.time.ZonedDateTime;
import java.util.Date;
import java.util.Iterator;

Expand Down Expand Up @@ -53,9 +54,9 @@
@NonNullByDefault
public abstract class AbstractTwoItemIntegrationTest extends BaseIntegrationTest {

protected static @Nullable Date beforeStore;
protected static @Nullable Date afterStore1;
protected static @Nullable Date afterStore2;
protected static @Nullable ZonedDateTime beforeStore;
protected static @Nullable ZonedDateTime afterStore1;
protected static @Nullable ZonedDateTime afterStore2;

protected abstract String getItemName();

Expand Down Expand Up @@ -118,12 +119,12 @@ private void assertIterableContainsItems(Iterable<HistoricItem> iterable, boolea
}

assertStateEquals(getFirstItemState(), storedFirst.getState());
assertTrue(storedFirst.getTimestamp().before(afterStore1));
assertTrue(storedFirst.getTimestamp().after(beforeStore));
assertTrue(storedFirst.getTimestamp().before(Date.from(afterStore1.toInstant())));
assertTrue(storedFirst.getTimestamp().after(Date.from(beforeStore.toInstant())));

assertStateEquals(getSecondItemState(), storedSecond.getState());
assertTrue(storedSecond.getTimestamp().before(afterStore2));
assertTrue(storedSecond.getTimestamp().after(afterStore1));
assertTrue(storedSecond.getTimestamp().before(Date.from(afterStore2.toInstant())));
assertTrue(storedSecond.getTimestamp().after(Date.from(afterStore1.toInstant())));
}

@Test
Expand Down Expand Up @@ -208,8 +209,8 @@ public void testQueryUsingNameAndStartAndEndWithNEQOperator() {
HistoricItem actual1 = iterator.next();
assertFalse(iterator.hasNext());
assertStateEquals(getFirstItemState(), actual1.getState());
assertTrue(actual1.getTimestamp().before(afterStore1));
assertTrue(actual1.getTimestamp().after(beforeStore));
assertTrue(actual1.getTimestamp().before(Date.from(afterStore1.toInstant())));
assertTrue(actual1.getTimestamp().after(Date.from(beforeStore.toInstant())));
}

@Test
Expand All @@ -225,8 +226,8 @@ public void testQueryUsingNameAndStartAndEndWithEQOperator() {
HistoricItem actual1 = iterator.next();
assertFalse(iterator.hasNext());
assertStateEquals(getFirstItemState(), actual1.getState());
assertTrue(actual1.getTimestamp().before(afterStore1));
assertTrue(actual1.getTimestamp().after(beforeStore));
assertTrue(actual1.getTimestamp().before(Date.from(afterStore1.toInstant())));
assertTrue(actual1.getTimestamp().after(Date.from(beforeStore.toInstant())));
}

@Test
Expand All @@ -242,8 +243,8 @@ public void testQueryUsingNameAndStartAndEndWithLTOperator() {
HistoricItem actual1 = iterator.next();
assertFalse(iterator.hasNext());
assertStateEquals(getFirstItemState(), actual1.getState());
assertTrue(actual1.getTimestamp().before(afterStore1));
assertTrue(actual1.getTimestamp().after(beforeStore));
assertTrue(actual1.getTimestamp().before(Date.from(afterStore1.toInstant())));
assertTrue(actual1.getTimestamp().after(Date.from(beforeStore.toInstant())));
}

@Test
Expand Down Expand Up @@ -272,8 +273,8 @@ public void testQueryUsingNameAndStartAndEndWithLTEOperator() {
HistoricItem actual1 = iterator.next();
assertFalse(iterator.hasNext());
assertStateEquals(getFirstItemState(), actual1.getState());
assertTrue(actual1.getTimestamp().before(afterStore1));
assertTrue(actual1.getTimestamp().after(beforeStore));
assertTrue(actual1.getTimestamp().before(Date.from(afterStore1.toInstant())));
assertTrue(actual1.getTimestamp().after(Date.from(beforeStore.toInstant())));
}

@Test
Expand All @@ -292,8 +293,8 @@ public void testQueryUsingNameAndStartAndEndWithGTOperator() {
HistoricItem actual1 = iterator.next();
assertFalse(iterator.hasNext());
assertStateEquals(getSecondItemState(), actual1.getState());
assertTrue(actual1.getTimestamp().before(afterStore2));
assertTrue(actual1.getTimestamp().after(afterStore1));
assertTrue(actual1.getTimestamp().before(Date.from(afterStore2.toInstant())));
assertTrue(actual1.getTimestamp().after(Date.from(afterStore1.toInstant())));
}

@Test
Expand Down Expand Up @@ -322,8 +323,8 @@ public void testQueryUsingNameAndStartAndEndWithGTEOperator() {
HistoricItem actual1 = iterator.next();
assertFalse(iterator.hasNext());
assertStateEquals(getSecondItemState(), actual1.getState());
assertTrue(actual1.getTimestamp().before(afterStore2));
assertTrue(actual1.getTimestamp().after(afterStore1));
assertTrue(actual1.getTimestamp().before(Date.from(afterStore2.toInstant())));
assertTrue(actual1.getTimestamp().after(Date.from(afterStore1.toInstant())));
}

@Test
Expand All @@ -339,8 +340,8 @@ public void testQueryUsingNameAndStartAndEndFirst() {
HistoricItem actual1 = iterator.next();
assertFalse(iterator.hasNext());
assertStateEquals(getFirstItemState(), actual1.getState());
assertTrue(actual1.getTimestamp().before(afterStore1));
assertTrue(actual1.getTimestamp().after(beforeStore));
assertTrue(actual1.getTimestamp().before(Date.from(afterStore1.toInstant())));
assertTrue(actual1.getTimestamp().after(Date.from(beforeStore.toInstant())));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import static org.junit.Assert.assertEquals;

import java.util.Date;
import java.time.ZonedDateTime;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
Expand All @@ -32,24 +32,24 @@
public class CallItemIntegrationTest extends AbstractTwoItemIntegrationTest {

private static final String NAME = "call";
// values are encoded as dest##orig, ordering goes wrt strings
private static final StringListType STATE1 = new StringListType("orig1", "dest1");
private static final StringListType STATE2 = new StringListType("orig1", "dest3");
private static final StringListType STATE_BETWEEN = new StringListType("orig2", "dest2");
// values are encoded as part1,part2 - ordering goes wrt strings
private static final StringListType STATE1 = new StringListType("part1", "foo");
private static final StringListType STATE2 = new StringListType("part3", "bar");
private static final StringListType STATE_BETWEEN = new StringListType("part2", "zzz");

@BeforeClass
public static void storeData() throws InterruptedException {
CallItem item = (CallItem) ITEMS.get(NAME);
item.setState(STATE1);
beforeStore = new Date();
beforeStore = ZonedDateTime.now();
Thread.sleep(10);
service.store(item);
afterStore1 = new Date();
afterStore1 = ZonedDateTime.now();
Thread.sleep(10);
item.setState(STATE2);
service.store(item);
Thread.sleep(10);
afterStore2 = new Date();
afterStore2 = ZonedDateTime.now();

LOGGER.info("Created item between {} and {}", AbstractDynamoDBItem.DATEFORMATTER.format(beforeStore),
AbstractDynamoDBItem.DATEFORMATTER.format(afterStore1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
package org.openhab.persistence.dynamodb.internal;

import java.math.BigDecimal;
import java.util.Date;
import java.time.ZonedDateTime;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
Expand Down Expand Up @@ -52,15 +52,15 @@ private static HSBType color(String hue, int saturation, int brightness) {
public static void storeData() throws InterruptedException {
ColorItem item = (ColorItem) ITEMS.get(NAME);
item.setState(STATE1);
beforeStore = new Date();
beforeStore = ZonedDateTime.now();
Thread.sleep(10);
service.store(item);
afterStore1 = new Date();
afterStore1 = ZonedDateTime.now();
Thread.sleep(10);
item.setState(STATE2);
service.store(item);
Thread.sleep(10);
afterStore2 = new Date();
afterStore2 = ZonedDateTime.now();

LOGGER.info("Created item between {} and {}", AbstractDynamoDBItem.DATEFORMATTER.format(beforeStore),
AbstractDynamoDBItem.DATEFORMATTER.format(afterStore1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
*/
package org.openhab.persistence.dynamodb.internal;

import java.util.Date;
import java.time.ZonedDateTime;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
Expand All @@ -39,15 +39,15 @@ public class ContactItemIntegrationTest extends AbstractTwoItemIntegrationTest {
public static void storeData() throws InterruptedException {
ContactItem item = (ContactItem) ITEMS.get(NAME);
item.setState(STATE1);
beforeStore = new Date();
beforeStore = ZonedDateTime.now();
Thread.sleep(10);
service.store(item);
afterStore1 = new Date();
afterStore1 = ZonedDateTime.now();
Thread.sleep(10);
item.setState(STATE2);
service.store(item);
Thread.sleep(10);
afterStore2 = new Date();
afterStore2 = ZonedDateTime.now();

LOGGER.info("Created item between {} and {}", AbstractDynamoDBItem.DATEFORMATTER.format(beforeStore),
AbstractDynamoDBItem.DATEFORMATTER.format(afterStore1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
*/
package org.openhab.persistence.dynamodb.internal;

import java.time.ZonedDateTime;
import java.util.Calendar;
import java.util.Date;
import java.util.TimeZone;

import org.eclipse.jdt.annotation.NonNullByDefault;
Expand Down Expand Up @@ -52,15 +52,15 @@ public static void storeData() throws InterruptedException {

item.setState(STATE1);

beforeStore = new Date();
beforeStore = ZonedDateTime.now();
Thread.sleep(10);
service.store(item);
afterStore1 = new Date();
afterStore1 = ZonedDateTime.now();
Thread.sleep(10);
item.setState(STATE2);
service.store(item);
Thread.sleep(10);
afterStore2 = new Date();
afterStore2 = ZonedDateTime.now();

LOGGER.info("Created item between {} and {}", AbstractDynamoDBItem.DATEFORMATTER.format(beforeStore),
AbstractDynamoDBItem.DATEFORMATTER.format(afterStore1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
*/
package org.openhab.persistence.dynamodb.internal;

import java.util.Date;
import java.time.ZonedDateTime;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
Expand Down Expand Up @@ -40,15 +40,15 @@ public static void storeData() throws InterruptedException {

item.setState(STATE1);

beforeStore = new Date();
beforeStore = ZonedDateTime.now();
Thread.sleep(10);
service.store(item);
afterStore1 = new Date();
afterStore1 = ZonedDateTime.now();
Thread.sleep(10);
item.setState(STATE2);
service.store(item);
Thread.sleep(10);
afterStore2 = new Date();
afterStore2 = ZonedDateTime.now();

LOGGER.info("Created item between {} and {}", AbstractDynamoDBItem.DATEFORMATTER.format(beforeStore),
AbstractDynamoDBItem.DATEFORMATTER.format(afterStore1));
Expand Down
Loading