Add service info retrieving [ECR-3859] #1247

MakarovS · 2019-11-28T01:28:22Z

Overview

Add service info retrieving.
Targeted into ECR-3860 until it's merged.

See: https://jira.bf.local/browse/ECR-3859

Definition of Done

There are no TODOs left in the code
Change is covered by automated tests
The coding guidelines are followed
Public API has Javadoc
Method preconditions are checked and documented in the Javadoc of the method
Changelog is updated if needed (in case of notable or breaking changes)
The continuous integration build passes

dmitry-timofeev · 2019-11-28T15:44:03Z

exonum-light-client/src/main/java/com/exonum/client/ExonumClient.java

+   *
+   * @param serviceName the name of a service instance
+   */
+  Optional<ServiceInfo> getServiceInfoByName(String serviceName);


Shall we use get or find prefix to highlight it returns an Optional?

Changed to findServiceInfoByName

dmitry-timofeev · 2019-11-28T15:45:24Z

exonum-light-client/src/main/java/com/exonum/client/ExonumClient.java

+  Optional<ServiceInfo> getServiceInfoByName(String serviceName);
+
+  /**
+   * Returns the list of service info of all started service instances.


Returns information on/about all started service instances.

@see #getServiceInfoByName

dmitry-timofeev · 2019-11-28T15:47:55Z

exonum-light-client/src/main/lombok/com/exonum/client/response/ServiceInfo.java

+  String name;
+
+  /**
+   * Service id.


I think it must use just as good documentation as com.exonum.binding.core.runtime.ServiceInstanceSpec.

dmitry-timofeev · 2019-11-28T15:51:01Z

exonum-light-client/src/test/java/com/exonum/client/ExonumHttpClientIntegrationTest.java

+  }
+
+  @Test
+  void getServiceInfoByInvalidName() throws InterruptedException {


The name seems OK in this case, but the service is not found by the name. Unknown name/NotFound.

dmitry-timofeev · 2019-11-28T16:02:10Z

exonum-light-client/README.md

-
+
+### Retrieving service info
+To build a transaction, service id is needed. It can be obtained with either


This documentation seems out of place. I think it shall be included in the section on creating a transaction message, saying that the service id, required to create a message, can be obtained, if needed, by the service name:

// Update the code to be correct and compile! int serviceId = exonumClient.getServiceInfoByName(serviceName) .mapToInt(#getId) .orElseThrow(() -> new IllegalStateException("No service with the given name found: " + serviceName);

This section may remain, but rewritten as just the way to get information on active services in the blockchain.

but rewritten as just the way to get information on active services in the blockchain.

It still starts as "To build a transaction, service id is needed."

dmitry-timofeev · 2019-11-29T14:39:06Z

exonum-light-client/README.md

-
+
+### Retrieving service info
+To build a transaction, service id is needed. It can be obtained with either


but rewritten as just the way to get information on active services in the blockchain.

It still starts as "To build a transaction, service id is needed."

dmitry-timofeev · 2019-11-29T14:41:28Z

exonum-light-client/src/main/java/com/exonum/client/ExonumClient.java

   * @param serviceName the name of a service instance
   */
-  Optional<ServiceInfo> getServiceInfoByName(String serviceName);
+  Optional<ServiceInfo> findServiceInfoByName(String serviceName);


