sdedic commented on code in PR #7540:
URL: https://github.com/apache/netbeans/pull/7540#discussion_r1668498198
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/actions/AddADBAction.java:
##########
@@ -75,14 +75,16 @@
@NbBundle.Messages({
"AddADB=Add Oracle Autonomous DB",
"SelectProfile=Select OCI Profile",
- "# {0} - tenancy name",
- "# {1} - region id",
- "SelectProfile_Description={0} (region: {1})",
"SelectCompartment=Select Compartment",
"SelectDatabase=Select Database",
"NoDatabase=No Database available in this Compartment",
"EnterUsername=Enter Username",
- "EnterPassword=Enter Password"
+ "EnterPassword=Enter Password",
+ "MSG_CollectingProfiles=Searching for OCI Profiles",
+ "MSG_CollectingProfiles_Text=Loading OCI Profiles",
+ "MSG_CollectingItems=Loading OCI contents",
+ "MSG_CollectingItems_Text=Listing compartments and databases",
+ "SelectProfile_Description={0} (region: {1})"
Review Comment:
Nitpick: parameter defs missing.
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/actions/DownloadWalletDialog.java:
##########
@@ -92,7 +92,7 @@ static Optional<WalletInfo> showDialog(OCIItem db) {
String dbUser = dlgPanel.dbUserField.getText();
char[] dbPasswd = dlgPanel.dbPasswordField.getPassword();
NbPreferences.forModule(DownloadWalletAction.class).put(LAST_USED_DIR, path);
//NOI18N
- return Optional.of(new WalletInfo(path, generatePassword(),
dbUser, dbPasswd, db.getKey().getValue(), db.getCompartmentId()));
+ return Optional.of(new WalletInfo(path, generatePassword(),
dbUser, dbPasswd, (DatabaseItem) db));
Review Comment:
why `db` is typed just as `OCIItem` and not `DatabaseItem` ?
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/assets/Steps.java:
##########
@@ -177,16 +147,31 @@ public NextStepProvider build() {
}
}
}
-
+
+ /**
+ * Provides values for steps in the multi step dialog.
+ *
+ */
+ public interface Values {
+
+ /**
+ * Returns a value for a given {@link Step}.
+ * @param step
+ * @return
+ */
+ public Object getValueForStep(Class<? extends Step> step);
Review Comment:
If this is declared as
```
public <X> X getValueForStep(Class<? extends Step<X>> step);
```
then `Steps` clients do not need to do typecasts when extracting the data,
if the individual Step classes are properly typed. Isn't it too screwed use of
generics @jlahoda ?
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/assets/AbstractStep.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.cloud.oracle.assets;
+
+import org.netbeans.api.progress.ProgressHandle;
+import org.openide.util.Lookup;
+import org.openide.util.NbBundle;
+
+/**
+ *
+ * @author Jan Horvath
+ */
[email protected]({
+ "LoadingItems=Loading items for the next step"
+})
+public abstract class AbstractStep<U> implements Step<U> {
Review Comment:
I would suggest to merge `AbstractStep` with `Step` to one abstract
inheritable class. Then `Steps` implementation (`MultiStep`) can directly send
`Steps.Values` to this abstract class using package-private methods.
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/assets/AbstractStep.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.cloud.oracle.assets;
+
+import org.netbeans.api.progress.ProgressHandle;
+import org.openide.util.Lookup;
+import org.openide.util.NbBundle;
+
+/**
+ *
+ * @author Jan Horvath
+ */
[email protected]({
+ "LoadingItems=Loading items for the next step"
+})
+public abstract class AbstractStep<U> implements Step<U> {
+ private Lookup lookup;
+ protected Steps.Values values;
+
+
+ @Override
+ public final Step prepare(Lookup lookup) {
+ this.lookup = lookup;
+ values = lookup.lookup(Steps.Values.class);
+ ProgressHandle h = ProgressHandle.createHandle(Bundle.LoadingItems());
+ h.start();
+ try {
+ prepare(h);
+ } finally {
+ h.finish();
+ }
+ return this;
+ }
+
+ public void prepare(ProgressHandle handle) {
+ }
+
+ @Override
+ public Step getNext() {
+ Steps.NextStepProvider nsProvider =
lookup.lookup(Steps.NextStepProvider.class);
+ if (nsProvider != null) {
+ Step ns = nsProvider.nextStepFor(this);
Review Comment:
I'd move this logic into `Multistep` and just return `null` at
`AbstractStep` level to indicate the provider should be used. Either the
returned `Step` instance returned from `getNext()` or the provider-returned
`Step` instance would be then `prepare()`d by Steps.
This is motivated by "hidden SPI" between `Multistep` and `AbstractStep`
through Lookup, we should either make it explicit or hide entirely from Step
implementors.
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/steps/CompartmentStep.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.cloud.oracle.steps;
+
+import com.oracle.bmc.identity.Identity;
+import com.oracle.bmc.identity.IdentityClient;
+import com.oracle.bmc.identity.model.Compartment;
+import com.oracle.bmc.identity.requests.ListCompartmentsRequest;
+import com.oracle.bmc.identity.responses.ListCompartmentsResponse;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.TreeMap;
+import org.netbeans.api.progress.ProgressHandle;
+import org.netbeans.modules.cloud.oracle.OCIManager;
+import org.netbeans.modules.cloud.oracle.OCISessionInitiator;
+import org.netbeans.modules.cloud.oracle.assets.AbstractStep;
+import static org.netbeans.modules.cloud.oracle.assets.Steps.createQuickPick;
+import org.netbeans.modules.cloud.oracle.compartment.CompartmentItem;
+import org.netbeans.modules.cloud.oracle.items.OCID;
+import org.netbeans.modules.cloud.oracle.items.OCIItem;
+import org.netbeans.modules.cloud.oracle.items.TenancyItem;
+import org.openide.NotifyDescriptor;
+import org.openide.util.NbBundle;
+
+/**
+ *
+ * @author Jan Horvath
+ */
[email protected]({
+ "NoCompartment=There are no compartments in the Tenancy",
+ "CollectingItems_Text=Listing compartments and databases",
+ "SelectCompartment=Select Compartment",})
+public final class CompartmentStep extends AbstractStep<CompartmentItem> {
+
+ private Map<String, OCIItem> compartments = null;
+ private CompartmentItem selected;
+
+ @Override
+ public void prepare(ProgressHandle h) {
+ h.progress(Bundle.CollectingItems_Text());
+ TenancyItem tenancy = (TenancyItem)
values.getValueForStep(TenancyStep.class);
+ compartments = getFlatCompartment(tenancy);
+ }
+
+ @Override
+ public NotifyDescriptor createInput() {
+ if (onlyOneChoice()) {
+ throw new IllegalStateException("Input shouldn't be displayed for
one choice"); // NOI18N
+ }
+ if (compartments.isEmpty()) {
+ return createQuickPick(compartments, Bundle.NoCompartment());
+ }
+ return createQuickPick(compartments, Bundle.SelectCompartment());
+ }
+
+ @Override
+ public void setValue(String selected) {
+ this.selected = (CompartmentItem) compartments.get(selected);
+ }
+
+ @Override
+ public CompartmentItem getValue() {
+ if (onlyOneChoice()) {
+ return (CompartmentItem) compartments.values().iterator().next();
+ }
+ return selected;
+ }
+
+ @Override
+ public boolean onlyOneChoice() {
+ return compartments.size() == 1;
+ }
+
+ /**
+ * Retrieve all compartments from a tenancy.
+ *
+ * @param tenancy
+ * @return
+ */
+ static private Map<String, OCIItem> getFlatCompartment(TenancyItem
tenancy) {
+ Map<OCID, FlatCompartmentItem> compartments = new HashMap<>();
+ OCISessionInitiator session =
OCIManager.getDefault().getActiveSession();
+ Identity identityClient = session.newClient(IdentityClient.class);
+ String nextPageToken = null;
+
+ do {
+ ListCompartmentsResponse response
+ = identityClient.listCompartments(
+ ListCompartmentsRequest.builder()
+ .compartmentId(tenancy.getKey().getValue())
+ .compartmentIdInSubtree(true)
+
.lifecycleState(Compartment.LifecycleState.Active)
+
.accessLevel(ListCompartmentsRequest.AccessLevel.Accessible)
+ .limit(1000)
+ .page(nextPageToken)
+ .build());
+ for (Compartment comp : response.getItems()) {
+ FlatCompartmentItem ci = new FlatCompartmentItem(comp) {
+ @Override
+ FlatCompartmentItem getItem(OCID compId) {
+ return compartments.get(compId);
+ }
+ };
+ compartments.put(ci.getKey(), ci);
+ }
+ nextPageToken = response.getOpcNextPage();
+ } while (nextPageToken != null);
+ Map<String, OCIItem> pickItems = computeFlatNames(compartments);
+ pickItems.put(tenancy.getName() + " (root)", tenancy); // NOI18N
Review Comment:
I18N
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/assets/AbstractStep.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.cloud.oracle.assets;
+
+import org.netbeans.api.progress.ProgressHandle;
+import org.openide.util.Lookup;
+import org.openide.util.NbBundle;
+
+/**
+ *
+ * @author Jan Horvath
+ */
[email protected]({
+ "LoadingItems=Loading items for the next step"
+})
+public abstract class AbstractStep<U> implements Step<U> {
+ private Lookup lookup;
+ protected Steps.Values values;
+
+
+ @Override
+ public final Step prepare(Lookup lookup) {
+ this.lookup = lookup;
+ values = lookup.lookup(Steps.Values.class);
+ ProgressHandle h = ProgressHandle.createHandle(Bundle.LoadingItems());
Review Comment:
This progresshandle wrapper around `prepare()` could better fit into
`Multistep` wizard driver.
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/steps/DatasourceNameStep.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.cloud.oracle.steps;
+
+import org.netbeans.api.progress.ProgressHandle;
+import org.netbeans.modules.cloud.oracle.assets.AbstractStep;
+import org.openide.NotifyDescriptor;
+import org.openide.util.NbBundle;
+
+/**
+ *
+ * @author Jan Horvath
+ */
[email protected]({
+ "DatasourceName=Datasource Name",
+})
+public class DatasourceNameStep extends AbstractStep<String> {
+ private String selected;
+
+ public DatasourceNameStep() {
+ }
+
+ @Override
+ public void prepare(ProgressHandle h) {
+ }
+
+ @Override
+ public NotifyDescriptor createInput() {
+ return new NotifyDescriptor.InputLine("DEFAULT",
Bundle.DatasourceName()); //NOI18N
Review Comment:
Q: what if the user returns to this step: the previously entered value will
be lost (not offered in the input line) ?
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/items/OCIItem.java:
##########
@@ -27,11 +27,12 @@
*
* @author Jan Horvath
*/
-public abstract class OCIItem {
+public abstract class OCIItem implements NamedReference {
final OCID id;
final String name;
final String compartmentId;
String description;
+ private String referenceName = "DEFAULT";
Review Comment:
Q: each `OCI item` becomes a valid `NamedReference` (regadless of OCI item
type ?)
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/steps/TenancyStep.java:
##########
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.cloud.oracle.steps;
+
+import com.oracle.bmc.identity.model.Tenancy;
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicReference;
+import org.netbeans.api.progress.ProgressHandle;
+import org.netbeans.modules.cloud.oracle.OCIManager;
+import org.netbeans.modules.cloud.oracle.OCIProfile;
+import org.netbeans.modules.cloud.oracle.assets.AbstractStep;
+import org.netbeans.modules.cloud.oracle.items.TenancyItem;
+import org.openide.NotifyDescriptor;
+import org.openide.util.NbBundle;
+
+/**
+ *
+ * @author Jan Horvath
+ */
[email protected]({
+ "SelectProfile=Select OCI Profile",
+ "SelectProfile_Description={0} (region: {1})",
+ "NoProfile=There is not any OCI profile in the config",
+ "CollectingProfiles_Text=Loading OCI Profiles",
+ "SelectKey=Select Key"
+})
+public final class TenancyStep extends AbstractStep<TenancyItem> {
+
+ private List<OCIProfile> profiles = new LinkedList<>();
+ private final AtomicReference<TenancyItem> selected = new
AtomicReference<>();
+
+ @Override
+ public NotifyDescriptor createInput() {
+ if (onlyOneChoice()) {
+ throw new IllegalStateException("No data to create input"); //
NOI18N
+ }
+ String title = Bundle.SelectProfile();
+ List<NotifyDescriptor.QuickPick.Item> items = new
ArrayList<>(profiles.size());
+ for (OCIProfile p : profiles) {
+ Tenancy t = p.getTenancyData();
+ if (t != null) {
+ items.add(new NotifyDescriptor.QuickPick.Item(p.getId(),
Bundle.SelectProfile_Description(t.getName(), t.getHomeRegionKey())));
+ }
+ }
+ if (profiles.stream().filter(p -> p.getTenancy().isPresent()).count()
== 0) {
+ title = Bundle.NoProfile();
+ }
+ return new NotifyDescriptor.QuickPick(title, title, items, false);
+ }
+
+ @Override
+ public void prepare(ProgressHandle h) {
+ h.progress(Bundle.CollectingProfiles_Text());
+ profiles = OCIManager.getDefault().getConnectedProfiles();
+ }
+
+ @Override
+ public void setValue(String value) {
+ for (OCIProfile profile : profiles) {
+ if (profile.getId().equals(value)) {
+ profile.getTenancy().ifPresent(t -> this.selected.set(t));
Review Comment:
AtomicRef is used here, but in all other Step implementations, just a plain
assignment is used ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
