Skip to content

Make the data update follow the specification #79

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.

Sign up for GitHub

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

Draft
jwrosewell wants to merge 7 commits into
base: rc
Choose a base branch
from
Draft

Make the data update follow the specification #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
74b0725
Test the data update logging, the If-Modified-Since time and the lice…
jwrosewell Nov 9, 2026
b989bc4
Make the data update follow the specification
jwrosewell Nov 9, 2026
733a1e2
Test that HTTP status failures are warnings
jwrosewell Nov 9, 2026
98f3a9a
Log a 429, a 403 and any other failing HTTP status as a warning
jwrosewell Nov 9, 2026
5a1c49e
Merge rc into the data update draft
jwrosewell Nov 9, 2026
6169531
Test that the update on start-up log names the engine type
jwrosewell Nov 9, 2026
adce29b
Name the engine in the update on start-up log before the engine exists
jwrosewell Nov 9, 2026
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
8 changes: 8 additions & 0 deletions ...n/src/test/java/fiftyone/devicedetection/DeviceDetectionOnPremisePipelineBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public void setup() throws Exception {
when(loggerFactory.getLogger(anyString())).thenReturn(logger);
builder = new DeviceDetectionPipelineBuilder(loggerFactory)
.useOnPremise(datafile.getAbsolutePath(), false)
.setAutoUpdate(false)
.setShareUsage(false);
}

