diff --git a/.github/workflows/nightly-pipeline.yml b/.github/workflows/nightly-pipeline.yml index e93a02f..41a4ae9 100644 --- a/.github/workflows/nightly-pipeline.yml +++ b/.github/workflows/nightly-pipeline.yml @@ -7,7 +7,7 @@ on: type: boolean default: false schedule: - - cron: '0 2 * * *' + - cron: "0 2 * * *" jobs: PackageUpdate: @@ -31,7 +31,7 @@ jobs: cache-assets: true secrets: token: ${{ secrets.ACCESS_TOKEN }} - DeviceDetectionUrl: ${{ secrets.IPI_DATA_FILE_URL }} + DeviceDetectionUrl: ${{ secrets.IPI_DATA_FILE_SAMPLE_URL }} # TODO: Do not merge PR untill you see this line IPI_DATA_FILE_URL Publish: if: ${{ !cancelled() }} @@ -45,4 +45,4 @@ jobs: cache-assets: true secrets: token: ${{ secrets.ACCESS_TOKEN }} - DeviceDetectionUrl: ${{ secrets.IPI_DATA_FILE_URL }} + DeviceDetectionUrl: ${{ secrets.IPI_DATA_FILE_SAMPLE_URL }} # TODO: Do not merge PR untill you see this line IPI_DATA_FILE_URL diff --git a/.github/workflows/nightly-pull-requests.yml b/.github/workflows/nightly-pull-requests.yml index a033321..a6e7aa0 100644 --- a/.github/workflows/nightly-pull-requests.yml +++ b/.github/workflows/nightly-pull-requests.yml @@ -18,4 +18,4 @@ jobs: cache-assets: true secrets: token: ${{ secrets.ACCESS_TOKEN }} - DeviceDetectionUrl: ${{ secrets.IPI_DATA_FILE_URL }} + DeviceDetectionUrl: ${{ secrets.IPI_DATA_FILE_SAMPLE_URL }} # TODO: Do not merge PR untill you see this line IPI_DATA_FILE_URL diff --git a/.gitignore b/.gitignore index c5e4b33..2604e53 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,11 @@ -.idea +# IDE specific files +.idea/ +.idea/* +.vscode/ +.vscode/* +*.swp +*.swo +*~ +.DS_Store 4.**/ diff --git a/ipi_interop/exception.go b/ipi_interop/exception.go index b6e4445..cd9a8c9 100644 --- a/ipi_interop/exception.go +++ b/ipi_interop/exception.go @@ -25,6 +25,7 @@ package ipi_interop //#include //#include "ip-intelligence-cxx.h" import "C" +import "runtime" const StatusNotSet = C.FIFTYONE_DEGREES_STATUS_NOT_SET @@ -33,14 +34,32 @@ type Exception struct { CPtr *C.Exception } +// exceptionFinalizer check if C resource has been explicitly +// freed by Free method. Panic if it was not. +func exceptionFinalizer(e *Exception) { + if e.CPtr != nil { + panic("ERROR: Exception should be freed explicitly by its Free method.") + } +} + // NewException creates a new Exception object func NewException() *Exception { ce := new(C.Exception) e := &Exception{ce} e.Clear() + runtime.SetFinalizer(e, exceptionFinalizer) return e } +// Free frees the Exception object and sets CPtr to nil +func (e *Exception) Free() { + if e.CPtr != nil { + // Exception is Go-allocated, just set to nil + // No C.free needed as it's not C.malloc'd + e.CPtr = nil + } +} + // Clear resets the Exception object func (e *Exception) Clear() { e.CPtr.file = nil diff --git a/ipi_interop/resource_ipi.go b/ipi_interop/resource_ipi.go index 0e5b94f..897abeb 100644 --- a/ipi_interop/resource_ipi.go +++ b/ipi_interop/resource_ipi.go @@ -36,6 +36,7 @@ import ( // fiftyoneDegreesIpiInitManagerFromFile. func InitManagerFromFile(manager *ResourceManager, config ConfigIpi, properties string, filePath string) error { exp := NewException() + defer exp.Free() cPath := C.CString(filePath) defer C.free(unsafe.Pointer(cPath)) @@ -71,6 +72,7 @@ func InitManagerFromFile(manager *ResourceManager, config ConfigIpi, properties // fiftyoneDegreesIpiReloadManagerFromFile. func (manager *ResourceManager) ReloadFromFile(config ConfigIpi, properties string, filePath string) error { exp := NewException() + defer exp.Free() cPath := C.CString(filePath) defer C.free(unsafe.Pointer(cPath)) @@ -107,6 +109,8 @@ func (manager *ResourceManager) ReloadFromFile(config ConfigIpi, properties stri // fiftyoneDegreesIpiReloadManagerFromOriginalFile func (manager *ResourceManager) ReloadFromOriginalFile() error { exp := NewException() + defer exp.Free() + C.IpiReloadManagerFromOriginalFile( manager.CPtr, exp.CPtr, diff --git a/ipi_interop/results_ipi.go b/ipi_interop/results_ipi.go index 54b4820..118df2a 100644 --- a/ipi_interop/results_ipi.go +++ b/ipi_interop/results_ipi.go @@ -50,8 +50,8 @@ func NewResultsIpi(manager *ResourceManager) *ResultsIpi { var cResults interface{} = (*[math.MaxInt32 / int(C.sizeof_ResultIpi)]C.ResultIpi)(unsafe.Pointer(r.items))[:r.capacity:r.capacity] res := &ResultsIpi{ - CPtr: r, - CResults: &cResults, + CPtr: r, + CResults: &cResults, } runtime.SetFinalizer(res, resultsFinalizer) @@ -62,6 +62,7 @@ func NewResultsIpi(manager *ResourceManager) *ResultsIpi { // Returns an error if the operation fails. func (r *ResultsIpi) ResultsIpiFromIpAddress(ipAddress string) error { exception := NewException() + defer exception.Free() char := C.CString(ipAddress) defer C.free(unsafe.Pointer(char)) @@ -113,7 +114,6 @@ func (r *ResultsIpi) GetPropertyIndexByName(propertyName string) int { return int(i) } - // getPropertyNameSafe retrieves the property name associated with a required index from the given dataset safely. // Returns an empty string if the required index is invalid or out of bounds. func (r *ResultsIpi) getPropertyNameSafe(dataSet *C.DataSetIpi, requiredIndex C.int) string { @@ -136,14 +136,13 @@ type header struct { rawWeighting C.uint16_t } - - // GetWeightedValuesByIndexes retrieves weighted values using pre-computed property indexes // and a property name resolver function to avoid expensive CGO calls for name resolution. // The resolver function should provide fast index→name mapping (e.g., from Engine's cache). func (r *ResultsIpi) GetWeightedValuesByIndexes(indexes []int, propertyNameResolver func(int) string) (Values, error) { dataSet := (*C.DataSetIpi)(r.CPtr.b.dataSet) exception := NewException() + defer exception.Free() var cIndexes *C.int var cIndexesCount C.uint @@ -151,6 +150,10 @@ func (r *ResultsIpi) GetWeightedValuesByIndexes(indexes []int, propertyNameResol if len(indexes) > 0 { // Allocate C memory for the array cIndexes = (*C.int)(C.malloc(C.size_t(len(indexes)) * C.size_t(unsafe.Sizeof(C.int(0))))) + if cIndexes == nil { + return nil, fmt.Errorf("failed to allocate memory for indexes") + } + defer C.free(unsafe.Pointer(cIndexes)) // Copy Go slice elements to C array @@ -168,12 +171,12 @@ func (r *ResultsIpi) GetWeightedValuesByIndexes(indexes []int, propertyNameResol cIndexesCount, nil, exception.CPtr, ) + // Release the collection + defer C.fiftyoneDegreesWeightedValuesCollectionRelease(&collection) + if !exception.IsOkay() { return nil, fmt.Errorf(C.GoString(C.ExceptionGetMessage(exception.CPtr))) } - - // Release the collection - defer C.fiftyoneDegreesWeightedValuesCollectionRelease(&collection) values := make(Values, collection.itemsCount) @@ -226,12 +229,15 @@ func (r *ResultsIpi) GetWeightedValuesByIndexes(indexes []int, propertyNameResol val = C.GoString(weightedString.value) } - // Calculate weight - weight := float64(nextHeader.rawWeighting) / uint16Max - // append values to the map - values.Append(propName, val, weight) + // For MCC property, append with weight; for all others, append without weight + if propName == "Mcc" { + weight := float64(nextHeader.rawWeighting) / uint16Max + values.AppendWithWeight(propName, val, weight) + } else { + values.Append(propName, val) + } } return values, nil -} \ No newline at end of file +} diff --git a/ipi_interop/weighted_value.go b/ipi_interop/weighted_value.go index 82d75dd..ef43bd2 100644 --- a/ipi_interop/weighted_value.go +++ b/ipi_interop/weighted_value.go @@ -21,7 +21,8 @@ * ********************************************************************* */ package ipi_interop -// WeightedValue represents a value paired with a specific weight for use in weighted calculations or prioritization. +// WeightedValue represents a value that may include a weight (used for MCC property). +// For non-weighted properties, Weight will be 0.0. type WeightedValue struct { Value interface{} Weight float64 @@ -30,6 +31,16 @@ type WeightedValue struct { // Values is a map where each key is a string representing a property, and the value is a slice of WeightedValue pointers. type Values map[string][]*WeightedValue +// GetValueByProperty retrieves the first value for the specified property (ignoring weight). +// This is the recommended method for all properties except MCC. +// Returns the value and a boolean indicating success or failure. +func (v Values) GetValueByProperty(property string) (interface{}, bool) { + if val, ok := v[property]; ok && len(val) > 0 { + return val[0].Value, true + } + return "", false +} + // GetValueWeightByProperty retrieves the first value and its weight for the specified property. // Returns the value, weight, and a boolean indicating success or failure. func (v Values) GetValueWeightByProperty(property string) (interface{}, float64, bool) { @@ -39,7 +50,18 @@ func (v Values) GetValueWeightByProperty(property string) (interface{}, float64, return "", 0, false } -func (v Values) Append(property string, value interface{}, weight float64) { +// Append adds a value without weight (Weight will be set to 0.0). +// Use this for all properties except MCC. +func (v Values) Append(property string, value interface{}) { + v[property] = append(v[property], &WeightedValue{ + Value: value, + Weight: 0.0, + }) +} + +// AppendWithWeight adds a value with a specific weight. +// This should only be used for MCC property. +func (v Values) AppendWithWeight(property string, value interface{}, weight float64) { v[property] = append(v[property], &WeightedValue{ Value: value, Weight: weight, diff --git a/ipi_interop/weighted_value_test.go b/ipi_interop/weighted_value_test.go index dbf83ee..0e81272 100644 --- a/ipi_interop/weighted_value_test.go +++ b/ipi_interop/weighted_value_test.go @@ -233,125 +233,87 @@ func TestValues_Append(t *testing.T) { initialValues Values property string value interface{} - weight float64 expectedLength int expectedValue interface{} - expectedWeight float64 }{ { name: "append to empty values", initialValues: make(Values), property: "browser", value: "Chrome", - weight: 0.9, expectedLength: 1, expectedValue: "Chrome", - expectedWeight: 0.9, }, { name: "append to existing property", initialValues: Values{ "browser": []*WeightedValue{ - {Value: "Firefox", Weight: 0.8}, + {Value: "Firefox", Weight: 0.0}, }, }, property: "browser", value: "Chrome", - weight: 0.9, expectedLength: 2, expectedValue: "Chrome", - expectedWeight: 0.9, }, { name: "append to new property in existing values", initialValues: Values{ "os": []*WeightedValue{ - {Value: "Windows", Weight: 0.7}, + {Value: "Windows", Weight: 0.0}, }, }, property: "browser", value: "Safari", - weight: 0.6, expectedLength: 1, expectedValue: "Safari", - expectedWeight: 0.6, }, { name: "append integer value", initialValues: make(Values), property: "version", value: 42, - weight: 1.0, expectedLength: 1, expectedValue: 42, - expectedWeight: 1.0, }, { name: "append boolean value", initialValues: make(Values), property: "mobile", value: true, - weight: 0.95, expectedLength: 1, expectedValue: true, - expectedWeight: 0.95, }, { name: "append float64 value", initialValues: make(Values), property: "score", value: 3.14159, - weight: 0.5, expectedLength: 1, expectedValue: 3.14159, - expectedWeight: 0.5, }, { name: "append nil value", initialValues: make(Values), property: "nullable", value: nil, - weight: 0.0, expectedLength: 1, expectedValue: nil, - expectedWeight: 0.0, }, { name: "append with empty string property", initialValues: make(Values), property: "", value: "empty_key", - weight: 1.0, expectedLength: 1, expectedValue: "empty_key", - expectedWeight: 1.0, - }, - { - name: "append with zero weight", - initialValues: make(Values), - property: "zero_weight", - value: "test", - weight: 0.0, - expectedLength: 1, - expectedValue: "test", - expectedWeight: 0.0, - }, - { - name: "append with negative weight", - initialValues: make(Values), - property: "negative", - value: "test", - weight: -0.5, - expectedLength: 1, - expectedValue: "test", - expectedWeight: -0.5, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { values := tt.initialValues - values.Append(tt.property, tt.value, tt.weight) + values.Append(tt.property, tt.value) // Check if property exists propertyValues, exists := values[tt.property] @@ -377,8 +339,9 @@ func TestValues_Append(t *testing.T) { t.Errorf("Append() value = %v, want %v", lastValue.Value, tt.expectedValue) } - if lastValue.Weight != tt.expectedWeight { - t.Errorf("Append() weight = %v, want %v", lastValue.Weight, tt.expectedWeight) + // Weight should be 0.0 for non-weighted append + if lastValue.Weight != 0.0 { + t.Errorf("Append() weight = %v, want 0.0", lastValue.Weight) } }) } @@ -485,8 +448,8 @@ func TestValues_AppendAfterInit(t *testing.T) { } // Append some values - values.Append(property, "value1", 0.8) - values.Append(property, "value2", 0.6) + values.Append(property, "value1") + values.Append(property, "value2") // Verify append worked correctly if len(values[property]) != 2 { @@ -494,12 +457,12 @@ func TestValues_AppendAfterInit(t *testing.T) { } // Verify values are in correct order - if values[property][0].Value != "value1" || values[property][0].Weight != 0.8 { + if values[property][0].Value != "value1" || values[property][0].Weight != 0.0 { t.Errorf("First appended value incorrect: got %v with weight %v", values[property][0].Value, values[property][0].Weight) } - if values[property][1].Value != "value2" || values[property][1].Weight != 0.6 { + if values[property][1].Value != "value2" || values[property][1].Weight != 0.0 { t.Errorf("Second appended value incorrect: got %v with weight %v", values[property][1].Value, values[property][1].Weight) } @@ -511,19 +474,16 @@ func TestValues_MultipleAppendsOrder(t *testing.T) { property := "order_test" // Append multiple values - testValues := []struct { - value interface{} - weight float64 - }{ - {"first", 1.0}, - {"second", 0.9}, - {"third", 0.8}, - {42, 0.7}, - {true, 0.6}, + testValues := []interface{}{ + "first", + "second", + "third", + 42, + true, } for _, tv := range testValues { - values.Append(property, tv.value, tv.weight) + values.Append(property, tv) } // Verify all values are present in correct order @@ -532,13 +492,153 @@ func TestValues_MultipleAppendsOrder(t *testing.T) { } for i, tv := range testValues { - if !reflect.DeepEqual(values[property][i].Value, tv.value) { + if !reflect.DeepEqual(values[property][i].Value, tv) { t.Errorf("MultipleAppendsOrder() value at index %d = %v, want %v", - i, values[property][i].Value, tv.value) + i, values[property][i].Value, tv) } - if values[property][i].Weight != tv.weight { - t.Errorf("MultipleAppendsOrder() weight at index %d = %v, want %v", - i, values[property][i].Weight, tv.weight) + if values[property][i].Weight != 0.0 { + t.Errorf("MultipleAppendsOrder() weight at index %d = %v, want 0.0", + i, values[property][i].Weight) } } } + +func TestValues_AppendWithWeight(t *testing.T) { + // Test AppendWithWeight specifically for MCC property + tests := []struct { + name string + initialValues Values + property string + value interface{} + weight float64 + expectedLength int + expectedValue interface{} + expectedWeight float64 + }{ + { + name: "append MCC with weight", + initialValues: make(Values), + property: "Mcc", + value: "310", + weight: 0.95, + expectedLength: 1, + expectedValue: "310", + expectedWeight: 0.95, + }, + { + name: "append MCC with weight to existing", + initialValues: Values{ + "Mcc": []*WeightedValue{ + {Value: "250", Weight: 0.8}, + }, + }, + property: "Mcc", + value: "310", + weight: 0.9, + expectedLength: 2, + expectedValue: "310", + expectedWeight: 0.9, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + values := tt.initialValues + values.AppendWithWeight(tt.property, tt.value, tt.weight) + + // Check if property exists + propertyValues, exists := values[tt.property] + if !exists { + t.Errorf("AppendWithWeight() property %s was not created", tt.property) + return + } + + // Check length + if len(propertyValues) != tt.expectedLength { + t.Errorf("AppendWithWeight() length = %d, want %d", len(propertyValues), tt.expectedLength) + } + + // Check the last added value (should be at the end) + lastIndex := len(propertyValues) - 1 + if lastIndex < 0 { + t.Errorf("AppendWithWeight() no values found in property %s", tt.property) + return + } + + lastValue := propertyValues[lastIndex] + if !reflect.DeepEqual(lastValue.Value, tt.expectedValue) { + t.Errorf("AppendWithWeight() value = %v, want %v", lastValue.Value, tt.expectedValue) + } + + if lastValue.Weight != tt.expectedWeight { + t.Errorf("AppendWithWeight() weight = %v, want %v", lastValue.Weight, tt.expectedWeight) + } + }) + } +} + +func TestValues_GetValueByProperty(t *testing.T) { + tests := []struct { + name string + values Values + property string + wantValue interface{} + wantSuccess bool + }{ + { + name: "existing property with single value", + values: Values{ + "browser": []*WeightedValue{ + {Value: "Chrome", Weight: 0.0}, + }, + }, + property: "browser", + wantValue: "Chrome", + wantSuccess: true, + }, + { + name: "existing property with multiple values", + values: Values{ + "platform": []*WeightedValue{ + {Value: "Windows", Weight: 0.0}, + {Value: "Linux", Weight: 0.0}, + }, + }, + property: "platform", + wantValue: "Windows", + wantSuccess: true, + }, + { + name: "non-existing property", + values: Values{ + "os": []*WeightedValue{ + {Value: "Android", Weight: 0.0}, + }, + }, + property: "browser", + wantValue: "", + wantSuccess: false, + }, + { + name: "empty values map", + values: Values{}, + property: "anything", + wantValue: "", + wantSuccess: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotValue, gotSuccess := tt.values.GetValueByProperty(tt.property) + + if gotSuccess != tt.wantSuccess { + t.Errorf("GetValueByProperty() success = %v, want %v", gotSuccess, tt.wantSuccess) + } + + if !reflect.DeepEqual(gotValue, tt.wantValue) { + t.Errorf("GetValueByProperty() value = %v, want %v", gotValue, tt.wantValue) + } + }) + } +}