Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Commit

Permalink
PAN-2592: Rename methods that create and return streams away from get…
Browse files Browse the repository at this point in the history
…X() (#1368)

* Change all Stream<?> getX() and Stream<?> x() methods to Stream<?> streanX methods, such as `Stream<Peer> streamIdlePeers()`
* Update coding conventions to reflect this.
  • Loading branch information
Danno Ferrin authored May 8, 2019
1 parent 30833fb commit e115ea3
Show file tree
Hide file tree
Showing 41 changed files with 202 additions and 181 deletions.
4 changes: 3 additions & 1 deletion CODING-CONVENTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,9 @@ Method parameters must be final. Class level and local fields should be final w

* Getters follow idiomatic format with `get` prefix. For example, `getBlock()` gets a block property.
* Setters follow idiomatic format with `set` prefix. For example, `setBlock(Block block)` sets a block property.
* For `toString methods`, use the Guava 18+ `MoreObjects.toStringHelper`
* The Setter pattern should not be used for chained builder methods.
* Methods returning a `Stream` should be prefixed with `stream`. For example `streamIdlePeers()` returns a stream of the idle peers.
* For `toString` methods use the Guava 18+ `MoreObjects.toStringHelper`
* Equals and `hashCode()` methods use the `Object.equals` and `Object.hash` methods (this is the _Java 7+_ template in IntelliJ. Don’t accept subclasses and don’t use getters)

## 4.2.4 Testing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public GenesisConfigOptions getConfigOptions() {
return new JsonGenesisConfigOptions(configRoot.getJsonObject("config"));
}

