Skip to content
Snippets Groups Projects

Initial project files

Merged Martin Lowe requested to merge github/fork/autumnfound/malowe/master/init into master

Change-Id: I403713686bb7a5842feb211241de5f2530e0a457 Signed-off-by: Martin Lowe martin.lowe@eclipse-foundation.org

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Christopher Guindon
  • Christopher Guindon
  • 68 public <T, M extends SolrBeanMapper<T>> List<T> get(SolrQuery q, M mapper) {
    69 List<T> beans = new LinkedList<>();
    70 try {
    71 // query solr for documents using the below query
    72 QueryResponse r = client.query(q);
    73 List<SolrDocument> docs = r.getResults();
    74
    75 // log the data to trace, to not burden the logs
    76 if (LOGGER.isTraceEnabled()) {
    77 LOGGER.trace("Found {} results for query {}", docs.size(), q.toQueryString());
    78 }
    79
    80 // convert the solr documents using the bean mappers
    81 for (SolrDocument doc : docs) {
    82 beans.add(mapper.toBean(doc));
    83 }
    • Created by: mbarbero

      I like functional for those simple stuff. What do you think about:

      return docs.stream().map(mapper::toBean).collect(Collectors.toList());
  • Christopher Guindon
  • Christopher Guindon
  • 89 "Error while streaming results for current query: " + q.toString(), e);
    90 }
    91 }
    92
    93 @Override
    94 public <T, M extends SolrBeanMapper<T>> void add(List<T> beans, M mapper) {
    95 // create a list of documents to upload to Solr
    96 List<SolrInputDocument> solrDocuments = new ArrayList<>(beans.size());
    97 for (T bean : beans) {
    98 // generate the Solr document from the current bean
    99 SolrInputDocument solrDoc = mapper.toDocument(bean);
    100 if (solrDoc != null) {
    101 solrDocuments.add(solrDoc);
    102 }
    103 }
    104
    • Created by: mbarbero

      Same:

      List<SolrInputDocument> solrDocuments = beans.stream().map(mapper::toDocument).filter(Objects::nonNull).collect(Collectors.toList())
  • Christopher Guindon
  • 1 /*
    2 * Copyright (C) 2019 Eclipse Foundation and others.
    3 *
    4 * This program and the accompanying materials are made
    5 * available under the terms of the Eclipse Public License 2.0
    6 * which is available at https://www.eclipse.org/legal/epl-2.0/
    7 *
    8 * SPDX-License-Identifier: EPL-2.0
    9 */
    10 package org.eclipsefoundation.marketplace.model;
    11
    12 import javax.ws.rs.core.Response;
    13 import javax.ws.rs.core.Response.Status;
    14
    15 public class Error {
    • Created by: mbarbero

      I don't think this class is required to be mutable. Once the fields are set in the constructor, I don't see the use case for modifying them. I'd suggest to remove all setters and and set all fields as final

  • Christopher Guindon
  • Christopher Guindon
  • Christopher Guindon
  • Christopher Guindon
  • 126 throw new IllegalArgumentException(EMPTY_KEY_MESSAGE);
    127 }
    128
    129 // return immediately if theres no value
    130 if (value == null) {
    131 return;
    132 }
    133
    134 // set the param, creating list if it doesn't exist
    135 List<String> param = this.params.get(key);
    136 if (param == null) {
    137 this.setParam(key, value);
    138 } else {
    139 param.add(value);
    140 }
    141 }
    • Created by: mbarbero

      while reading addParam and setParam, I've the feeling that they should be merged into somehting like

      public void addParam(String key, String value) {
          if (StringUtils.isBlank(key)) {
               throw new IllegalArgumentException(EMPTY_KEY_MESSAGE);
           }
          if (value == null) {
              throw new NullPointerException("value must not be null");
          }
          map.computeIfAbsent(key, k -> new ArrayList<>()).add(value);
      }

      and add the unsetParam(String key) method as mentioned above.

  • Created by mbarbero

    Review: Changes requested

    Pleas have a look at the couple of comments I made. Thanks.

  • Christopher Guindon
  • Christopher Guindon
  • 25 private RawSolrResultMapper() {
    26 }
    27
    28 @Override
    29 public RawSolrResult toBean(SolrDocument doc) {
    30 RawSolrResult result = new RawSolrResult();
    31 if (doc.getFieldValueMap().isEmpty()) {
    32 return result;
    33 }
    34
    35 // due to Solr implementation of getFieldValuesMap, the values must be iterated
    36 // over using keyset to pick up all fields, otherwise unsupported operation
    37 // exception is thrown.
    38 for (String key : doc.getFieldValuesMap().keySet()) {
    39 result.setField(key, doc.getFieldValue(key));
    40 }
    • Created by: mbarbero

      Never iterate of keys of map to do a get on each key. It's very ineffective.

      Better iterate on the Map.Entry<K, V> return type of entries() (or even better, use forEach)

      doc.getFieldValuesMap().forEach((key, value) -> result.setField(key, value));
  • Christopher Guindon
  • Christopher Guindon
  • Christopher Guindon
  • Christopher Guindon
  • Martin Lowe
  • Martin Lowe
    Martin Lowe @malowe started a thread on commit 3a9aa03e
  • 25 private RawSolrResultMapper() {
    26 }
    27
    28 @Override
    29 public RawSolrResult toBean(SolrDocument doc) {
    30 RawSolrResult result = new RawSolrResult();
    31 if (doc.getFieldValueMap().isEmpty()) {
    32 return result;
    33 }
    34
    35 // due to Solr implementation of getFieldValuesMap, the values must be iterated
    36 // over using keyset to pick up all fields, otherwise unsupported operation
    37 // exception is thrown.
    38 for (String key : doc.getFieldValuesMap().keySet()) {
    39 result.setField(key, doc.getFieldValue(key));
    40 }
    • Author Maintainer

      The reason I do this is a limitation of SolrJ when handling a SolrDocument. The map returned by getFieldValuesMap() is an anonymous implementation of the Map interface with the following for entrySet():

            @Override
            public Set<java.util.Map.Entry<String, Collection<Object>>> entrySet() {throw new UnsupportedOperationException();}

      While it does require the extra operations of retrieving the value for the key, it is the best available option.

  • Christopher Guindon
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading