Throw GetAttributeNotAllowed in AbstractBridge getter (#2988)

Author: blegat

src/Bridges/Bridges.jl

@@ -239,6 +239,7 @@ _runtests_error_handler(err, ::Bool) = rethrow(err)
 function _runtests_error_handler(
     err::Union{
         MOI.GetAttributeNotAllowed{MOI.ConstraintFunction},
+        MOI.GetAttributeNotAllowed{MOI.ConstraintSet},
         MOI.GetAttributeNotAllowed{MOI.ConstraintPrimalStart},
     },
     cannot_unbridge::Bool,

src/Bridges/bridge.jl

@@ -146,7 +146,7 @@ function MOI.get(
     bridge::AbstractBridge,
 )
     message = _attribute_error_message(attr, typeof(bridge), "accessing")
-    return throw(ArgumentError(message))
+    return throw(MOI.GetAttributeNotAllowed(attr, message))
 end
 
 function MOI.get(

test/Bridges/General/test_abstract_bridge.jl

@@ -0,0 +1,115 @@
+# Copyright (c) 2017: Miles Lubin and contributors
+# Copyright (c) 2017: Google Inc.
+#
+# Use of this source code is governed by an MIT-style license that can be found
+# in the LICENSE.md file or at https://opensource.org/licenses/MIT.
+
+# Test with a bridge for which the map is defined on the bridge value and not
+# the bridge type
+module TestAbstractBridge
+
+using Test
+
+import MathOptInterface as MOI
+
+# A minimal constraint bridge that doesn't implement ConstraintFunction getter.
+# Used to test that `cannot_unbridge = true` works with the default bridge
+# `MOI.get` (which throws `GetAttributeNotAllowed`).
+struct _NoUnbridgeTestBridge{T} <: MOI.Bridges.Constraint.AbstractBridge
+    ci::MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.EqualTo{T}}
+end
+
+function MOI.Bridges.Constraint.bridge_constraint(
+    ::Type{_NoUnbridgeTestBridge{T}},
+    model::MOI.ModelLike,
+    f::MOI.ScalarAffineFunction{T},
+    ::MOI.GreaterThan{T},
+) where {T}
+    ci = MOI.add_constraint(model, f, MOI.EqualTo(zero(T)))
+    return _NoUnbridgeTestBridge{T}(ci)
+end
+
+function MOI.supports_constraint(
+    ::Type{_NoUnbridgeTestBridge{T}},
+    ::Type{MOI.ScalarAffineFunction{T}},
+    ::Type{MOI.GreaterThan{T}},
+) where {T}
+    return true
+end
+
+function MOI.Bridges.Constraint.concrete_bridge_type(
+    ::Type{_NoUnbridgeTestBridge{T}},
+    ::Type{MOI.ScalarAffineFunction{T}},
+    ::Type{MOI.GreaterThan{T}},
+) where {T}
+    return _NoUnbridgeTestBridge{T}
+end
+
+function MOI.Bridges.added_constraint_types(
+    ::Type{_NoUnbridgeTestBridge{T}},
+) where {T}
+    return Tuple{Type,Type}[(MOI.ScalarAffineFunction{T}, MOI.EqualTo{T})]
+end
+
+function MOI.Bridges.added_constrained_variable_types(
+    ::Type{<:_NoUnbridgeTestBridge},
+)
+    return Tuple{Type}[]
+end
+
+function MOI.get(
+    ::_NoUnbridgeTestBridge{T},
+    ::MOI.NumberOfConstraints{MOI.ScalarAffineFunction{T},MOI.EqualTo{T}},
+) where {T}
+    return 1
+end
+
+function MOI.get(
+    bridge::_NoUnbridgeTestBridge{T},
+    ::MOI.ListOfConstraintIndices{MOI.ScalarAffineFunction{T},MOI.EqualTo{T}},
+) where {T}
+    return [bridge.ci]
+end
+
+function MOI.delete(model::MOI.ModelLike, bridge::_NoUnbridgeTestBridge)
+    MOI.delete(model, bridge.ci)
+    return
+end
+
+# Note: ConstraintFunction and ConstraintSet getters are NOT implemented.
+# The default MOI.get on AbstractBridge throws GetAttributeNotAllowed.
+
+function test_cannot_unbridge_with_default_getter()
+    # This test verifies that `cannot_unbridge = true` works when a constraint
+    # bridge does not implement `ConstraintFunction` getter. Before the fix,
+    # the default bridge `MOI.get` threw `ArgumentError` which was not caught
+    # by `_runtests_error_handler`.
+    MOI.Bridges.runtests(
+        _NoUnbridgeTestBridge,
+        model -> begin
+            x = MOI.add_variable(model)
+            MOI.add_constraint(model, 1.0 * x, MOI.GreaterThan(0.0))
+        end,
+        model -> begin
+            x = MOI.add_variable(model)
+            MOI.add_constraint(model, 1.0 * x, MOI.EqualTo(0.0))
+        end;
+        cannot_unbridge = true,
+    )
+    return
+end
+
+function runtests()
+    for name in names(@__MODULE__; all = true)
+        if startswith("$(name)", "test_")
+            @testset "$(name)" begin
+                getfield(@__MODULE__, name)()
+            end
+        end
+    end
+    return
+end
+
+end
+
+TestAbstractBridge.runtests()

test/Bridges/General/test_bridge_optimizer.jl

@@ -250,7 +250,7 @@ function test_unsupported_constraint_attribute()
     @test !MOI.Bridges.is_bridged(bridged_mock, ci)
     @test MOI.Bridges.is_bridged(bridged_mock.model, ci)
     @test !MOI.supports(bridged_mock, attr, typeof(ci))
-    err = ArgumentError(message("accessing"))
+    err = MOI.GetAttributeNotAllowed(attr, message("accessing"))
     @test_throws err MOI.get(bridged_mock, attr, ci)
     err = MOI.UnsupportedAttribute(attr, message("setting a value for"))
     @test_throws err MOI.set(bridged_mock, attr, ci, 1)

test/Bridges/Variable/test_zeros.jl

@@ -51,6 +51,7 @@ function test_bridge_error_handler()
         ErrorException("abc") => false,
         MOI.GetAttributeNotAllowed(MOI.ObjectiveSense()) => false,
         MOI.GetAttributeNotAllowed(MOI.ConstraintFunction()) => true,
+        MOI.GetAttributeNotAllowed(MOI.ConstraintSet()) => true,
     )
         @test_throws err try
             @assert false