Symulacja datasource w pamięcie

0

Hej

Mam za zadanie napisać repozytorium z lokalnym storagem, wymaganiem jest, żeby było wszytko thread safe. Wybrałem do tego ConcurrencyHashMap, czy takie rozwiązanie jest ok ?

public class InMemoryRepository implements Repository {

    private ConcurrentHashMap<Integer, Note> noteStorage;

    public InMemoryRepository() {
        this.noteStorage = new ConcurrentHashMap<>();
    }

    @Override
    public void save(Note note) {
        noteStorage.put(note.getId(), note);
    }

    @Override
    public Optional<Note> findById(int id) {
        return Optional.ofNullable(noteStorage.get(id));
    }

    @Override
    public Optional<Note> update(Note note){
        return Optional.ofNullable(noteStorage.put(note.getId(), note));
    }

    @Override
    public Collection<Note> findAll() {
        return noteStorage
                .entrySet()
                .stream()
                .map(Map.Entry::getValue)
                .collect(Collectors.toList());
    }

    @Override
    public Collection<Note> findCompletedNorteByCategoryAndSubCategoryAndMarketRefName(String category, String subCategory, String refName) {
        return noteStorage
                .entrySet()
                .parallelStream()
                .map(Map.Entry::getValue)
                .filter(Note::isCompleted)
                .filter(note -> (refName == null || note.hasMarketName(refName)))
                .filter(note -> {
                    if (category != null && subCategory != null) {
                        final SubCategory noteSubCategory = note.getSubCategory();
                        return noteSubCategory.getName().equals(subCategory) && noteSubCategory.getCategory().getName().equals(category);
                    } else {
                        return true;
                    }
                })
                .collect(Collectors.toList());
    }
}

Mam takze podany interfejs NoteClient który musze zaimplementowac, oczywiscie wszystko thread safe, nalezy przypomniec ze Note ma wszystkie pola final wiec jak zmieniam jakas zmienna to zwracam nowy obiekt (Note jest Immutable):

public class NoteClientImpl implements NoteClient{

    private final NoteRepository noteRepository;
    private final Object updateLock = new Object();

    public ClientImpl(NoteRepository noteRepository){
        Objects.requireNonNull(noteRepository, "NoteRepository can not be null");
        this.noteRepository = noteRepository;
    }

    @Override
    public void addNote(Note note) {
        noteRepository.save(note);
    }

    @Override
    public void noteCompleted(Integer id) {
        noteRepository
                .findById(id)
                .map(note -> {
                    synchronized (updateLock) {
                        Note updatedNote = note.updateCompleted();
                        return noteRepository.update(updatedNote);
                    }
                });
    }

    @Override
    public void addRefTypeToNote(Integer id, RefType type) {
        noteRepository
                .findById(id)
                .map(note->{
                    synchronized (updateLock) {
                        Note updatedNote = note.addRefType(type);
                        return noteRepository.update(updatedNote);
                    }
                });
    }

    @Override
    public void removeRefTypeFromNote(Integer id, RefType type) {
        noteRepository
                .findById(id)
                .map(note->{
                    synchronized (updateLock) {
                        Note updatedNote = note.removeRefType(type);
                        return noteRepository.update(updatedNote);
                    }
                });
    }

    @Override
    public Collection<String> futureNote(String category, String subCategory, String nameRef) {
        return noteRepository
                .findCompletedNorteByCategoryAndSubCategoryAndMarketRefName(category, subCategory, nameRef)
                .stream()
                .map(Note::getName)
                .collect(Collectors.toList());
    }
    
    @Override
    public String allStructure() {
        return noteRepository
                .findAll()
                .stream()
                .map(Note::toString)
                .collect(Collectors.joining(", "));
    }
}

Obiekt Note, tych pól i implementacji nie można zmieniać, takie wymagania:

public class Note implements Serializable {

    private final Integer id;
    private final String name;
    private final SubCategory subCategory;
    private final Collection<RefType> refTypes;
    private final Boolean completed;