public Stream<GenesisAllocation> getAllocations() {
public Stream<GenesisAllocation> streamAllocations() {
final JsonObject allocations = configRoot.getJsonObject("alloc", new JsonObject());
return allocations.fieldNames().stream()
.map(key -> new GenesisAllocation(key, allocations.getJsonObject(key)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void shouldLoadMainnetConfigFile() {
// Sanity check some basic properties to confirm this is the mainnet file.
assertThat(config.getConfigOptions().isEthHash()).isTrue();
assertThat(config.getConfigOptions().getChainId()).hasValue(MAINNET_CHAIN_ID);
assertThat(config.getAllocations().map(GenesisAllocation::getAddress))
assertThat(config.streamAllocations().map(GenesisAllocation::getAddress))
.contains(
"000d836201318ec6899a67540690382780743280",
"001762430ea9c3a26e5749afdb70da5f78ddbb8c",
Expand All @@ -48,7 +48,7 @@ public void shouldLoadDevelopmentConfigFile() {
// Sanity check some basic properties to confirm this is the dev file.
assertThat(config.getConfigOptions().isEthHash()).isTrue();
assertThat(config.getConfigOptions().getChainId()).hasValue(DEVELOPMENT_CHAIN_ID);
assertThat(config.getAllocations().map(GenesisAllocation::getAddress))
assertThat(config.streamAllocations().map(GenesisAllocation::getAddress))
.contains(
"fe3b557e8fb62b89f4916b721be55ceb828dbd73",
"627306090abab3a6e1400e9345bc60c78a8bef57",
Expand Down Expand Up @@ -155,7 +155,7 @@ public void shouldGetAllocations() {

final Map<String, String> allocations =
config
.getAllocations()
.streamAllocations()
.collect(
Collectors.toMap(GenesisAllocation::getAddress, GenesisAllocation::getBalance));
assertThat(allocations)
Expand All @@ -168,7 +168,7 @@ public void shouldGetAllocations() {
@Test
public void shouldGetEmptyAllocationsWhenAllocNotPresent() {
final GenesisConfigFile config = GenesisConfigFile.fromConfig("{}");
assertThat(config.getAllocations()).isEmpty();
assertThat(config.streamAllocations()).isEmpty();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ private static Hash parseMixHash(final GenesisConfigFile genesis) {
}

private static Stream<GenesisAccount> parseAllocations(final GenesisConfigFile genesis) {
return genesis.getAllocations().map(GenesisAccount::fromAllocation);
return genesis.streamAllocations().map(GenesisAccount::fromAllocation);
}

private static long parseNonce(final GenesisConfigFile genesis) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,5 @@ public interface WorldState extends WorldView {
* @return a stream of all the accounts (in no particular order) contained in the world state
* represented by the root hash of this object at the time of the call.
*/
Stream<Account> accounts();
Stream<Account> streamAccounts();
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ public WorldUpdater updater() {
}

@Override
public Stream<Account> accounts() {
public Stream<Account> streamAccounts() {
return info.accounts.stream().map(this::get).filter(Objects::nonNull);
}

@Override
public String toString() {
final StringBuilder builder = new StringBuilder();
builder.append(rootHash()).append(":\n");
accounts()
streamAccounts()
.forEach(
account -> {
final Address address = account.getAddress();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,13 @@ public Account get(final Address address) {
private AccountState deserializeAccount(
final Address address, final Hash addressHash, final BytesValue encoded) throws RLPException {
final RLPInput in = RLP.input(encoded);
StateTrieAccountValue accountValue = StateTrieAccountValue.readFrom(in);
final StateTrieAccountValue accountValue = StateTrieAccountValue.readFrom(in);
return new AccountState(address, addressHash, accountValue);
}

private static BytesValue serializeAccount(
final long nonce, final Wei balance, final Hash storageRoot, final Hash codeHash) {
StateTrieAccountValue accountValue =
final StateTrieAccountValue accountValue =
new StateTrieAccountValue(nonce, balance, storageRoot, codeHash);
return RLP.encode(accountValue::writeTo);
}
Expand All @@ -120,7 +120,7 @@ public WorldUpdater updater() {
}

@Override
public Stream<Account> accounts() {
public Stream<Account> streamAccounts() {
// TODO: the current trie implementation doesn't have walking capability yet (pending NC-746)
// so this can't be implemented.
throw new UnsupportedOperationException("TODO");
Expand Down Expand Up @@ -226,7 +226,7 @@ public BytesValue getCode() {
return updatedCode;
}
// No code is common, save the KV-store lookup.
Hash codeHash = getCodeHash();
final Hash codeHash = getCodeHash();
if (codeHash.equals(Hash.EMPTY)) {
return BytesValue.EMPTY;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,12 @@ public int peerCount() {
return connections.size();
}

public Stream<EthPeer> availablePeers() {
public Stream<EthPeer> streamAvailablePeers() {
return connections.values().stream().filter(EthPeer::readyForRequests);
}

public Optional<EthPeer> bestPeer() {
return availablePeers().max(BEST_CHAIN);
return streamAvailablePeers().max(BEST_CHAIN);
}

@FunctionalInterface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ private Optional<EthPeer> getLeastBusySuitablePeer() {
return peer.isPresent()
? peer
: ethPeers
.availablePeers()
.streamAvailablePeers()
.filter(peer -> peer.chainState().getEstimatedHeight() >= minimumBlockNumber)
.min(EthPeers.LEAST_TO_MOST_BUSY);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ public void propagate(final Block block, final UInt256 totalDifficulty) {
final NewBlockMessage newBlockMessage = NewBlockMessage.create(block, totalDifficulty);
ethContext
.getEthPeers()
.availablePeers()
.streamAvailablePeers()
.filter(ethPeer -> !ethPeer.hasSeenBlock(block.getHash()))
.forEach(
ethPeer -> {
ethPeer.registerKnownBlock(block.getHash());
try {
ethPeer.send(newBlockMessage);
} catch (PeerConnection.PeerNotConnected e) {
} catch (final PeerConnection.PeerNotConnected e) {
LOG.trace("Failed to broadcast new block to peer", e);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public void enforceTrailingPeerLimit() {
final long maxTrailingPeers = requirements.getMaxTrailingPeers();
final List<EthPeer> trailingPeers =
ethPeers
.availablePeers()
.streamAvailablePeers()
.filter(peer -> peer.chainState().hasEstimatedHeight())
.filter(peer -> peer.chainState().getEstimatedHeight() < minimumHeightToBeUpToDate)
.sorted(BY_CHAIN_HEIGHT)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private CompletableFuture[] requestHeaderFromAllPeers() {
final List<EthPeer> peersToQuery =
ethContext
.getEthPeers()
.availablePeers()
.streamAvailablePeers()
.filter(peer -> peer.chainState().getEstimatedHeight() >= pivotBlockNumber)
.collect(Collectors.toList());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public TransactionSender(
public void onTransactionsAdded(final Iterable<Transaction> transactions) {
ethContext
.getEthPeers()
.availablePeers()
.streamAvailablePeers()
.forEach(
peer ->
transactions.forEach(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ public Optional<MessageData> peekNextOutgoingRequest() {
return Optional.of(outgoingMessages.peek().messageData);
}

public Stream<MessageData> pendingOutgoingRequests() {
public Stream<MessageData> streamPendingOutgoingRequests() {
return outgoingMessages.stream().map(OutgoingMessage::messageData);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class BlockBroadcasterTest {
public void blockPropagationUnitTest() throws PeerConnection.PeerNotConnected {
final EthPeer ethPeer = mock(EthPeer.class);
final EthPeers ethPeers = mock(EthPeers.class);
when(ethPeers.availablePeers()).thenReturn(Stream.of(ethPeer));
when(ethPeers.streamAvailablePeers()).thenReturn(Stream.of(ethPeer));

final EthContext ethContext = mock(EthContext.class);
when(ethContext.getEthPeers()).thenReturn(ethPeers);
Expand All @@ -63,7 +63,7 @@ public void blockPropagationUnitTestSeenUnseen() throws PeerConnection.PeerNotCo
final EthPeer ethPeer1 = mock(EthPeer.class);

final EthPeers ethPeers = mock(EthPeers.class);
when(ethPeers.availablePeers()).thenReturn(Stream.of(ethPeer0, ethPeer1));
when(ethPeers.streamAvailablePeers()).thenReturn(Stream.of(ethPeer0, ethPeer1));

final EthContext ethContext = mock(EthContext.class);
when(ethContext.getEthPeers()).thenReturn(ethPeers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public class TrailingPeerLimiterTest {

@Before
public void setUp() {
when(ethPeers.availablePeers()).then(invocation -> peers.stream());
when(ethPeers.streamAvailablePeers()).then(invocation -> peers.stream());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ public void requestsCheckpointsFromSyncTarget() {
// Check that any requests for checkpoint headers are only sent to the best peer
final long checkpointRequestsToOtherPeers =
otherPeers.stream()
.map(RespondingEthPeer::pendingOutgoingRequests)
.map(RespondingEthPeer::streamPendingOutgoingRequests)
.flatMap(Function.identity())
.filter(m -> m.getCode() == EthPV62.GET_BLOCK_HEADERS)
.map(GetBlockHeadersMessage::readFrom)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ public String getName() {
@Override
public JsonRpcResponse response(final JsonRpcRequest request) {
final Map<String, Object> observations = new HashMap<>();
metricsSystem.getMetrics().forEach(observation -> addObservation(observations, observation));
metricsSystem
.streamObservations()
.forEach(observation -> addObservation(observations, observation));
return new JsonRpcSuccessResponse(request.getId(), observations);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void shouldHaveCorrectName() {

@Test
public void shouldReportUnlabelledObservationsByCategory() {
when(metricsSystem.getMetrics())
when(metricsSystem.streamObservations())
.thenReturn(
Stream.of(
new Observation(PEERS, "peer1", "peer1Value", Collections.emptyList()),
Expand All @@ -62,7 +62,7 @@ public void shouldReportUnlabelledObservationsByCategory() {

@Test
public void shouldNestObservationsByLabel() {
when(metricsSystem.getMetrics())
when(metricsSystem.streamObservations())
.thenReturn(
Stream.of(
new Observation(PEERS, "peer1", "value1", asList("label1A", "label2A")),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public Collection<PeerConnection> getPeers() {
}

@Override
public Stream<DiscoveryPeer> getDiscoveredPeers() {
public Stream<DiscoveryPeer> streamDiscoveredPeers() {
return Stream.empty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public Collection<PeerConnection> getPeers() {
}

@Override
public Stream<DiscoveryPeer> getDiscoveredPeers() {
public Stream<DiscoveryPeer> streamDiscoveredPeers() {
return Stream.empty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public interface P2PNetwork extends Closeable {
*
* @return A stream of discovered peers on the network.
*/
Stream<DiscoveryPeer> getDiscoveredPeers();
Stream<DiscoveryPeer> streamDiscoveredPeers();

/**
* Connects to a {@link Peer}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public void addPeerRequirement(final PeerRequirement peerRequirement) {
}

private void startController() {
PeerDiscoveryController controller = createController();
final PeerDiscoveryController controller = createController();
this.controller = Optional.of(controller);
controller.start();
}
Expand Down Expand Up @@ -240,8 +240,8 @@ protected void handleOutgoingPacket(final DiscoveryPeer peer, final Packet packe
});
}

public Stream<DiscoveryPeer> getPeers() {
return controller.map(PeerDiscoveryController::getPeers).orElse(Stream.empty());
public Stream<DiscoveryPeer> streamDiscoveredPeers() {
return controller.map(PeerDiscoveryController::streamDiscoveredPeers).orElse(Stream.empty());
}

public Optional<DiscoveryPeer> getAdvertisedPeer() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ synchronized boolean evict(final PeerId peer) {
*
* @return immutable view of the peer array
*/
synchronized List<DiscoveryPeer> peers() {
synchronized List<DiscoveryPeer> getPeers() {
return unmodifiableList(asList(Arrays.copyOf(kBucket, tailIndex + 1)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ public void start() {

// if smart contract permissioning is enabled, bond with bootnodes
if (nodePermissioningController.get().getSyncStatusNodePermissioningProvider().isPresent()) {
for (DiscoveryPeer p : initialDiscoveryPeers) {
for (final DiscoveryPeer p : initialDiscoveryPeers) {
bond(p);
}
}
Expand Down Expand Up @@ -554,8 +554,8 @@ private <T extends PeerDiscoveryEvent> void dispatchEvent(
*
* @return List of peers.
*/
public Stream<DiscoveryPeer> getPeers() {
return peerTable.getAllPeers();
public Stream<DiscoveryPeer> streamDiscoveredPeers() {
return peerTable.streamAllPeers();
}

public void setRetryDelayFunction(final RetryDelayFunction retryDelayFunction) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public EvictResult tryEvict(final PeerId peer) {

distanceCache.remove(id);

if (table[distance].peers().isEmpty()) {
if (table[distance].getPeers().isEmpty()) {
return EvictResult.absent();
}

Expand All @@ -176,7 +176,7 @@ public EvictResult tryEvict(final PeerId peer) {
private void buildBloomFilter() {
final BloomFilter<BytesValue> bf =
BloomFilter.create((id, val) -> val.putBytes(id.extractArray()), maxEntriesCnt, 0.001);
getAllPeers().map(Peer::getId).forEach(bf::put);
streamAllPeers().map(Peer::getId).forEach(bf::put);
this.evictionCnt = 0;
this.idBloom = bf;
}
Expand All @@ -191,15 +191,15 @@ private void buildBloomFilter() {
*/
public List<DiscoveryPeer> nearestPeers(final BytesValue target, final int limit) {
final BytesValue keccak256 = Hash.keccak256(target);
return getAllPeers()
return streamAllPeers()
.filter(p -> p.getStatus() == PeerDiscoveryStatus.BONDED)
.sorted(comparingInt((peer) -> distance(peer.keccak256(), keccak256)))
.limit(limit)
.collect(toList());
}

public Stream<DiscoveryPeer> getAllPeers() {
return Arrays.stream(table).flatMap(e -> e.peers().stream());
public Stream<DiscoveryPeer> streamAllPeers() {
return Arrays.stream(table).flatMap(e -> e.getPeers().stream());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ void attemptPeerConnections() {
}

final List<DiscoveryPeer> peers =
getDiscoveredPeers()
streamDiscoveredPeers()
.filter(peer -> peer.getStatus() == PeerDiscoveryStatus.BONDED)
.filter(peer -> !isConnected(peer) && !isConnecting(peer))
.collect(Collectors.toList());
Expand All @@ -426,8 +426,8 @@ public Collection<PeerConnection> getPeers() {
}

@Override
public Stream<DiscoveryPeer> getDiscoveredPeers() {
return peerDiscoveryAgent.getPeers();
public Stream<DiscoveryPeer> streamDiscoveredPeers() {
return peerDiscoveryAgent.streamDiscoveredPeers();
}

@Override
Expand Down
Loading

0 comments on commit e115ea3

Please sign in to comment.