Apache
/
poi
mirror of https://github.com/apache/poi.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix a case in IdentifierManager.release(), reformat, add tests 

The case when all are reserved was not handled properly

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1909807 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Dominik Stadler 2023-05-14 07:47:12 +00:00
parent ddafec4608
commit a647cc97e7
2 changed files with 77 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
poi-ooxml/src/main/java/org/apache/poi/ooxml/util/IdentifierManager.java
View File

@ -39,17 +39,15 @@ public class IdentifierManager {
public IdentifierManager(long lowerbound, long upperbound) {
if (lowerbound > upperbound) {
throw new IllegalArgumentException("lowerbound must not be greater than upperbound, had " + lowerbound + " and " + upperbound);
}
else if (lowerbound < MIN_ID) {
String message = "lowerbound must be greater than or equal to " + Long.toString(MIN_ID);
} else if (lowerbound < MIN_ID) {
String message = "lowerbound must be greater than or equal to " + MIN_ID;
throw new IllegalArgumentException(message);
}
else if (upperbound > MAX_ID) {
} else if (upperbound > MAX_ID) {
/*
* while MAX_ID is Long.MAX_VALUE, this check is pointless. But if
* someone subclasses / tweaks the limits, this check is fine.
*/
throw new IllegalArgumentException("upperbound must be less than or equal to " + Long.toString(MAX_ID) + " but had " + upperbound);
throw new IllegalArgumentException("upperbound must be less than or equal to " + MAX_ID + " but had " + upperbound);
}
this.lowerbound = lowerbound;
this.upperbound = upperbound;
@ -92,25 +90,21 @@ public class IdentifierManager {
Segment segment = iter.next();
if (segment.end < id) {
continue;
}
else if (segment.start > id) {
} else if (segment.start > id) {
break;
}
else if (segment.start == id) {
} else if (segment.start == id) {
segment.start = id + 1;
if (segment.end < segment.start) {
iter.remove();
}
return id;
}
else if (segment.end == id) {
} else if (segment.end == id) {
segment.end = id - 1;
if (segment.start > segment.end) {
iter.remove();
}
return id;
}
else {
} else {
iter.add(new Segment(id + 1, segment.end));
segment.end = id - 1;
return id;
@ -159,6 +153,13 @@ public class IdentifierManager {
}
}
// if there are no segments then all are reserved currently
// and so we need to mark this id as released now
if (segments.isEmpty()) {
segments.add(new Segment(id, id));
return true;
}
if (id == lowerbound) {
Segment firstSegment = segments.getFirst();
if (firstSegment.start == lowerbound + 1) {
@ -189,8 +190,7 @@ public class IdentifierManager {
if (segment.start == higher) {
segment.start = id;
return true;
}
else if (segment.end == lower) {
} else if (segment.end == lower) {
segment.end = id;
/* check if releasing this elements glues two segments into one */
if (iter.hasNext()) {
@ -201,8 +201,7 @@ public class IdentifierManager {
}
}
return true;
}
else {
} else {
/* id was not reserved, return false */
break;
}
@ -224,7 +223,8 @@ public class IdentifierManager {
*/
private void verifyIdentifiersLeft() {
if (segments.isEmpty()) {
throw new IllegalStateException("No identifiers left");
throw new IllegalStateException("No identifiers left for range "
+ "[" + lowerbound + "," + upperbound + "]");
}
}

60
poi-ooxml/src/test/java/org/apache/poi/util/tests/TestIdentifierManager.java
View File

@ -27,8 +27,7 @@ import org.junit.jupiter.api.Test;
class TestIdentifierManager {
@Test
void testBasic()
{
void testBasic() {
IdentifierManager manager = new IdentifierManager(0L,100L);
assertEquals(101L,manager.getRemainingIdentifiers());
assertEquals(0L,manager.reserveNew());
@ -42,6 +41,7 @@ class TestIdentifierManager {
long min = IdentifierManager.MIN_ID;
long max = IdentifierManager.MAX_ID;
IdentifierManager manager = new IdentifierManager(min,max);
//noinspection ConstantValue
assertTrue(max - min + 1 > 0, "Limits lead to a long variable overflow");
assertTrue(manager.getRemainingIdentifiers() > 0, "Limits lead to a long variable overflow");
assertEquals(min,manager.reserveNew());
@ -103,4 +103,60 @@ class TestIdentifierManager {
assertEquals(11L,manager.reserve(11L));
assertTrue(manager.release(12L));
}
@Test
void testBounds() {
assertThrows(IllegalArgumentException.class,
() -> new IdentifierManager(1, 0),
"Lower bound higher than upper");
assertThrows(IllegalArgumentException.class,
() -> new IdentifierManager(-1, 0),
"Out of lower bound");
assertThrows(IllegalArgumentException.class,
() -> new IdentifierManager(0, Long.MAX_VALUE),
"Out of upper bound");
IdentifierManager manager = new IdentifierManager(10, 100);
assertThrows(IllegalArgumentException.class,
() -> manager.reserve(9),
"Reserve out of lower bound");
assertThrows(IllegalArgumentException.class,
() -> manager.reserve(101),
"Reserve out of upper bound");
// normal reserving works
assertEquals(10, manager.reserve(10));
assertEquals(100, manager.reserve(100));
assertEquals(11, manager.reserve(10));
assertEquals(12, manager.reserve(100));
for (int i = 13; i < 100; i++) {
assertEquals(i, manager.reserve(10));
assertEquals(100 - i - 1,
manager.getRemainingIdentifiers());
}
// now the manager is exhausted
assertThrows(IllegalStateException.class,
() -> manager.reserve(10),
"No more ids left any more");
assertThrows(IllegalArgumentException.class,
() -> manager.release(9),
"Release out of lower bound");
assertThrows(IllegalArgumentException.class,
() -> manager.release(101),
"Release out of upper bound");
for (int i = 10; i < 100; i++) {
assertTrue(manager.release(i));
assertEquals(i - 10 + 1,
manager.getRemainingIdentifiers());
}
assertEquals(100 - 10, manager.getRemainingIdentifiers());
}
}