    public Note(Integer id,
                 String name,
                 SubCategory subCategory,
                 Collection<RefType> refTypes,
                 Boolean completed
    ) {
        Objects.requireNonNull(id, "Id can not be null");
        Objects.requireNonNull(name, "Name can not be null");
        Objects.requireNonNull(subCategory, "SubCategory can not be null");
        Objects.requireNonNull(completed, "Completed can not be null");
        this.id = id;
        this.name = name;
        this.subCategory = subCategory;
        //I do not know business domain co i think that event can exist without marketRefTypes
        this.refTypes = (refTypes != null ? refTypes : Collections.emptyList());
        this.completed = completed;
    }
......
//metody dodane przeze mnie:
 public Note addRefType(RefType refType) {
        ensureNotCompleted();
        if (hasRefName(refType.getRefName())) {
            return this;
        }
        Collection<RefType> updatedType = getRefTypes();
        updatedType.add(refType);
        return new Note(id, name, subCategory, updatedType, completed);
    }

    public Note removeRefType(RefType refType) {
        ensureNotCompleted();
        if (hasRefName(refType.getRefName())) {
            Collection<RefType> updatedRefTypes = getRefTypes();
            updatedRefTypes.remove(refType);
            return new Note(id, name, subCategory, updatedRefTypes, completed);
        }
        return this;
    }

   private void ensureNotCompleted() {
        if (getCompleted()) {
            throw new NoteAlreadyCompletedException();
        }
    }

    public Note updateCompleted() {
        ensureNotCompleted();
        return new Note(id, name, subCategory, marketRefTypes, true);
    }
}

Zastanawiam się nad 3 sprawami :

  1. Musze synchronizowac wszystkie operacje update na evencie i najlepiej chyba do tego dodac obiekt zamiast this aby nie blokowac odczytów ?
  2. Jak można przeprowadzić testy czy wszystko jest thread safe ? Czy wszystkie operacje są ok?
  3. Czy można to zaimplementować lepiej ?
0

Jeżeli to jest thread safe to dlaczego nie oznaczysz pola noteStorage jako final ??
Czy obiekt Note jest immutable ?

0

Dlaczego to jest synchronizowane ?

    @Override
    public void noteCompleted(Integer id) {
        noteRepository
                .findById(id)
                .map(note -> {
                    synchronized (updateLock) {
                        Note updatedNote = note.updateCompleted();
                        return noteRepository.update(updatedNote);
                    }
                });
    }
    @Override
    public void addRefTypeToNote(Integer id, RefType type) {
        noteRepository
                .findById(id)
                .map(note->{
                    synchronized (updateLock) {
                        Note updatedNote = note.addRefType(type);
                        return noteRepository.update(updatedNote);
                    }
                });

eh....

niech refType w Note będzie ConcurrentLinkedQueue będziesz mógł zrobić

    @Override
    public void addRefTypeToNote(Integer id, RefType type) {
        noteRepository
                .findById(id)
                .map(note->{
                        note.addRefType(type); //pod spodem masz  thread-safe wait-free kolekcje 
                });

to samo tutaj

 
    @Override
    public void removeRefTypeFromNote(Integer id, RefType type) {
        noteRepository
                .findById(id)
                .map(note->{
                    synchronized (updateLock) {
                        Note updatedNote = note.removeRefType(type);
                        return noteRepository.update(updatedNote);
                    }
                });
    }

Tak ogólnie to nie szalej tak z tymi lockami - to kosztuje

P.S

dodatkowo jeżeli nawet musiałbyś to synchronizować, i dodałbyś możliwość usuwania tych elementów to ta operacja

    @Override
    public void addRefTypeToNote(Integer id, RefType type) {
        noteRepository
                .findById(id)
                .map(note->{
                    synchronized (updateLock) {
                        Note updatedNote = note.addRefType(type);
                        return noteRepository.update(updatedNote);
                    }
                });

nie jest atomowa, możesz pobrać id, ktoś wywali element (zakładając że masz tego samego locka dla tych operacji) i pupa, masz nie konsystencje :) Oczywiście, może być to dla ciebie dopuszczalne, zależy co chesz osiągnąć :)

0

Ok masz rację jest jakiś prosty sposób, żeby tego uniknąć ?, nie jestem ekspertem od concurrency i dopiero co zacząłem czytać wspomnianą przez Ciebie książkę :)