Expand Down Expand Up @@ -122,6 +123,7 @@ public void setEngineData_PropertyPropagatedToDataFileConfiguration() throws Exc
try (Pipeline pipeline = new DeviceDetectionPipelineBuilder(loggerFactory)
.useOnPremise(new byte[]{}, Enums.DeviceDetectionAlgorithm.Hash)
.setEngineData(data)
.setAutoUpdate(false)
.setShareUsage(false)
.build()) {
DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline);
Expand All @@ -144,6 +146,7 @@ public void setEngineData_FilenameIsSet_PropertyNotPropagated() throws Exception
public void setShareUsage_False_SharedFlowElementNotAdded() throws Exception {
try (Pipeline pipeline = new DeviceDetectionPipelineBuilder(loggerFactory)
.useOnPremise(datafile.getAbsolutePath(), false)
.setAutoUpdate(false)
.setShareUsage(false)
.build()) {
List<FlowElement> flowElements = pipeline.getFlowElements();
Expand All @@ -156,6 +159,7 @@ public void setShareUsage_False_SharedFlowElementNotAdded() throws Exception {
public void setShareUsage_True_SharedFlowElementAdded() throws Exception {
try (Pipeline pipeline = new DeviceDetectionPipelineBuilder(loggerFactory)
.useOnPremise(datafile.getAbsolutePath(), false)
.setAutoUpdate(false)
.setShareUsage(true)
.build()) {
List<FlowElement> flowElements = pipeline.getFlowElements();
Expand All @@ -168,6 +172,7 @@ public void setShareUsage_True_SharedFlowElementAdded() throws Exception {
public void setAutoUpdate_PropertyPropagatedToDataFileConfiguration() throws Exception {
try (Pipeline pipeline = builder
.setAutoUpdate(true)
.setDataUpdateLicenseKey("licence_key")
.build()) {
DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline);
assertTrue(dataFileConfiguration.getAutomaticUpdatesEnabled());
Expand All @@ -189,6 +194,7 @@ public void setDataUpdateService_RegisterDataFileTriggered() throws Exception {
DataUpdateServiceDefault dataUpdateServiceDefault = mock(DataUpdateServiceDefault.class);
try (Pipeline ignored = builder
.setAutoUpdate(true)
.setDataUpdateLicenseKey("licence_key")
.setDataUpdateService(dataUpdateServiceDefault)
.build()) {
verify(dataUpdateServiceDefault).registerDataFile(any());
Expand All @@ -199,6 +205,8 @@ public void setDataUpdateService_RegisterDataFileTriggered() throws Exception {
public void setDataUpdateOnStartup_PropertyPropagatedToDataFileConfiguration() throws Exception {
try (Pipeline pipeline = builder
.setDataUpdateOnStartup(true)
.setDataUpdateLicenseKey("licence_key")
.setDataUpdateService(mock(DataUpdateServiceDefault.class))
.build()) {
DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline);
assertTrue(dataFileConfiguration.getUpdateOnStartup());
Expand Down
7 changes: 7 additions & 0 deletions ...nce/src/test/java/fiftyone/ipintelligence/IPIntelligenceOnPremisePipelineBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public void setup() throws Exception {
when(loggerFactory.getLogger(anyString())).thenReturn(logger);
builder = new IPIntelligencePipelineBuilder(loggerFactory)
.useOnPremise(datafile.getAbsolutePath(), false)
.setAutoUpdate(false)
.setShareUsage(false);
}

Expand Down Expand Up @@ -121,6 +122,7 @@ public void setFilename_OtherExtension_ExceptionThrown() {
public void setShareUsage_False_SharedFlowElementNotAdded() throws Exception {
try (Pipeline pipeline = new IPIntelligencePipelineBuilder(loggerFactory)
.useOnPremise(datafile.getAbsolutePath(), false)
.setAutoUpdate(false)
.setShareUsage(false)
.build()) {
List<FlowElement> flowElements = pipeline.getFlowElements();
Expand All @@ -133,6 +135,7 @@ public void setShareUsage_False_SharedFlowElementNotAdded() throws Exception {
public void setShareUsage_True_SharedFlowElementAdded() throws Exception {
try (Pipeline pipeline = new IPIntelligencePipelineBuilder(loggerFactory)
.useOnPremise(datafile.getAbsolutePath(), false)
.setAutoUpdate(false)
.setShareUsage(true)
.build()) {
List<FlowElement> flowElements = pipeline.getFlowElements();
Expand All @@ -145,6 +148,7 @@ public void setShareUsage_True_SharedFlowElementAdded() throws Exception {
public void setAutoUpdate_PropertyPropagatedToDataFileConfiguration() throws Exception {
try (Pipeline pipeline = builder
.setAutoUpdate(true)
.setDataUpdateLicenseKey("licence_key")
.build()) {
DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline);
assertTrue(dataFileConfiguration.getAutomaticUpdatesEnabled());
Expand All @@ -166,6 +170,7 @@ public void setDataUpdateService_RegisterDataFileTriggered() throws Exception {
DataUpdateServiceDefault dataUpdateServiceDefault = mock(DataUpdateServiceDefault.class);
try (Pipeline ignored = builder
.setAutoUpdate(true)
.setDataUpdateLicenseKey("licence_key")
.setDataUpdateService(dataUpdateServiceDefault)
.build()) {
verify(dataUpdateServiceDefault).registerDataFile(any());
Expand All @@ -176,6 +181,8 @@ public void setDataUpdateService_RegisterDataFileTriggered() throws Exception {
public void setDataUpdateOnStartup_PropertyPropagatedToDataFileConfiguration() throws Exception {
try (Pipeline pipeline = builder
.setDataUpdateOnStartup(true)
.setDataUpdateLicenseKey("licence_key")
.setDataUpdateService(mock(DataUpdateServiceDefault.class))
.build()) {
DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline);
assertTrue(dataFileConfiguration.getUpdateOnStartup());
Expand Down
12 changes: 12 additions & 0 deletions ...yone/pipeline/engines/fiftyone/flowelements/FiftyOneOnPremiseAspectEngineBuilderBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

package fiftyone.pipeline.engines.fiftyone.flowelements;

import fiftyone.pipeline.engines.configuration.DataFileConfiguration;
import fiftyone.pipeline.engines.data.AspectEngineDataFile;
import fiftyone.pipeline.engines.fiftyone.data.FiftyOneDataFile;
import fiftyone.pipeline.engines.fiftyone.data.FiftyOneDataFileDefault;
Expand Down Expand Up @@ -80,6 +81,17 @@ public FiftyOneOnPremiseAspectEngineBuilderBase(
setDataUpdateUrlFormatter(new FiftyOneUrlFormatter());
}

/**
* The Distributor serves nothing without a license key, so a key is
* required while the data update URL is the Distributor's.
* @param dataFileConfig the data file configuration to check
* @return true if the data update URL is the Distributor's
*/
@Override
protected boolean isLicenseKeyRequired(DataFileConfiguration dataFileConfig) {
return DATA_UPDATE_URL.equals(dataFileConfig.getDataUpdateUrl());
}

/**
* Create a new empty data file instance to be populated with the details of
* the data file to be used.
Expand Down
127 changes: 127 additions & 0 deletions ...e/engines/fiftyone/flowelements/FiftyOneDegreesOnPremiseAspectEngineBuilderBaseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@

package fiftyone.pipeline.engines.fiftyone.flowelements;

import fiftyone.pipeline.core.exceptions.PipelineConfigurationException;
import fiftyone.pipeline.engines.Constants;
import fiftyone.pipeline.engines.data.AspectData;
import fiftyone.pipeline.engines.data.AspectEngineDataFile;
import fiftyone.pipeline.engines.data.AspectEngineDataFileDefault;
import fiftyone.pipeline.engines.data.AspectPropertyMetaData;
import fiftyone.pipeline.engines.fiftyone.data.FiftyOneDataFile;
import fiftyone.pipeline.engines.services.DataUpdateService;
import org.mockito.ArgumentCaptor;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.invocation.InvocationOnMock;
Expand All @@ -38,7 +41,10 @@
import java.util.List;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.*;

Expand Down Expand Up @@ -118,6 +124,7 @@ public void DataFile_SetEngine() throws Exception {
ImplementedBuilder builder = new ImplementedBuilder(
LoggerFactory.getILoggerFactory(),
updateService);
builder.setAutoUpdate(false);
builder.build("", false);

assertNotNull(engine.getDataFileMetaData());
Expand Down Expand Up @@ -145,4 +152,124 @@ public void DataFile_UrlFormatter() throws Exception {
"https://distributor.51degrees.com/api/v2/download?LicenseKeys="+licenseKey+"&Download=True&Type="+downloadType,
engine.getDataFileMetaData().getFormattedUrl());
}

private ImplementedBuilder newBuilder() {
return new ImplementedBuilder(
LoggerFactory.getILoggerFactory(),
updateService);
}

/**
* The data file is registered with the update service when update on
* start-up is on, even though auto update is off.
*/
@Test
public void UpdateOnStartup_AutoUpdateOff_DataFileRegistered()
throws Exception {
ImplementedBuilder builder = newBuilder()
.setAutoUpdate(false)
.setDataUpdateOnStartup(true)
.setDataUpdateLicenseKey(licenseKey);

builder.build("", false);

verify(updateService, times(1)).registerDataFile(any());
}

/**
* The engine does not exist when the data file is registered, so the
* data file carries the engine's type name for the update service's log.
*/
@Test
public void UpdateOnStartup_DataFileCarriesTheEngineTypeName()
throws Exception {
ImplementedBuilder builder = newBuilder()
.setAutoUpdate(false)
.setDataUpdateOnStartup(true)
.setDataUpdateLicenseKey(licenseKey);

builder.build("", false);

ArgumentCaptor<AspectEngineDataFile> registered =
ArgumentCaptor.forClass(AspectEngineDataFile.class);
verify(updateService).registerDataFile(registered.capture());
assertEquals("Implemented",
((AspectEngineDataFileDefault) registered.getValue())
.getEngineTypeName());
}

@Test
public void AutoUpdateAndUpdateOnStartupOff_DataFileNotRegistered()
throws Exception {
ImplementedBuilder builder = newBuilder()
.setAutoUpdate(false)
.setDataUpdateOnStartup(false);

builder.build("", false);

verify(updateService, never()).registerDataFile(any());
}

/**
* The default update URL is the Distributor's, which serves nothing
* without a license key.
*/
@Test
public void AutoUpdate_DefaultUrlNoLicenseKey_BuildRefused() {
ImplementedBuilder builder = newBuilder().setAutoUpdate(true);

PipelineConfigurationException e = assertThrows(
PipelineConfigurationException.class,
() -> builder.build("", false));

assertTrue(e.getMessage().contains("setAutoUpdate(false)"));
assertFalse(e.getMessage().contains("setDataUpdateOnStartup(false)"));
verify(updateService, never()).registerDataFile(any());
}

@Test
public void UpdateOnStartup_DefaultUrlNoLicenseKey_BuildRefused() {
ImplementedBuilder builder = newBuilder()
.setAutoUpdate(false)
.setDataUpdateOnStartup(true);

PipelineConfigurationException e = assertThrows(
PipelineConfigurationException.class,
() -> builder.build("", false));

assertTrue(e.getMessage().contains("setDataUpdateOnStartup(false)"));
assertFalse(e.getMessage().contains("setAutoUpdate(false)"));
}

@Test
public void AutoUpdate_LicenseKeySet_BuildAllowed() throws Exception {
ImplementedBuilder builder = newBuilder()
.setAutoUpdate(true)
.setDataUpdateLicenseKey(licenseKey);

builder.build("", false);

verify(updateService, times(1)).registerDataFile(any());
}

@Test
public void AutoUpdate_OtherUrlNoLicenseKey_BuildAllowed()
throws Exception {
ImplementedBuilder builder = newBuilder()
.setAutoUpdate(true)
.setDataUpdateUrl("https://example.com/data.file");

builder.build("", false);

verify(updateService, times(1)).registerDataFile(any());
}

@Test
public void AutoUpdateOff_NoLicenseKey_BuildAllowed() throws Exception {
ImplementedBuilder builder = newBuilder().setAutoUpdate(false);

builder.build("", false);

verify(updateService, never()).registerDataFile(any());
}
}
18 changes: 18 additions & 0 deletions ...ine.engines/src/main/java/fiftyone/pipeline/engines/data/AspectEngineDataFileDefault.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public class AspectEngineDataFileDefault implements AspectEngineDataFile {
private DataFileConfiguration configuration;
private DataUpdateService dataUpdateService = null;
private String tempDataDirPath;
private String engineTypeName;

@Override
public String getIdentifier() {
Expand Down Expand Up @@ -87,6 +88,23 @@ public void setTempDataFilePath(String path) {
this.tempDataFilePath = path;
}

/**
* The type name of the engine this data file is for, which the update
* service logs until the engine is set.
* @return the engine type name, or null if it is not known
*/
public String getEngineTypeName() {
return engineTypeName;
}

/**
* Set the type name of the engine this data file is for.
* @param engineTypeName the engine type name
*/
public void setEngineTypeName(String engineTypeName) {
this.engineTypeName = engineTypeName;
}

@Override
public String getTempDataDirPath() {
if (tempDataDirPath == null &&
Expand Down
Loading
Loading