Add models operations to twin lifecycle sample #14781

abhipsaMisra · 2020-09-03T16:44:48Z

Add models operations to twin lifecycle sample

abhipsaMisra · 2020-09-03T16:46:19Z

...ure-digitaltwins-core/src/main/java/com/azure/digitaltwins/core/DigitalTwinsAsyncClient.java

@@ -909,7 +909,7 @@ public HttpPipeline getHttpPipeline() {
    @ServiceMethod(returns = ReturnType.SINGLE)
    public Mono<Void> deleteModel(String modelId) {
        return deleteModelWithResponse(modelId)
-            .map(Response::getValue);
+            .flatMap(voidResponse -> Mono.empty());


.map(Response::getValue) returns null, which is not a valid return item from a Mono. So we need to map it to Mono.empty() instead.

abhipsaMisra · 2020-09-03T16:48:08Z

...wins-core/src/samples/java/com/azure/digitaltwins/core/DigitalTwinsLifecycleAsyncSample.java

+            Semaphore deleteRelationshipsSemaphore = new Semaphore(0);
+            CountDownLatch deleteTwinsLatch = new CountDownLatch(1);
+
+            // Call APIs to retrieve all relationships.


This implementation is very long and verbose. I am trying to see how I could chain the list and delete operations together, I'll put up an update in the next PR.

abhipsaMisra · 2020-09-03T16:54:52Z

...wins-core/src/samples/java/com/azure/digitaltwins/core/DigitalTwinsLifecycleAsyncSample.java

+        List<String> models = asList(RoomModelId, WifiModelId, BuildingModelId, FloorModelId, HvacModelId);
+
+        // Call APIs to delete the models.
+        // Note that we are blocking the async API call. This is to ensure models are deleted in an order such that no other models are referencing it.


We should have been able to schedule the async API on a single thread, and forced them to run sequentially; however, I didn't have success with that.
I am blocking the async API call for now, until I figure that out.

bikamani · 2020-09-03T20:20:14Z

...wins-core/src/samples/java/com/azure/digitaltwins/core/DigitalTwinsLifecycleAsyncSample.java

+                    .doOnTerminate(deleteTwinsLatch::countDown)
+                    .subscribe();
+
+                // Wait until the latch has been counted down for each async delete operation, signifying that the async call has completed successfully.


For each async delete? Can you elaborate what you mean here?

What is a latch?

Appears to be some kind of threadsafe counter, eh?

I'd reword this to be "Wait until the latch count reaches zero..."

A countdown latch is a mechanism to block the calling thread until other threads that are running in parallel have completed (counted down). The .countdown() essentially decrements a thread-safe counter.

From this article online: https://www.baeldung.com/java-countdown-latch
Simply put, a CountDownLatch has a counter field, which you can decrement as we require. We can then use it to block a calling thread until it's been counted down to zero.

If we were doing some parallel processing, we could instantiate the CountDownLatch with the same value for the counter as a number of threads we want to work across. Then, we could just call countdown() after each thread finishes, guaranteeing that a dependent thread calling await() will block until the worker threads are finished.

drwill-ms · 2020-09-03T20:25:43Z

...wins-core/src/samples/java/com/azure/digitaltwins/core/DigitalTwinsLifecycleAsyncSample.java

+                    .forEach(relationship -> client.deleteRelationship(relationship.getSourceId(), relationship.getId())
+                        .doOnSuccess(aVoid -> {
+                            if (twinId.equals(relationship.getSourceId())) {
+                                System.out.println("Found and deleted relationship: " + relationship.getId());


Does Java support string interpolation?

We can use String.format() to specify a template, but nothing as handy as $ in C#.

drwill-ms · 2020-09-03T20:29:46Z

...wins-core/src/samples/java/com/azure/digitaltwins/core/DigitalTwinsLifecycleAsyncSample.java

+            List<BasicRelationship> relationshipList = new ArrayList<>();
+            Semaphore listRelationshipSemaphore = new Semaphore(0);
+            Semaphore deleteRelationshipsSemaphore = new Semaphore(0);
+            CountDownLatch deleteTwinsLatch = new CountDownLatch(1);


Unfortunate that you have to know ahead of time how many items you are counting down. This presents a "magic" number, which can be confusing to someone else reviewing the code who doesn't realize the significance (this will be used to asynchronously delete 1 dt).

Also, in C# we have a general rule of not declaring a variable until it is needed (so it is more contextual). Does Java have a different standard of all variables at the top?

FWIW, I think this could at least use a comment.

We need to initialize these latches and semaphore within the scope that they are referenced in; but yes, I agree, the count can be confusing to understand.
I'll add some more comments around this.

drwill-ms · 2020-09-03T20:31:08Z

...wins-core/src/samples/java/com/azure/digitaltwins/core/DigitalTwinsLifecycleAsyncSample.java

+                    System.out.println("Deleted model: " + modelId);
+                } catch (ErrorResponseException ex) {
+                    if (ex.getResponse().getStatusCode() != HttpStatus.SC_NOT_FOUND) {
+                        System.err.println("Could not delete model " + modelId + " due to " + ex);


If Java doesn't have string interpolation, I'd think this would be much more readable as a string format (like below).

azabbasi

I think this sample is doing way too much work. Reconsider making it just creating a brand new model with brand new id and create a twin off of that and then clean up after itself.
That would make it concise and easier to navigate.

azabbasi · 2020-09-03T19:59:40Z

...ure-digitaltwins-core/src/main/java/com/azure/digitaltwins/core/DigitalTwinsAsyncClient.java

@@ -788,7 +788,7 @@ public HttpPipeline getHttpPipeline() {
                        objectPagedResponse.getHeaders(),
                        convertedList,
                        null,
-                        ((PagedResponseBase) objectPagedResponse).getDeserializedHeaders());
+                        ((ResponseBase) objectPagedResponse).getDeserializedHeaders());


abhipsaMisra commented Sep 3, 2020

View reviewed changes

samples(adt): Add model operations to twin sample

48c3c37

abhipsaMisra force-pushed the feature/adt/abmisr/twinSample branch from e65c336 to 48c3c37 Compare September 3, 2020 16:50

typo

3f48f68

abhipsaMisra commented Sep 3, 2020

View reviewed changes

abhipsaMisra added the Digital Twins label Sep 3, 2020

abhipsaMisra changed the title ~~Add models operations to twin lifecycle~~ Add models operations to twin lifecycle sample Sep 3, 2020

cannot infer type args in anonymous inner classes

fec01ea

abhipsaMisra marked this pull request as ready for review September 3, 2020 17:34

abhipsaMisra requested review from azabbasi, barustum, bikamani, drwill-ms, timtay-microsoft and vinagesh as code owners September 3, 2020 17:34

abhipsaMisra added 2 commits September 3, 2020 12:23

add static func criteria for ignorning errors

e2ccee8

remove latch assertions

f02069d

abhipsaMisra mentioned this pull request Sep 3, 2020

Add Model API tests to ADT, fix a few ADT implementation details #14791

Merged

bikamani reviewed Sep 3, 2020

View reviewed changes

drwill-ms reviewed Sep 3, 2020

View reviewed changes

drwill-ms approved these changes Sep 3, 2020

View reviewed changes

azabbasi reviewed Sep 3, 2020

View reviewed changes

azabbasi approved these changes Sep 3, 2020

View reviewed changes

abhipsaMisra added 3 commits September 3, 2020 14:51

added additional comments

c3212ab

typo

267370d

add comments

c94e5d9

abhipsaMisra closed this Sep 3, 2020

abhipsaMisra reopened this Sep 3, 2020

abhipsaMisra merged commit f5fbc85 into master Sep 3, 2020

abhipsaMisra deleted the feature/adt/abmisr/twinSample branch September 3, 2020 22:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add models operations to twin lifecycle sample #14781

Add models operations to twin lifecycle sample #14781

abhipsaMisra commented Sep 3, 2020 •

edited

Loading

abhipsaMisra Sep 3, 2020

abhipsaMisra Sep 3, 2020

abhipsaMisra Sep 3, 2020

bikamani Sep 3, 2020

drwill-ms Sep 3, 2020

drwill-ms Sep 3, 2020

abhipsaMisra Sep 3, 2020

drwill-ms Sep 3, 2020

abhipsaMisra Sep 3, 2020

drwill-ms Sep 3, 2020

abhipsaMisra Sep 3, 2020

drwill-ms Sep 3, 2020

azabbasi left a comment

azabbasi Sep 3, 2020

Add models operations to twin lifecycle sample #14781

Add models operations to twin lifecycle sample #14781

Conversation

abhipsaMisra commented Sep 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azabbasi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhipsaMisra commented Sep 3, 2020 •

edited

Loading