Skip to content

Commit

Permalink
Merge pull request #381 from apache/SpotBugs_Fixes
Browse files Browse the repository at this point in the history
Fixes problems found when running latest version of SpotBugs.
  • Loading branch information
leerho committed Jan 12, 2022
2 parents 98489ec + 7066e8a commit 7027661
Show file tree
Hide file tree
Showing 20 changed files with 230 additions and 54 deletions.
20 changes: 16 additions & 4 deletions README.md
Expand Up @@ -92,12 +92,23 @@ See the pom.xml file for test dependencies.

Building and running tests using JDK 8 should not be a problem.

However, with JDK 9+, and Eclipse version up to and including 4.22.0 (2021-12), Eclipse fails to translate the required JPMS JVM arguments specified in the POM into the *.classpath* file, causing illegal reflection access errors.
However, with JDK 9+, and Eclipse versions up to and including 4.22.0 (2021-12), Eclipse fails to translate the required JPMS JVM arguments specified in the POM compiler or surefire plugins into the *.classpath* file, causing illegal reflection access errors
[eclipse-m2e/m2e-core Bug 543631](https://github.com/eclipse-m2e/m2e-core/issues/129).

There are two ways to fix this:

#### Manually update *.classpath* file:
Open the *.classpath* file in a text editor and insert the following *classpathentry* element (this assumes JDK11, change to suit) then *refresh*.:
#### Method 1: Manually update *.classpath* file:
Open the *.classpath* file in a text editor and find the following *classpathentry* element (this assumes JDK11, change to suit):

```
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-11">
<attributes>
<attribute name="module" value="true"/>
<attribute name="maven.pomderived" value="true"/>
</attributes>
</classpathentry>
```
Then edit it as follows:

```
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-11">
Expand All @@ -109,8 +120,9 @@ Open the *.classpath* file in a text editor and insert the following *classpathe
</attributes>
</classpathentry>
```
Finally, *refresh*.

#### Manually update *Module Dependencies*
#### Method 2: Manually update *Module Dependencies*

In Eclipse, open the project *Properties / Java Build Path / Module Dependencies ...*

Expand Down
5 changes: 4 additions & 1 deletion src/main/java/org/apache/datasketches/Util.java
Expand Up @@ -33,6 +33,7 @@
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Objects;

/**
* Common utility functions.
Expand Down Expand Up @@ -846,13 +847,15 @@ public static boolean isLessThanUnsigned(final long n1, final long n2) {
* @return the absolute path of the given resource file's shortName.
*/
public static String getResourcePath(final String shortFileName) {
Objects.requireNonNull(shortFileName, "input parameter " + shortFileName + " cannot be null.");
try {
final URL url = Util.class.getClassLoader().getResource(shortFileName);
Objects.requireNonNull(url, "resource " + shortFileName + " could not be acquired.");
final URI uri = url.toURI();
//decodes any special characters
final String path = uri.isAbsolute() ? Paths.get(uri).toAbsolutePath().toString() : uri.getPath();
return path;
} catch (final NullPointerException | URISyntaxException e) {
} catch (final URISyntaxException e) {
throw new SketchesArgumentException("Cannot find resource: " + shortFileName + LS + e);
}
}
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/org/apache/datasketches/fdt/FdtSketch.java
Expand Up @@ -82,6 +82,23 @@ public FdtSketch(final double threshold, final double rse) {
super(computeLgK(threshold, rse));
}

/**
* Copy Constructor
* @param sketch the sketch to copy
*/
public FdtSketch(final FdtSketch sketch) {
super(sketch);
}

/**
* @return a deep copy of this sketch
*/
@SuppressWarnings("unchecked")
@Override
public FdtSketch copy() {
return new FdtSketch(this);
}

/**
* Update the sketch with the given string array tuple.
* @param tuple the given string array tuple.
Expand Down
Expand Up @@ -58,7 +58,7 @@ public class PostProcessor {
* @param sep the separator character
*/
public PostProcessor(final FdtSketch sketch, final Group group, final char sep) {
this.sketch = sketch;
this.sketch = sketch.copy();
this.sep = sep;
final int numEntries = sketch.getRetainedEntries();
mapArrSize = ceilingPowerOf2((int)(numEntries / 0.75));
Expand Down
Expand Up @@ -42,6 +42,7 @@
import static org.apache.datasketches.quantiles.Util.computeBitPattern;

import org.apache.datasketches.Family;
import org.apache.datasketches.SketchesArgumentException;
import org.apache.datasketches.memory.MemoryRequestServer;
import org.apache.datasketches.memory.WritableMemory;

Expand Down Expand Up @@ -250,6 +251,11 @@ private WritableMemory growCombinedMemBuffer(final int itemSpaceNeeded) {
assert needBytes > memBytes;

memReqSvr = (memReqSvr == null) ? mem_.getMemoryRequestServer() : memReqSvr;
if (memReqSvr == null) {
throw new SketchesArgumentException(
"A request for more memory has been denied, "
+ "or a default MemoryRequestServer has not been provided. Must abort. ");
}

final WritableMemory newMem = memReqSvr.request(mem_, needBytes);

Expand Down
4 changes: 1 addition & 3 deletions src/main/java/org/apache/datasketches/theta/AnotBimpl.java
Expand Up @@ -78,7 +78,6 @@ public void setA(final Sketch skA) {

//process A
hashArr_ = getHashArrA(skA);
hashArr_ = (hashArr_ == null) ? new long[0] : hashArr_;
empty_ = false;
thetaLong_ = skA.getThetaLong();
curCount_ = hashArr_.length;
Expand All @@ -94,7 +93,6 @@ public void notB(final Sketch skB) {

//process B
hashArr_ = getResultHashArr(thetaLong_, curCount_, hashArr_, skB);
hashArr_ = (hashArr_ == null) ? new long[0] : hashArr_;
curCount_ = hashArr_.length;
empty_ = curCount_ == 0 && thetaLong_ == Long.MAX_VALUE;
}
Expand Down Expand Up @@ -135,7 +133,7 @@ public CompactSketch aNotB(final Sketch skA, final Sketch skB, final boolean dst

//process A
final long[] hashArrA = getHashArrA(skA);
final int countA = (hashArrA == null) ? 0 : hashArrA.length;
final int countA = hashArrA.length;


//process B
Expand Down
25 changes: 25 additions & 0 deletions src/main/java/org/apache/datasketches/tuple/QuickSelectSketch.java
Expand Up @@ -134,6 +134,24 @@ private enum Flags { IS_BIG_ENDIAN, IS_IN_SAMPLING_MODE, IS_EMPTY, HAS_ENTRIES,
setRebuildThreshold();
}

/**
* Copy constructor
* @param sketch the QuickSelectSketch to be copied.
*/
QuickSelectSketch(final QuickSelectSketch<S> sketch) {
nomEntries_ = sketch.nomEntries_;
lgCurrentCapacity_ = sketch.lgCurrentCapacity_;
lgResizeFactor_ = sketch.lgResizeFactor_;
count_ = sketch.count_;
samplingProbability_ = sketch.samplingProbability_;
rebuildThreshold_ = sketch.rebuildThreshold_;
thetaLong_ = sketch.thetaLong_;
empty_ = sketch.empty_;
summaryFactory_ = sketch.summaryFactory_;
hashTable_ = sketch.hashTable_.clone();
summaryTable_ = Util.copySummaryArray(sketch.summaryTable_);
}

/**
* This is to create an instance of a QuickSelectSketch given a serialized form
* @param mem Memory object with serialized QukckSelectSketch
Expand Down Expand Up @@ -205,6 +223,13 @@ private enum Flags { IS_BIG_ENDIAN, IS_IN_SAMPLING_MODE, IS_EMPTY, HAS_ENTRIES,
setRebuildThreshold();
}

/**
* @return a deep copy of this sketch
*/
QuickSelectSketch<S> copy() {
return new QuickSelectSketch<S>(this);
}

long[] getHashTable() {
return hashTable_;
}
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/org/apache/datasketches/tuple/UpdatableSketch.java
Expand Up @@ -75,6 +75,23 @@ public UpdatableSketch(final Memory srcMem, final SummaryDeserializer<S> deseria
super(srcMem, deserializer, summaryFactory);
}

/**
* Copy Constructor
* @param sketch the sketch to copy
*/
public UpdatableSketch(final UpdatableSketch<U, S> sketch) {
super(sketch);
}

/**
* @return a deep copy of this sketch
*/
@SuppressWarnings("unchecked")
@Override
public UpdatableSketch<U,S> copy() {
return new UpdatableSketch<U,S>(this);
}

/**
* Updates this sketch with a long key and U value.
* The value is passed to update() method of the Summary object associated with the key
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/org/apache/datasketches/tuple/Util.java
Expand Up @@ -137,12 +137,20 @@ public static long stringArrHash(final String[] strArray) {
return hashCharArr(s.toCharArray(), 0, s.length(), PRIME);
}

/**
* Will copy compact summary arrays as well as hashed summary tables (with nulls).
* @param <S> type of summary
* @param summaryArr the given summary array or table
* @return the copy
*/
@SuppressWarnings("unchecked")
public static <S extends Summary> S[] copySummaryArray(final S[] summaryArr) {
final int len = summaryArr.length;
final S[] tmpSummaryArr = newSummaryArray(summaryArr, len);
for (int i = 0; i < len; i++) {
tmpSummaryArr[i] = (S) summaryArr[i].copy();
final S summary = summaryArr[i];
if (summary == null) { continue; }
tmpSummaryArr[i] = (S) summary.copy();
}
return tmpSummaryArr;
}
Expand Down
Expand Up @@ -73,6 +73,23 @@ public ArrayOfStringsSketch(final Memory mem) {
super(mem, new ArrayOfStringsSummaryDeserializer(), new ArrayOfStringsSummaryFactory());
}

/**
* Copy Constructor
* @param sketch the sketch to copy
*/
public ArrayOfStringsSketch(final ArrayOfStringsSketch sketch) {
super(sketch);
}

/**
* @return a deep copy of this sketch
*/
@SuppressWarnings("unchecked")
@Override
public ArrayOfStringsSketch copy() {
return new ArrayOfStringsSketch(this);
}

/**
* Updates the sketch with String arrays for both key and value.
* @param strArrKey the given String array key
Expand Down
6 changes: 3 additions & 3 deletions src/test/java/org/apache/datasketches/UtilTest.java
Expand Up @@ -89,7 +89,7 @@ public void numTrailingOnes() {
public void checkBoundsTest() {
Util.checkBounds(999, 2, 1000);
}

@Test
public void checkIsPowerOf2() {
Assert.assertEquals(isPowerOf2(0), false);
Expand Down Expand Up @@ -396,7 +396,7 @@ public void resourcefileExists() {
assertTrue(file.exists());
}

@Test(expectedExceptions = SketchesArgumentException.class)
@Test(expectedExceptions = NullPointerException.class)
public void resourceFileNotFound() {
final String shortFileName = "cpc-empty.sk";
getResourceFile(shortFileName + "123");
Expand All @@ -409,7 +409,7 @@ public void resourceBytesCorrect() {
assertTrue(bytes.length == 8);
}

@Test(expectedExceptions = SketchesArgumentException.class)
@Test(expectedExceptions = NullPointerException.class)
public void resourceBytesFileNotFound() {
final String shortFileName = "cpc-empty.sk";
getResourceBytes(shortFileName + "123");
Expand Down
11 changes: 11 additions & 0 deletions src/test/java/org/apache/datasketches/fdt/FdtSketchTest.java
Expand Up @@ -151,6 +151,17 @@ public void checkEstimatingPostProcessing() {
}
}

@Test
public void checkCopyCtor() {
final int lgK = 14;
final FdtSketch sk = new FdtSketch(lgK);

final String[] nodesArr = {"abc", "def" };
sk.update(nodesArr);
assertEquals(sk.getRetainedEntries(), 1);
final FdtSketch sk2 = sk.copy();
assertEquals(sk2.getRetainedEntries(), 1);
}

@Test
public void printlnTest() {
Expand Down
Expand Up @@ -152,15 +152,17 @@ public void directSketchShouldMoveOntoHeapEventually() {

@Test
public void directSketchShouldMoveOntoHeapEventually2() {
try (WritableHandle wdh = WritableMemory.allocateDirect(50)) {
int i = 0;
try (WritableHandle wdh =
WritableMemory.allocateDirect(50, ByteOrder.LITTLE_ENDIAN, new DefaultMemoryRequestServer())) {
WritableMemory mem = wdh.getWritable();
UpdateDoublesSketch sketch = DoublesSketch.builder().build(mem);
Assert.assertTrue(sketch.isSameResource(mem));
for (int i = 0; i < 1000; i++) {
try {
for (; i < 1000; i++) {
if (sketch.isSameResource(mem)) {
sketch.update(i);
if (sketch.isSameResource(mem)) { continue; }
} catch (NullPointerException e) {
} else {
System.out.println("MOVED at i = " + i);
break;
}
}
Expand Down
Expand Up @@ -515,17 +515,9 @@ public void checkReset() {
for (int i = 0; i < (2 * k); ++i) {
sketch.update("a", 100.0 + i);
}

assertEquals(sketch.getN(), 2 * k);
assertEquals(sketch.getHRegionCount(), 0);
assertEquals(sketch.getRRegionCount(), k);
try {
sketch.getMark(0);
fail();
} catch (final NullPointerException e) {
// expected
}

sketch.reset();
assertEquals(sketch.getN(), 0);
assertEquals(sketch.getHRegionCount(), 0);
Expand Down
Expand Up @@ -115,7 +115,7 @@ public void emptyDegenerate() {
}

@Test
public void EmptyEstimation() {
public void emptyEstimation() {
UpdateSketch thetaA = getSketch(SkType.EMPTY, 0, 0);
UpdateSketch thetaB = getSketch(SkType.ESTIMATION, LOWP, LT_LOWP_V);
final double expectedIntersectTheta = 1.0;
Expand Down Expand Up @@ -301,7 +301,7 @@ public void estimationEstimation() {
//=================================

@Test
public void DegenerateEmpty() {
public void degenerateEmpty() {
UpdateSketch thetaA = getSketch(SkType.DEGENERATE, LOWP, GT_LOWP_V); //entries = 0
UpdateSketch thetaB = getSketch(SkType.EMPTY, 0, 0);
final double expectedIntersectTheta = 1.0;
Expand All @@ -321,7 +321,7 @@ public void DegenerateEmpty() {
}

@Test
public void DegenerateExact() {
public void degenerateExact() {
UpdateSketch thetaA = getSketch(SkType.DEGENERATE, LOWP, GT_LOWP_V); //entries = 0
UpdateSketch thetaB = getSketch(SkType.EXACT, 0, LT_LOWP_V);
final double expectedIntersectTheta = LOWP_THETA;
Expand All @@ -341,7 +341,7 @@ public void DegenerateExact() {
}

@Test
public void DegenerateDegenerate() {
public void degenerateDegenerate() {
UpdateSketch thetaA = getSketch(SkType.DEGENERATE, MIDP, GT_MIDP_V); //entries = 0
UpdateSketch thetaB = getSketch(SkType.DEGENERATE, LOWP, GT_LOWP_V);
final double expectedIntersectTheta = LOWP_THETA;
Expand All @@ -361,7 +361,7 @@ public void DegenerateDegenerate() {
}

@Test
public void DegenerateEstimation() {
public void degenerateEstimation() {
UpdateSketch thetaA = getSketch(SkType.DEGENERATE, MIDP, GT_MIDP_V); //entries = 0
UpdateSketch thetaB = getSketch(SkType.ESTIMATION, LOWP, LT_LOWP_V);
final double expectedIntersectTheta = LOWP_THETA;
Expand Down
Expand Up @@ -378,7 +378,7 @@ public void checkWrapCompactSketchGivenDefaultSeed() {
assertEquals(sv3cskResult.getEstimate(), sv3usk.getEstimate());
assertEquals(sv3cskResult.getSeedHash(), seedHash);
assertFalse(sv3cskResult.isDirect());
try { wh.close(); } catch (Exception e) {}
try { wh.close(); } catch (Exception e) {/* ignore */}
}

@Test
Expand Down

0 comments on commit 7027661

Please sign in to comment.