Maybe drop ByName since it must be clear enough from the parameter list and there are no other String query parameters (i.e., no BySomeOtherStringProperty that won't work with overloads)?

dmitry-timofeev · 2019-11-29T14:43:04Z

exonum-light-client/src/test/java/com/exonum/client/ExplorerApiHelperTest.java


    List<ServiceInfo> actual = ExplorerApiHelper.parseServicesResponse(json);
    assertThat(actual, contains(expected.toArray()));
+    int serviceId = Optional.of(serviceInfo1)


That must be a draft.

dmitry-timofeev · 2019-11-29T14:43:29Z

exonum-light-client/src/test/java/com/exonum/client/ExplorerApiHelperTest.java

    List<ServiceInfo> actual = ExplorerApiHelper.parseServicesResponse(json);
    assertThat(actual, contains(expected.toArray()));
+    int serviceId = Optional.of(serviceInfo1)
+        .map(ServiceInfo::getId)


Will mapToInt be more appropriate (in the example also)? OptionalInt also has https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/OptionalInt.html#orElseThrow(java.util.function.Supplier)

Sorry, either I don't understand what exactly do you propose here or it's something like

int serviceId = Optional.of(serviceInfo1).stream() .mapToInt(ServiceInfo::getId) .findFirst() .orElseThrow(() -> new IllegalStateException("No service with the given name found: " + serviceName);

which doesn't seem better to me.
Optional doesn't have mapToInt method.

I see, sorry, I must have missed there is no mapToInt in Optional. It's good then, a stream is certainly inferior.

exonum-light-client/src/test/java/com/exonum/client/ExplorerApiHelperTest.java

dmitry-timofeev · 2019-12-02T10:58:38Z

Why does it target ECR-3860 and not master?

MakarovS · 2019-12-02T20:58:28Z

Why does it target ECR-3860 and not master?

Sorry, I expected ECR-3860 to be merged before this one and it was easier to develop based on a working version of LC. I still think it's easier to just merge ECR-3860 first, as it's ready as well.

MakarovS added 6 commits November 26, 2019 20:06

ECR-3860 Update LC with DS changes

b35a48e

Merge remote-tracking branch 'origin/master' into ECR-3860

76b4050

ECR-3860 Review fixes

dbc08c9

ECR-3859 Add service info retrieving

3977d48

ECR-3860 Minor changelog fix

589f861

ECR-3859 Minor fixes

1469d0a

MakarovS requested review from bullet-tooth and dmitry-timofeev November 28, 2019 01:28

MakarovS added 3 commits November 27, 2019 20:29

ECR-3859 Update changelog

6171a47

Merge remote-tracking branch 'origin/ECR-3860' into ECR-3859

d605811

ECR-3859 Fix services URL

595849c

dmitry-timofeev reviewed Nov 28, 2019

View reviewed changes

MakarovS added 3 commits November 28, 2019 16:42

ECR-3860 Add other possible execution statuses

cc470a0

Merge remote-tracking branch 'origin/master' into ECR-3860

9fb0e60

ECR-3859 Review fixes

466bdad

dmitry-timofeev reviewed Nov 29, 2019

View reviewed changes

MakarovS added 3 commits November 29, 2019 18:00

ECR-3859 Review fixes

5a92078

ECR-3860 Refactor ExplorerApiHelperTest

453c623

Merge remote-tracking branch 'remotes/origin/ECR-3860' into ECR-3859

cdb6a1c

dmitry-timofeev reviewed Dec 2, 2019

View reviewed changes

exonum-light-client/src/test/java/com/exonum/client/ExplorerApiHelperTest.java Show resolved Hide resolved

dmitry-timofeev approved these changes Dec 2, 2019

View reviewed changes

ECR-3860 Use arguments method

9c1a4a8

MakarovS added 6 commits December 2, 2019 17:21

ECR-3860 Change Transaction format

dfde8ce

Merge remote-tracking branch 'remotes/origin/master' into ECR-3860

003a8e1

ECR-3859 Update list of capabilities in README

1e5c6e1

Merge remote-tracking branch 'remotes/origin/ECR-3860' into ECR-3859

81267b5

ECR-3860 Remove time and redundant pom property

2bff988

Merge remote-tracking branch 'remotes/origin/ECR-3860' into ECR-3859

c3b4951

MakarovS added 2 commits December 4, 2019 01:34

Merge remote-tracking branch 'remotes/origin/master' into ECR-3860

41d708e

Merge remote-tracking branch 'remotes/origin/ECR-3860' into ECR-3859

2c71bda

bullet-tooth approved these changes Dec 4, 2019

View reviewed changes

dmitry-timofeev changed the base branch from ECR-3860 to master December 4, 2019 14:30

dmitry-timofeev and others added 5 commits December 4, 2019 16:33

Merge remote-tracking branch 'main/master' into ECR-3859

078f26c

Merge remote-tracking branch 'remotes/origin/master' into ECR-3859

53533d1

Merge remote-tracking branch 'remotes/origin/master' into ECR-3859

30d4ec2

Fix the import

9705085

Exclude the whole package

b765e1f

dmitry-timofeev merged commit 24a8f4b into master Dec 6, 2019

dmitry-timofeev deleted the ECR-3859 branch December 6, 2019 12:05



		### Retrieving service info
		To build a transaction, service id is needed. It can be obtained with either

Add service info retrieving [ECR-3859] #1247

Add service info retrieving [ECR-3859] #1247

Uh oh!

Conversation

MakarovS commented Nov 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Definition of Done

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmitry-timofeev commented Dec 2, 2019

Uh oh!

MakarovS commented Dec 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

MakarovS commented Nov 28, 2019 •

edited

Loading

MakarovS commented Dec 2, 2019 •

edited

Loading