Skip to content

Unset active objective when objectives are removed and add test (#250)#284

Open
dyrpsf wants to merge 1 commit intosbmlteam:masterfrom
dyrpsf:fix-fbc-active-objective-removal
Open

Unset active objective when objectives are removed and add test (#250)#284
dyrpsf wants to merge 1 commit intosbmlteam:masterfrom
dyrpsf:fix-fbc-active-objective-removal

Conversation

@dyrpsf
Copy link
Contributor

@dyrpsf dyrpsf commented Feb 16, 2026

This PR addresses #250.

Problem

When objectives are removed from a model using the FBC extension, the
activeObjective attribute is not updated. In particular, the following pattern
(from the issue) leaves activeObjective set even though no objectives remain:

Model m = new Model(2, 4);
FBCModelPlugin fbcPlugin = (FBCModelPlugin) m.getPlugin(FBCConstants.shortLabel);

Objective o1 = fbcPlugin.createObjective("obj1");
fbcPlugin.setActiveObjective(o1);

fbcPlugin.getListOfObjectives().removeAll(fbcPlugin.getListOfObjectives());

// Previously:
//   fbcPlugin.isSetActiveObjective() == true
//   fbcPlugin.getActiveObjective() == "obj1"
//   fbcPlugin.getObjectiveCount() == 0

This leaves the model in an inconsistent state: the active objective refers
to an objective that no longer exists in the list.

Fix
The logic for the active objective is encapsulated in:
extensions/fbc/src/org/sbml/jsbml/ext/fbc/ListOfObjectives.java.

This PR updates ListOfObjectives to ensure that the activeObjective always
refers to an objective that is still present in the list:

  • Added a helper method:
private void ensureActiveObjectiveStillPresent() {
  if (isSetActiveObjective()) {
    if (firstHit(new NameFilter(activeObjective)) == null) {
      unsetActiveObjective();
    }
  }
}
  • Overrode the list-modifying methods to call this helper whenever the list
    changes:
@Override
public boolean remove(Object o) { ... }

@Override
public Objective remove(int index) { ... }

@Override
public boolean removeAll(Collection<?> c) { ... }

@Override
public boolean retainAll(Collection<?> c) { ... }

@Override
public void clear() { ... }

If, after a modification, there is no Objective with the id stored in
activeObjective, the active objective is unset. For valid cases where the
active objective is still present, behaviour is unchanged.

New test
Added a regression test:

  • extensions/fbc/test/org/sbml/jsbml/ext/fbc/test/ActiveObjectiveRemovalTest.java
@Test
public void removingAllObjectivesUnsetsActiveObjective() {
  Model m = new Model(2, 4);
  FBCModelPlugin fbcPlugin = (FBCModelPlugin) m.getPlugin(FBCConstants.shortLabel);

  Objective o1 = fbcPlugin.createObjective("obj1");
  fbcPlugin.setActiveObjective(o1);

  // Sanity check before removal
  assertTrue(fbcPlugin.isSetActiveObjective());
  assertEquals("obj1", fbcPlugin.getActiveObjective());
  assertEquals(1, fbcPlugin.getObjectiveCount());

  // Remove all objectives (as in the issue)
  fbcPlugin.getListOfObjectives().removeAll(fbcPlugin.getListOfObjectives());

  // After removal, the active objective should be unset
  assertFalse(fbcPlugin.isSetActiveObjective());
  assertEquals("", fbcPlugin.getActiveObjective());
  assertEquals(0, fbcPlugin.getObjectiveCount());
}

This directly captures the scenario from issue #250.

Tests
Executed locally:

  • mvn -pl :jsbml-fbc test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant