Skip to content

Commit

Permalink
[astro] Suppress timeZoneProvider global variable (openhab#7895)
Browse files Browse the repository at this point in the history
Signed-off-by: Laurent Garnier <[email protected]>
  • Loading branch information
lolodomo authored and andrewfg committed Aug 31, 2020
1 parent 93370fa commit 5e856b9
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.openhab.binding.astro.internal.handler.AstroThingHandler;
import org.openhab.binding.astro.internal.handler.MoonHandler;
import org.openhab.binding.astro.internal.handler.SunHandler;
import org.openhab.binding.astro.internal.util.PropertyUtils;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
Expand All @@ -51,12 +50,13 @@ public class AstroHandlerFactory extends BaseThingHandlerFactory {
.collect(Collectors.toSet());
private static final Map<String, AstroThingHandler> ASTRO_THING_HANDLERS = new HashMap<>();
private final CronScheduler scheduler;
private final TimeZoneProvider timeZoneProvider;

@Activate
public AstroHandlerFactory(final @Reference CronScheduler scheduler,
final @Reference TimeZoneProvider timeZoneProvider) {
this.scheduler = scheduler;
PropertyUtils.setTimeZoneProvider(timeZoneProvider);
this.timeZoneProvider = timeZoneProvider;
}

@Override
Expand All @@ -69,9 +69,9 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
ThingTypeUID thingTypeUID = thing.getThingTypeUID();
AstroThingHandler thingHandler = null;
if (thingTypeUID.equals(THING_TYPE_SUN)) {
thingHandler = new SunHandler(thing, scheduler);
thingHandler = new SunHandler(thing, scheduler, timeZoneProvider);
} else if (thingTypeUID.equals(THING_TYPE_MOON)) {
thingHandler = new MoonHandler(thing, scheduler);
thingHandler = new MoonHandler(thing, scheduler, timeZoneProvider);
}
if (thingHandler != null) {
ASTRO_THING_HANDLERS.put(thing.getUID().toString(), thingHandler);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.apache.commons.lang.time.DateFormatUtils;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.smarthome.core.i18n.TimeZoneProvider;
import org.eclipse.smarthome.core.scheduler.CronScheduler;
import org.eclipse.smarthome.core.scheduler.ScheduledCompletableFuture;
import org.eclipse.smarthome.core.thing.Channel;
Expand Down Expand Up @@ -66,6 +67,8 @@ public abstract class AstroThingHandler extends BaseThingHandler {
/** Scheduler to schedule jobs */
private final CronScheduler cronScheduler;

private final TimeZoneProvider timeZoneProvider;

private final Lock monitor = new ReentrantLock();

private final Set<ScheduledFuture<?>> scheduledFutures = new HashSet<>();
Expand All @@ -76,9 +79,10 @@ public abstract class AstroThingHandler extends BaseThingHandler {

private @Nullable ScheduledCompletableFuture<?> dailyJob;

public AstroThingHandler(Thing thing, CronScheduler scheduler) {
public AstroThingHandler(Thing thing, final CronScheduler scheduler, final TimeZoneProvider timeZoneProvider) {
super(thing);
this.cronScheduler = scheduler;
this.timeZoneProvider = timeZoneProvider;
}

@Override
Expand Down Expand Up @@ -155,15 +159,17 @@ public void publishPlanet() {
* Publishes the channel with data if it's linked.
*/
public void publishChannelIfLinked(ChannelUID channelUID) {
if (isLinked(channelUID.getId()) && getPlanet() != null) {
Planet planet = getPlanet();
if (isLinked(channelUID.getId()) && planet != null) {
final Channel channel = getThing().getChannel(channelUID.getId());
if (channel == null) {
logger.error("Cannot find channel for {}", channelUID);
return;
}
try {
AstroChannelConfig config = channel.getConfiguration().as(AstroChannelConfig.class);
updateState(channelUID, PropertyUtils.getState(channelUID, config, getPlanet()));
updateState(channelUID,
PropertyUtils.getState(channelUID, config, planet, timeZoneProvider.getTimeZone()));
} catch (Exception ex) {
logger.error("Can't update state for channel {} : {}", channelUID, ex.getMessage(), ex);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.smarthome.core.i18n.TimeZoneProvider;
import org.eclipse.smarthome.core.scheduler.CronScheduler;
import org.eclipse.smarthome.core.thing.Thing;
import org.eclipse.smarthome.core.thing.ThingTypeUID;
Expand Down Expand Up @@ -49,8 +50,8 @@ public class MoonHandler extends AstroThingHandler {
/**
* Constructor
*/
public MoonHandler(Thing thing, CronScheduler scheduler) {
super(thing, scheduler);
public MoonHandler(Thing thing, final CronScheduler scheduler, final TimeZoneProvider timeZoneProvider) {
super(thing, scheduler, timeZoneProvider);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.smarthome.core.i18n.TimeZoneProvider;
import org.eclipse.smarthome.core.scheduler.CronScheduler;
import org.eclipse.smarthome.core.thing.Thing;
import org.eclipse.smarthome.core.thing.ThingTypeUID;
Expand Down Expand Up @@ -49,8 +50,8 @@ public class SunHandler extends AstroThingHandler {
/**
* Constructor
*/
public SunHandler(Thing thing, CronScheduler scheduler) {
super(thing, scheduler);
public SunHandler(Thing thing, final CronScheduler scheduler, final TimeZoneProvider timeZoneProvider) {
super(thing, scheduler, timeZoneProvider);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@
import java.lang.reflect.Method;
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.Calendar;
import java.util.GregorianCalendar;
import java.util.TimeZone;

import org.apache.commons.lang.StringUtils;
import org.eclipse.smarthome.core.i18n.TimeZoneProvider;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.smarthome.core.library.types.DateTimeType;
import org.eclipse.smarthome.core.library.types.DecimalType;
import org.eclipse.smarthome.core.library.types.StringType;
Expand All @@ -37,19 +39,19 @@
* @author Erdoan Hadzhiyusein - Adapted the class to work with the new DateTimeType
* @author Christoph Weitkamp - Introduced UoM
*/
@NonNullByDefault
public class PropertyUtils {

/** Constructor */
private PropertyUtils() {
throw new IllegalAccessError("Non-instantiable");
}

private static TimeZoneProvider timeZoneProvider;

/**
* Returns the state of the channel.
*/
public static State getState(ChannelUID channelUID, AstroChannelConfig config, Object instance) throws Exception {
public static State getState(ChannelUID channelUID, AstroChannelConfig config, Object instance, ZoneId zoneId)
throws Exception {
Object value = getPropertyValue(channelUID, instance);
if (value == null) {
return UnDefType.UNDEF;
Expand All @@ -58,7 +60,7 @@ public static State getState(ChannelUID channelUID, AstroChannelConfig config, O
} else if (value instanceof Calendar) {
Calendar cal = (Calendar) value;
GregorianCalendar gregorianCal = (GregorianCalendar) DateTimeUtils.applyConfig(cal, config);
cal.setTimeZone(TimeZone.getTimeZone(timeZoneProvider.getTimeZone()));
cal.setTimeZone(TimeZone.getTimeZone(zoneId));
ZonedDateTime zoned = gregorianCal.toZonedDateTime().withFixedOffsetZone();
return new DateTimeType(zoned);
} else if (value instanceof Number) {
Expand All @@ -71,15 +73,11 @@ public static State getState(ChannelUID channelUID, AstroChannelConfig config, O
}
}

public static void setTimeZoneProvider(TimeZoneProvider timeZoneProvider) {
PropertyUtils.timeZoneProvider = timeZoneProvider;
}

/**
* Returns the property value from the object instance, nested properties are possible. If the propertyName is for
* example rise.start, the methods getRise().getStart() are called.
*/
public static Object getPropertyValue(ChannelUID channelUID, Object instance) throws Exception {
public static @Nullable Object getPropertyValue(ChannelUID channelUID, Object instance) throws Exception {
String[] properties = StringUtils.split(channelUID.getId(), "#");
return getPropertyValue(instance, properties, 0);
}
Expand All @@ -88,7 +86,8 @@ public static Object getPropertyValue(ChannelUID channelUID, Object instance) th
* Iterates through the nested properties and returns the getter value.
*/
@SuppressWarnings("all")
private static Object getPropertyValue(Object instance, String[] properties, int nestedIndex) throws Exception {
private static @Nullable Object getPropertyValue(Object instance, String[] properties, int nestedIndex)
throws Exception {
String propertyName = properties[nestedIndex];
Method m = instance.getClass().getMethod(toGetterString(propertyName), null);
Object result = m.invoke(instance, (Object[]) null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@
*/
package org.openhab.binding.astro.internal.model;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.*;

import java.time.ZoneId;

import org.eclipse.smarthome.core.library.types.StringType;
import org.eclipse.smarthome.core.thing.ChannelUID;
import org.eclipse.smarthome.core.types.UnDefType;
import org.junit.Before;
import org.junit.Test;
import org.openhab.binding.astro.internal.config.AstroChannelConfig;
import org.openhab.binding.astro.internal.util.PropertyUtils;

/***
Expand All @@ -35,39 +35,46 @@
public class SunTest {

private Sun sun;
private AstroChannelConfig config;

private static ZoneId ZONE = ZoneId.systemDefault();

@Before
public void init() {
sun = new Sun();
config = new AstroChannelConfig();
}

@Test
public void testConstructor() throws Exception {
assertNotNull(sun.getPhase());
assertEquals(UnDefType.UNDEF, PropertyUtils.getState(new ChannelUID("astro:sun:home:phase#name"), null, sun));
assertEquals(UnDefType.UNDEF,
PropertyUtils.getState(new ChannelUID("astro:sun:home:phase#name"), config, sun, ZONE));
}

@Test
public void testGetStateWhenNullPhaseName() throws Exception {
sun.getPhase().setName(null);

assertEquals(UnDefType.UNDEF, PropertyUtils.getState(new ChannelUID("astro:sun:home:phase#name"), null, sun));
assertEquals(UnDefType.UNDEF,
PropertyUtils.getState(new ChannelUID("astro:sun:home:phase#name"), config, sun, ZONE));
}

@Test
public void testGetStateWhenNotNullPhaseName() throws Exception {
sun.getPhase().setName(SunPhaseName.DAYLIGHT);

assertEquals(new StringType("DAYLIGHT"),
PropertyUtils.getState(new ChannelUID("astro:sun:home:phase#name"), null, sun));
PropertyUtils.getState(new ChannelUID("astro:sun:home:phase#name"), config, sun, ZONE));
}

@Test(expected = NullPointerException.class)
public void testGetStateWhenNullPhase() throws Exception {
sun.setPhase(null);

assertNull(sun.getPhase());
assertEquals(UnDefType.UNDEF, PropertyUtils.getState(new ChannelUID("astro:sun:home:phase#name"), null, sun));
assertEquals(UnDefType.UNDEF,
PropertyUtils.getState(new ChannelUID("astro:sun:home:phase#name"), config, sun, ZONE));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
import static org.openhab.binding.astro.internal.AstroBindingConstants.THING_TYPE_SUN;
import static org.openhab.binding.astro.test.cases.AstroBindingTestsData.*;

import java.time.ZoneId;

import org.eclipse.smarthome.config.core.Configuration;
import org.eclipse.smarthome.core.i18n.TimeZoneProvider;
import org.eclipse.smarthome.core.scheduler.CronScheduler;
import org.eclipse.smarthome.core.thing.Channel;
import org.eclipse.smarthome.core.thing.ChannelUID;
Expand Down Expand Up @@ -60,7 +63,9 @@ public void testRefreshCommandUpdatesTheStateOfTheChannels() {

ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
CronScheduler cronScheduler = mock(CronScheduler.class);
AstroThingHandler sunHandler = spy(new SunHandler(thing, cronScheduler));
TimeZoneProvider timeZoneProvider = mock(TimeZoneProvider.class);
when(timeZoneProvider.getTimeZone()).thenReturn(ZoneId.systemDefault());
AstroThingHandler sunHandler = spy(new SunHandler(thing, cronScheduler, timeZoneProvider));

// Required from the AstroThingHandler to send the status update
doReturn(true).when(callback).isChannelLinked(eq(channelUID));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
import static org.openhab.binding.astro.internal.AstroBindingConstants.THING_TYPE_SUN;
import static org.openhab.binding.astro.test.cases.AstroBindingTestsData.*;

import java.time.ZoneId;

import org.eclipse.smarthome.config.core.Configuration;
import org.eclipse.smarthome.core.i18n.TimeZoneProvider;
import org.eclipse.smarthome.core.scheduler.CronScheduler;
import org.eclipse.smarthome.core.thing.Thing;
import org.eclipse.smarthome.core.thing.ThingStatus;
Expand Down Expand Up @@ -143,7 +146,9 @@ private void assertThingStatus(Configuration configuration, ThingStatus expected

ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
CronScheduler cronScheduler = mock(CronScheduler.class);
ThingHandler sunHandler = new SunHandler(thing, cronScheduler);
TimeZoneProvider timeZoneProvider = mock(TimeZoneProvider.class);
when(timeZoneProvider.getTimeZone()).thenReturn(ZoneId.systemDefault());
ThingHandler sunHandler = new SunHandler(thing, cronScheduler, timeZoneProvider);
sunHandler.setCallback(callback);

sunHandler.initialize();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.GregorianCalendar;
import java.util.List;

import org.eclipse.smarthome.core.i18n.TimeZoneProvider;
import org.eclipse.smarthome.core.thing.ChannelUID;
import org.eclipse.smarthome.core.thing.ThingUID;
import org.eclipse.smarthome.core.types.State;
Expand Down Expand Up @@ -72,13 +71,6 @@ public static List<Object[]> data() {

@Test
public void testParametrized() {
// Anonymous implementation of the service to adapt the time zone to the tested longitude and latitude
PropertyUtils.setTimeZoneProvider(new TimeZoneProvider() {
@Override
public ZoneId getTimeZone() {
return ZONE_ID;
}
});
try {
assertStateUpdate(thingID, channelId, expectedState);
} catch (Exception e) {
Expand All @@ -88,7 +80,7 @@ public ZoneId getTimeZone() {

private void assertStateUpdate(String thingID, String channelId, State expectedState) throws Exception {
ChannelUID testItemChannelUID = new ChannelUID(getThingUID(thingID), channelId);
State state = PropertyUtils.getState(testItemChannelUID, new AstroChannelConfig(), getPlanet(thingID));
State state = PropertyUtils.getState(testItemChannelUID, new AstroChannelConfig(), getPlanet(thingID), ZONE_ID);
assertEquals(expectedState, state);
}

Expand Down

0 comments on commit 5e856b9

Please sign in to comment.