W coś takiego bym szedł. Jak wiesz że będziesz miał obiekt modyfikowany prze kilka wątków i wrzucasz tam jako pole Collection - to wiedz że coś się dzieje, pogratuluj kolegom z pracy ( tutaj ich pozdrawiamy )

	public static class Note implements Serializable
	{

		private final Integer id;
		private final String name;
		private final SubCategory subCategory;
		private final Collection<RefType> immutableRefTypes;
		private final Boolean completed;

		public Note(Integer id, String name, SubCategory subCategory, Collection<RefType> refTypes, Boolean completed)
		{
			Objects.requireNonNull(id, "Id can not be null");
			Objects.requireNonNull(name, "Name can not be null");
			Objects.requireNonNull(subCategory, "SubCategory can not be null");
			Objects.requireNonNull(completed, "Completed can not be null");
			this.id = id;
			this.name = name;
			this.subCategory = subCategory;
			//I do not know business domain co i think that event can exist without marketRefTypes
			this.immutableRefTypes = (refTypes != null ? ImmutableList.copyOf(refTypes) : Collections.emptyList());
			this.completed = completed;
		}

		public Note addRefType(RefType refType) {
			ensureNotCompleted();
			if (hasRefName(refType.getRefName()))
			{
				return this;
			}
			return new Note(id, name, subCategory, ImmutableList.<RefType>builder().add(refType).addAll(immutableRefTypes).build(), completed);
		}

		public Note removeRefType(RefType refType) {
			ensureNotCompleted();
			if (hasRefName(refType.getRefName())) {
				
				return new Note(id, name, subCategory, ImmutableList.copyOf(Collections2.filter(immutableRefTypes, Predicates.not(Predicates.equalTo(refType)))), completed);
			}
			return this;
		}

	}

Tutaj jedyne co zrobiłem to dodałem niezmienną kolekcje z guavy (import dla gradla com.google.guava19.0 ) - i już masz niezmienny obiekt, a jak przekazujesz coś dalej, to tworzysz nową niezmienną kolekcje ::) jeszcze co bym wywalił Collections.emptyList() na implementacje z guavy ale misię nie chce.

Ja tego nie kompilowałem D:D chciałem Ci tylko idee nakreślić :D - ale powinno zadziałać od strzała

Ale mi sie to nie podoba, wiesz że masz service który robi takie coś jak robi (modyfikuje podległą litę która znajduje się w obiekcie Note ) - to pod to powinieneś zaimplementować obiekty a nie na odwrót - teraz musisz lockować - taka kaszana że nie mogę ... (wiem że nie twoja wina, nie ty robiłeś ten obiekt Note, ktoś tam się chciał pochwalić robiąc takie zadanie testowe i chyba mu nie wyszło :D)

0

Boolean nie rozumiem dlaczego ktoś używa Boolean a nie typu prostego boolean

pewnie pisali to na pałe :D

Tutaj nie będzie problemu z tym ze completed nie jest violatile ?

masz racje - zapomniałem o tym :D tak to będzie problem, bo przekazujesz ten obiekt dalej,
rozwiazania są trzy, tworzy nowy obiekt Boolean, albo zmieniasz na prymitywa albo robisz to jako volotile albo robisz AtomicBoolean - to cztery :D

czyli powinniśmy chyba synchronizować metody updateCompleted, removeRefType, addRefType bo gdybyśmy mieli completed violatile to by nie było problemu

Masz przeciez locki na metodach servicu - to daje Ci gwarancje poprawnosci statu dla kolekcji na ktorej ten service operuje, jak ktos w tym samym czasie bokiem wywoła removeRefType to stworzy nowy obiekt - nic sie nie stanie

PS polecasz jakieś materiały, żeby napisać testy pod thread safe ?

Nie mam pojęcia jak tutaj można sensowanie taki test przeprowadzić, co odpalisz 100 wątków na raz z nadzieją że coś się stanie ? Logikę możesz przetestować.

1 użytkowników online, w tym zalogowanych: 0, gości: 1