epugh commented on code in PR #4178:
URL: https://github.com/apache/solr/pull/4178#discussion_r3142383141


##########
solr/core/src/java/org/apache/solr/pkg/ClusterPackage.java:
##########
@@ -14,426 +14,291 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.apache.solr.pkg;
 
 import static org.apache.solr.common.cloud.ZkStateReader.SOLR_PKGS_PATH;
 import static 
org.apache.solr.security.PermissionNameProvider.Name.PACKAGE_EDIT_PERM;
 import static 
org.apache.solr.security.PermissionNameProvider.Name.PACKAGE_READ_PERM;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
+import jakarta.inject.Inject;
 import java.io.IOException;
-import java.io.StringWriter;
 import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import org.apache.solr.api.Command;
-import org.apache.solr.api.EndPoint;
-import org.apache.solr.api.PayloadObj;
+import java.util.stream.Collectors;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.client.api.endpoint.PackageApis;
+import org.apache.solr.client.api.model.AddPackageVersionRequestBody;
+import org.apache.solr.client.api.model.PackagesResponse;
+import org.apache.solr.client.api.model.SolrJerseyResponse;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.request.GenericSolrRequest;
-import org.apache.solr.client.solrj.request.beans.PackagePayload;
 import org.apache.solr.client.solrj.response.JavaBinResponseParser;
 import org.apache.solr.common.SolrException;
-import org.apache.solr.common.annotation.JsonProperty;
-import org.apache.solr.common.cloud.SolrZkClient;
-import org.apache.solr.common.cloud.ZooKeeperException;
 import org.apache.solr.common.params.ModifiableSolrParams;
-import org.apache.solr.common.util.CommandOperation;
-import org.apache.solr.common.util.EnvUtils;
-import org.apache.solr.common.util.ReflectMapWriter;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.filestore.FileStoreUtils;
+import org.apache.solr.jersey.PermissionName;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.response.SolrQueryResponse;
-import org.apache.solr.util.SolrJacksonAnnotationInspector;
 import org.apache.zookeeper.KeeperException;
-import org.apache.zookeeper.WatchedEvent;
-import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.data.Stat;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-/** This implements the public end points (/api/cluster/package) of package 
API. */
-public class PackageAPI {
-  public final boolean enablePackages = 
EnvUtils.getPropertyAsBool("solr.packages.enabled", false);
+/**
+ * JAX-RS implementation of the package management API ({@code 
/api/cluster/package}).
+ *
+ * @see PackageApis
+ */
+public class PackageAPI extends JerseyResource implements PackageApis {
   private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  public static final String ERR_MSG =
-      "Package loading is not enabled , Start your nodes with 
-Dsolr.packages.enabled=true";
+  private static final int SYNC_MAX_RETRIES = 10;
+  private static final long SYNC_SLEEP_MS = 10L;
 
-  final CoreContainer coreContainer;
-  private final ObjectMapper mapper = 
SolrJacksonAnnotationInspector.createObjectMapper();
-  private final SolrPackageLoader packageLoader;
-  Packages pkgs;
+  private final CoreContainer coreContainer;
+  private final SolrQueryRequest solrQueryRequest;
+  private final SolrQueryResponse solrQueryResponse;
 
-  public final Edit editAPI = new Edit();
-  public final Read readAPI = new Read();
-
-  public PackageAPI(CoreContainer coreContainer, SolrPackageLoader loader) {
+  @Inject
+  public PackageAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
     this.coreContainer = coreContainer;
-    this.packageLoader = loader;
-    pkgs = new Packages();
-    SolrZkClient zkClient = coreContainer.getZkController().getZkClient();
-    try {
-      pkgs = readPkgsFromZk(null, null);
-    } catch (KeeperException | InterruptedException e) {
-      pkgs = new Packages();
-      // ignore
+    this.solrQueryRequest = solrQueryRequest;
+    this.solrQueryResponse = solrQueryResponse;
+  }
+
+  @Override
+  @PermissionName(PACKAGE_READ_PERM)
+  public PackagesResponse listPackages(String refreshPackage, Integer 
expectedVersion) {
+    PackageStore packageStore = 
coreContainer.getPackageLoader().getPackageStore();
+
+    if (refreshPackage != null) {
+      packageStore.packageLoader.notifyListeners(refreshPackage);
+      return instantiateJerseyResponse(PackagesResponse.class);
     }
-    try {
-      registerListener(zkClient);
-    } catch (KeeperException | InterruptedException e) {
-      SolrZkClient.checkInterrupted(e);
+
+    if (expectedVersion != null) {
+      syncToVersion(packageStore, expectedVersion);
     }
-  }
 
-  private void registerListener(SolrZkClient zkClient)
-      throws KeeperException, InterruptedException {
-    zkClient.exists(
-        SOLR_PKGS_PATH,
-        new Watcher() {
-
-          @Override
-          public void process(WatchedEvent event) {
-            // session events are not change events, and do not remove the 
watcher
-            if (Event.EventType.None.equals(event.getType())) {
-              return;
-            }
-            synchronized (this) {
-              log.debug("Updating [{}] ... ", SOLR_PKGS_PATH);
-              // remake watch
-              final Watcher thisWatch = this;
-              refreshPackages(thisWatch);
-            }
-          }
-        });
+    final var response = instantiateJerseyResponse(PackagesResponse.class);
+    response.result = toPackageData(packageStore.pkgs);
+    return response;
   }
 
-  public void refreshPackages(Watcher watcher) {
-    final Stat stat = new Stat();
-    try {
-      final byte[] data =
-          
coreContainer.getZkController().getZkClient().getData(SOLR_PKGS_PATH, watcher, 
stat);
-      pkgs = readPkgsFromZk(data, stat);
-      packageLoader.refreshPackageConf();
-    } catch (KeeperException.ConnectionLossException | 
KeeperException.SessionExpiredException e) {
-      log.warn("ZooKeeper watch triggered, but Solr cannot talk to ZK: ", e);
-    } catch (KeeperException e) {
-      log.error("A ZK error has occurred", e);
-      throw new ZooKeeperException(SolrException.ErrorCode.SERVER_ERROR, "", 
e);
-    } catch (InterruptedException e) {
-      // Restore the interrupted status
-      Thread.currentThread().interrupt();
-      log.warn("Interrupted", e);
+  @Override
+  @PermissionName(PACKAGE_READ_PERM)
+  public PackagesResponse getPackage(String packageName) {
+    PackageStore packageStore = 
coreContainer.getPackageLoader().getPackageStore();
+    final var response = instantiateJerseyResponse(PackagesResponse.class);
+    response.result = toPackageData(packageStore.pkgs);
+    // Filter to only the requested package
+    if (response.result != null && response.result.packages != null) {
+      final var pkgVersions = response.result.packages.get(packageName);
+      response.result.packages = Collections.singletonMap(packageName, 
pkgVersions);
     }
+    return response;
   }
 
-  private Packages readPkgsFromZk(byte[] data, Stat stat)
-      throws KeeperException, InterruptedException {
+  @Override
+  @PermissionName(PACKAGE_EDIT_PERM)
+  public SolrJerseyResponse addPackageVersion(
+      String packageName, AddPackageVersionRequestBody requestBody) {
+    final var response = instantiateJerseyResponse(SolrJerseyResponse.class);
+    PackageStore packageStore = 
coreContainer.getPackageLoader().getPackageStore();
 
-    if (data == null || stat == null) {
-      stat = new Stat();
-      data = 
coreContainer.getZkController().getZkClient().getData(SOLR_PKGS_PATH, null, 
stat);
+    if (!packageStore.isEnabled()) {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, 
PackageStore.ERR_MSG);
     }
-    Packages packages = null;
-    if (data == null || data.length == 0) {
-      packages = new Packages();
-    } else {
-      try {
-        packages = mapper.readValue(data, Packages.class);
-        packages.znodeVersion = stat.getVersion();
-      } catch (IOException e) {
-        // invalid data in packages
-        // TODO handle properly;
-        return new Packages();
-      }
+    if (requestBody == null || requestBody.files == null || 
requestBody.files.isEmpty()) {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "No files 
specified");
     }
-    return packages;
-  }
-
-  public static class Packages implements ReflectMapWriter {
-    @JsonProperty public int znodeVersion = -1;
 
-    @JsonProperty public Map<String, List<PkgVersion>> packages = new 
LinkedHashMap<>();
+    final List<String> errors = new ArrayList<>();
+    FileStoreUtils.validateFiles(
+        coreContainer.getFileStore(), requestBody.files, true, errors::add);
+    if (!errors.isEmpty()) {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, 
String.join("; ", errors));
+    }
 
-    public Packages copy() {
-      Packages p = new Packages();
-      p.znodeVersion = this.znodeVersion;
-      p.packages = new LinkedHashMap<>();
-      packages.forEach((s, versions) -> p.packages.put(s, new 
ArrayList<>(versions)));
-      return p;
+    final PackageStore.Packages[] finalState = new PackageStore.Packages[1];
+    try {
+      coreContainer
+          .getZkController()
+          .getZkClient()
+          .atomicUpdate(
+              SOLR_PKGS_PATH,
+              (stat, bytes) -> {
+                PackageStore.Packages packages;
+                try {
+                  packages =
+                      bytes == null
+                          ? new PackageStore.Packages()
+                          : packageStore.mapper.readValue(bytes, 
PackageStore.Packages.class);
+                  packages = packages.copy();
+                } catch (IOException e) {
+                  log.error("Error deserializing packages.json", e);
+                  packages = new PackageStore.Packages();
+                }
+                List<PackageStore.PkgVersion> list =
+                    packages.packages.computeIfAbsent(packageName, o -> new 
ArrayList<>());
+                for (PackageStore.PkgVersion pkgVersion : list) {
+                  if (Objects.equals(pkgVersion.version, requestBody.version)) 
{
+                    throw new SolrException(
+                        SolrException.ErrorCode.BAD_REQUEST,
+                        "Version '" + requestBody.version + "' exists 
already");
+                  }
+                }
+                list.add(new PackageStore.PkgVersion(packageName, 
requestBody));
+                packages.znodeVersion = stat.getVersion() + 1;
+                finalState[0] = packages;
+                return Utils.toJSON(packages);
+              });
+    } catch (KeeperException | InterruptedException e) {
+      finalState[0] = null;
+      packageStore.handleZkErr(e);
     }
-  }
 
-  public static class PkgVersion implements ReflectMapWriter {
+    if (finalState[0] != null) {
+      packageStore.pkgs = finalState[0];
+      notifyAllNodesToSync(packageStore.pkgs.znodeVersion);
+      coreContainer.getPackageLoader().refreshPackageConf();
+    }
 
-    @JsonProperty("package")
-    public String pkg;
+    return response;
+  }
 
-    @JsonProperty public String version;
+  @Override
+  @PermissionName(PACKAGE_EDIT_PERM)
+  public SolrJerseyResponse deletePackageVersion(String packageName, String 
version) {
+    final var response = instantiateJerseyResponse(SolrJerseyResponse.class);
+    PackageStore packageStore = 
coreContainer.getPackageLoader().getPackageStore();
 
-    @JsonProperty public List<String> files;
+    if (!packageStore.isEnabled()) {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, 
PackageStore.ERR_MSG);
+    }
 
-    @JsonProperty public String manifest;
+    try {
+      coreContainer
+          .getZkController()
+          .getZkClient()
+          .atomicUpdate(
+              SOLR_PKGS_PATH,
+              (stat, bytes) -> {
+                PackageStore.Packages packages;
+                try {
+                  packages = packageStore.mapper.readValue(bytes, 
PackageStore.Packages.class);
+                  packages = packages.copy();
+                } catch (IOException e) {
+                  packages = new PackageStore.Packages();
+                }
+
+                List<PackageStore.PkgVersion> versions = 
packages.packages.get(packageName);
+                if (versions == null || versions.isEmpty()) {
+                  throw new SolrException(
+                      SolrException.ErrorCode.BAD_REQUEST, "No such package: " 
+ packageName);
+                }
+                int idxToRemove = -1;
+                for (int i = 0; i < versions.size(); i++) {
+                  if (Objects.equals(versions.get(i).version, version)) {
+                    idxToRemove = i;
+                    break;
+                  }
+                }
+                if (idxToRemove == -1) {
+                  throw new SolrException(
+                      SolrException.ErrorCode.BAD_REQUEST, "No such version: " 
+ version);
+                }
+                versions.remove(idxToRemove);
+                packages.znodeVersion = stat.getVersion() + 1;
+                return Utils.toJSON(packages);
+              });
+    } catch (KeeperException | InterruptedException e) {
+      packageStore.handleZkErr(e);
+    }
 
-    @JsonProperty public String manifestSHA512;
+    return response;
+  }
 
-    public PkgVersion() {}
+  @Override
+  @PermissionName(PACKAGE_EDIT_PERM)
+  public SolrJerseyResponse refreshPackage(String packageName) {
+    final var response = instantiateJerseyResponse(SolrJerseyResponse.class);
+    PackageStore packageStore = 
coreContainer.getPackageLoader().getPackageStore();
 
-    public PkgVersion(PackagePayload.AddVersion addVersion) {
-      this.pkg = addVersion.pkg;
-      this.version = addVersion.version;
-      this.files = addVersion.files == null ? null : 
Collections.unmodifiableList(addVersion.files);
-      this.manifest = addVersion.manifest;
-      this.manifestSHA512 = addVersion.manifestSHA512;
+    SolrPackageLoader.SolrPackage pkg = 
coreContainer.getPackageLoader().getPackage(packageName);
+    if (pkg == null) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST, "No such package: " + 
packageName);
     }
+    // first refresh on the current node
+    packageStore.packageLoader.notifyListeners(packageName);
 
-    @Override
-    public boolean equals(Object obj) {
-      if (obj instanceof PkgVersion that) {
-        return Objects.equals(this.version, that.version) && 
Objects.equals(this.files, that.files);
-      }
-      return false;
-    }
+    final var solrParams = new ModifiableSolrParams();
+    solrParams.add("omitHeader", "true");
+    solrParams.add("refreshPackage", packageName);
 
-    @Override
-    public int hashCode() {
-      return Objects.hash(version);
-    }
+    final var request =

Review Comment:
   so many places to update...    I suspect in the future we need to do a 
"Check each GSR for fitness" check ;-)



-- 
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]

Reply via email to